From patchwork Mon Dec 10 21:11:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 153359 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp4019487ljp; Mon, 10 Dec 2018 13:13:06 -0800 (PST) X-Google-Smtp-Source: AFSGD/UVaAaUuC3oeV2WSL6s5IV+dE06qEEdbGl2YdFNKnQlfpvtI8mGigoedz00jAYkolZ6pJ6B X-Received: by 2002:a65:41c2:: with SMTP id b2mr12109177pgq.67.1544476385885; Mon, 10 Dec 2018 13:13:05 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1544476385; cv=none; d=google.com; s=arc-20160816; b=jsY878rmXf3eOc94ea+40IygWFatjxLbIo0u1mmYLmVA+fefnRR0cFUyREgkqF5fvi EgkSBxke3hys7teZ+bIY3gW1pNvRP4AIijOBDpWzZcBwJ5Zo1oIGxD+aW/n+UENE6VtJ NUWLHKfwDEl760YR3HrvW7D1M5BkDq8xUErtn2eCQLlLP4LHS4MTTwDp/k65h62ywIin hw4NEGVnIo2IBJ8nVLswdn4RxXgXg8mnB8V7B8KvUvwzUhGKzugHxuOsKGoZepig2R8D ffc+9roOOoD67bkbGbIZTsKUJCabfF7rQI1xyRPtzODSykYxgekfqA/JlfmkWKv6qKnD TVrw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=lxUOIyISKNuNVSrgQRevq/s2rwgbvtDDT8Dn2agvGGY=; b=qNAKddabDqWwQPq0GRDuLWHjtAe3M3ta4hsWwqmAfSXISa9GDO+d27Ag5/obQSG2ia 3h4AvKPWbXczsT4Ygru+sXy8wt2RQbe5nIMeDC0sEvOt9tzLv2gALyImSNdgn8qHR1tY 2E88RDjy7Ci/fs8jKglkgOVTuP6kEKryUahYsGOHzKD0kZ4Y3phQheW9F0TnNrW8uhbS 1tu3EGNn3l3RUsCtuTbot61rjh0P9YsoLgK48s6dD+JgfTDQT5riAJj6dOSdX6iCJ7zl AbDMr25u0eFI9BJXAoPr9ORIuuogk4CydOqZrUXCopbPqqksW1yCN1hQ/d45jDpDaVFs 4YXg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id o21si9336780pgj.415.2018.12.10.13.13.05; Mon, 10 Dec 2018 13:13:05 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729077AbeLJVNE (ORCPT + 31 others); Mon, 10 Dec 2018 16:13:04 -0500 Received: from mout.kundenserver.de ([212.227.17.10]:36305 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728303AbeLJVNE (ORCPT ); Mon, 10 Dec 2018 16:13:04 -0500 Received: from wuerfel.lan ([109.192.41.194]) by mrelayeu.kundenserver.de (mreue106 [212.227.15.145]) with ESMTPA (Nemesis) id 1M58OQ-1gVMOL3P2y-001AvP; Mon, 10 Dec 2018 22:12:33 +0100 From: Arnd Bergmann To: Eric Anholt , Stefan Wahren , Greg Kroah-Hartman Cc: Arnd Bergmann , Nicolas Saenz Julienne , Tara Null , linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: [PATCH] staging: vchiq: rework remove_event handling Date: Mon, 10 Dec 2018 22:11:58 +0100 Message-Id: <20181210211231.4084251-1-arnd@arndb.de> X-Mailer: git-send-email 2.20.0 MIME-Version: 1.0 X-Provags-ID: V03:K1:Qy/+4ZfSLzA8az2nBv7MNP8W5PmCaWXwLTxQ5PX9KmzGRsVcWqN oN7V3HDjK3fUaJxUwYOgN3Ux6MydnpyxhadJ6rP6NN9YDT2EOqDk8DHEWxVdy4mWcev5qjq hI/Ij/38EVuYy9kpAOCjYGOApvZkxHg3hLGXWFShKIXJgz+pGGKdYmrr8G2Ilaze3nVygvZ QIKysNREwM8ccCrb/9KfQ== X-Spam-Flag: NO X-UI-Out-Filterresults: notjunk:1; V03:K0:796iQkfXSdo=:Z1NVAwbOeU7FM4ov35ifzY 8cQc0CqrBbCfoBzhuSNmIfCXmTkkd9T0J+kvFRIojiGHddCv1ssF2rKbJkVCEbNubADdHGy6P VlKf4vFqUYNsbwSfY9Ya9edwkLDp4qZ6OzNsaCbULaU2UO/G29AE1LuGhdUuMMncXQy34QN/j iG/ZUVVo1Vq5j04bXRpjJq4TX8WUwMEthJt1wt2YLqhthy06Tgjh9PdzfIBo0gx8NazEmCwdf Rv/OUv5KhOthvhlFPRHSg+p4dOP/31ad4IcQfUJ+meBoP3CMYssT7rh1rXQwYDVeKB19l/MyL qLzV1uvnsiUor1nvJ3HnbY7Wm2Ysjv21/Qz07zNlWniQ5mJNZyomQexVfSQNMTpi8uwXtUEyu OZeSyK7dVQfSJjjsqgg0bIdCKLn9Fv9BSyaUTliJzX/wOT0nbfywFMWQYhPzfd4WFldtAZu9i 3/33xgikZAYODGsjFai7dvDhH09cH6dqf+TKbCLsoB272LE+5lW4JgzaEgxcR9rdP/a0J6XnL S4ty2FH5QjT6nAF1utwRey7A81b0/reC+i/+IRBPlweAGmyPk1cCNC1lGdMKbh6ybFdTHIF5v uXS3rXERJWLtL3uB3OTBVPuP6QK7OTWj7nWqVnvuFh/SL9gX/MmP7S9H3zpMN1AIuqcRlbDLx E71KYaXpOIwcbx0+2JqrpXZtEkROgbBlOEOGqRskzl9cg6PClFRFvrIcNZJ/wfBNDj0hihksU VPzjL/P/i6ZmaoEhO1yr3qBLGdlKwA0OMbr3PQ== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I had started the removal of semaphores in this driver without knowing that Nicolas Saenz Julienne also worked on this. In case of the "remote event" infrastructure, my solution seemed significantly better, so I'm proposing this as a change on top. The problem with using either semaphores or completions here is that it's an overly complex way of waking up a thread, and it looks like the 'count' of the semaphore can easily get out of sync, even though I found it hard to come up with a specific example. Changing it to a 'wait_queue_head_t' instead of a completion simplifies this by letting us wait directly on the 'event->fired' variable that is set by the videocore. Another simplification is passing the wait queue directly into the helper functions instead of going through the fragile logic of recording the offset inside of a structure as part of a shared memory variable. This also avoids one uncached memory read and should be faster. Note that I'm changing it back to 'killable' after the previous patch changed 'killable' to 'interruptible', apparently based on a misunderstanding of the subtle down_interruptible() macro override in vchiq_killable.h. Fixes: f27e47bc6b8b ("staging: vchiq: use completions instead of semaphores") Signed-off-by: Arnd Bergmann --- .../interface/vchiq_arm/vchiq_core.c | 63 +++++++------------ .../interface/vchiq_arm/vchiq_core.h | 12 ++-- 2 files changed, 30 insertions(+), 45 deletions(-) -- 2.20.0 Reviewed-by: Nicolas Saenz Julienne diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c index 482b5daf6c0c..eda3004a0c6a 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -417,26 +417,23 @@ vchiq_set_conn_state(VCHIQ_STATE_T *state, VCHIQ_CONNSTATE_T newstate) } static inline void -remote_event_create(REMOTE_EVENT_T *event) +remote_event_create(wait_queue_head_t *wq, REMOTE_EVENT_T *event) { event->armed = 0; /* Don't clear the 'fired' flag because it may already have been set ** by the other side. */ + init_waitqueue_head(wq); } static inline int -remote_event_wait(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event) +remote_event_wait(wait_queue_head_t *wq, REMOTE_EVENT_T *event) { if (!event->fired) { event->armed = 1; dsb(sy); - if (!event->fired) { - if (wait_for_completion_interruptible( - (struct completion *) - ((char *)state + event->event))) { - event->armed = 0; - return 0; - } + if (wait_event_killable(*wq, event->fired)) { + event->armed = 0; + return 0; } event->armed = 0; wmb(); @@ -447,26 +444,26 @@ remote_event_wait(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event) } static inline void -remote_event_signal_local(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event) +remote_event_signal_local(wait_queue_head_t *wq, REMOTE_EVENT_T *event) { event->armed = 0; - complete((struct completion *)((char *)state + event->event)); + wake_up_all(wq); } static inline void -remote_event_poll(VCHIQ_STATE_T *state, REMOTE_EVENT_T *event) +remote_event_poll(wait_queue_head_t *wq, REMOTE_EVENT_T *event) { if (event->fired && event->armed) - remote_event_signal_local(state, event); + remote_event_signal_local(wq, event); } void remote_event_pollall(VCHIQ_STATE_T *state) { - remote_event_poll(state, &state->local->sync_trigger); - remote_event_poll(state, &state->local->sync_release); - remote_event_poll(state, &state->local->trigger); - remote_event_poll(state, &state->local->recycle); + remote_event_poll(&state->sync_trigger_event, &state->local->sync_trigger); + remote_event_poll(&state->sync_release_event, &state->local->sync_release); + remote_event_poll(&state->trigger_event, &state->local->trigger); + remote_event_poll(&state->recycle_event, &state->local->recycle); } /* Round up message sizes so that any space at the end of a slot is always big @@ -550,7 +547,7 @@ request_poll(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service, int poll_type) wmb(); /* ... and ensure the slot handler runs. */ - remote_event_signal_local(state, &state->local->trigger); + remote_event_signal_local(&state->trigger_event, &state->local->trigger); } /* Called from queue_message, by the slot handler and application threads, @@ -1069,7 +1066,7 @@ queue_message_sync(VCHIQ_STATE_T *state, VCHIQ_SERVICE_T *service, (mutex_lock_killable(&state->sync_mutex) != 0)) return VCHIQ_RETRY; - remote_event_wait(state, &local->sync_release); + remote_event_wait(&state->sync_release_event, &local->sync_release); rmb(); @@ -1887,7 +1884,7 @@ slot_handler_func(void *v) while (1) { DEBUG_COUNT(SLOT_HANDLER_COUNT); DEBUG_TRACE(SLOT_HANDLER_LINE); - remote_event_wait(state, &local->trigger); + remote_event_wait(&state->trigger_event, &local->trigger); rmb(); @@ -1976,7 +1973,7 @@ recycle_func(void *v) return -ENOMEM; while (1) { - remote_event_wait(state, &local->recycle); + remote_event_wait(&state->recycle_event, &local->recycle); process_free_queue(state, found, length); } @@ -1998,7 +1995,7 @@ sync_func(void *v) int type; unsigned int localport, remoteport; - remote_event_wait(state, &local->sync_trigger); + remote_event_wait(&state->sync_trigger_event, &local->sync_trigger); rmb(); @@ -2193,11 +2190,6 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero) init_completion(&state->connect); mutex_init(&state->mutex); - init_completion(&state->trigger_event); - init_completion(&state->recycle_event); - init_completion(&state->sync_trigger_event); - init_completion(&state->sync_release_event); - mutex_init(&state->slot_mutex); mutex_init(&state->recycle_mutex); mutex_init(&state->sync_mutex); @@ -2229,24 +2221,17 @@ vchiq_init_state(VCHIQ_STATE_T *state, VCHIQ_SLOT_ZERO_T *slot_zero) state->data_use_count = 0; state->data_quota = state->slot_queue_available - 1; - local->trigger.event = offsetof(VCHIQ_STATE_T, trigger_event); - remote_event_create(&local->trigger); + remote_event_create(&state->trigger_event, &local->trigger); local->tx_pos = 0; - - local->recycle.event = offsetof(VCHIQ_STATE_T, recycle_event); - remote_event_create(&local->recycle); + remote_event_create(&state->recycle_event, &local->recycle); local->slot_queue_recycle = state->slot_queue_available; - - local->sync_trigger.event = offsetof(VCHIQ_STATE_T, sync_trigger_event); - remote_event_create(&local->sync_trigger); - - local->sync_release.event = offsetof(VCHIQ_STATE_T, sync_release_event); - remote_event_create(&local->sync_release); + remote_event_create(&state->sync_trigger_event, &local->sync_trigger); + remote_event_create(&state->sync_release_event, &local->sync_release); /* At start-of-day, the slot is empty and available */ ((VCHIQ_HEADER_T *)SLOT_DATA_FROM_INDEX(state, local->slot_sync))->msgid = VCHIQ_MSGID_PADDING; - remote_event_signal_local(state, &local->sync_release); + remote_event_signal_local(&state->sync_release_event, &local->sync_release); local->debug[DEBUG_ENTRIES] = DEBUG_MAX; diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h index b76281f7510e..aae2c59700bd 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h @@ -37,6 +37,7 @@ #include #include #include +#include #include "vchiq_cfg.h" @@ -262,8 +263,7 @@ typedef struct vchiq_bulk_queue_struct { typedef struct remote_event_struct { int armed; int fired; - /* Contains offset from the beginning of the VCHIQ_STATE_T structure */ - u32 event; + u32 __unused; } REMOTE_EVENT_T; typedef struct opaque_platform_state_t *VCHIQ_PLATFORM_STATE_T; @@ -426,16 +426,16 @@ struct vchiq_state_struct { struct task_struct *sync_thread; /* Local implementation of the trigger remote event */ - struct completion trigger_event; + wait_queue_head_t trigger_event; /* Local implementation of the recycle remote event */ - struct completion recycle_event; + wait_queue_head_t recycle_event; /* Local implementation of the sync trigger remote event */ - struct completion sync_trigger_event; + wait_queue_head_t sync_trigger_event; /* Local implementation of the sync release remote event */ - struct completion sync_release_event; + wait_queue_head_t sync_release_event; char *tx_data; char *rx_data;