Hi Umang,
Am 03.07.24 um 15:10 schrieb Umang Jain:
wait_event_interruptible() returns if the condition evaluates to true
it receives a signal. However, the current code always assume that the
wait_event_interruptible() returns only when the event is fired.
This should not be the case as wait_event_interruptible() can
return on receiving a signal (with -ERESTARTSYS as return value).
We should consider this and bubble up the return value of
wait_event_interruptible() to exactly know if the wait has failed
and error out. This will also help to properly stop kthreads in the
subsequent patch.
Meanwhile at it, remote_wait_event() is modified to return 0 on success,
and an error code (from wait_event_interruptible()) on failure. The
return value is now checked for remote_wait_event() calls.
Signed-off-by: Umang Jain <umang.jain@xxxxxxxxxxxxxxxx>
---
.../interface/vchiq_arm/vchiq_core.c | 31 ++++++++++++++-----
1 file changed, 24 insertions(+), 7 deletions(-)
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 4f65e4021c4d..dd70f2881eca 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -501,16 +501,21 @@ remote_event_create(wait_queue_head_t *wq, struct remote_event *event)
* routines where switched to the "interruptible" family of functions, as the
* former was deemed unjustified and the use "killable" set all VCHIQ's
* threads in D state.
+ *
+ * Returns: 0 on success, a negative error code on failure
*/
static inline int
remote_event_wait(wait_queue_head_t *wq, struct remote_event *event)
{
+ int ret = 0;
+
if (!event->fired) {
event->armed = 1;
dsb(sy);
- if (wait_event_interruptible(*wq, event->fired)) {
+ ret = wait_event_interruptible(*wq, event->fired);
+ if (ret) {
event->armed = 0;
- return 0;
+ return ret;
}
event->armed = 0;
/* Ensure that the peer sees that we are not waiting (armed == 0). */
@@ -518,7 +523,7 @@ remote_event_wait(wait_queue_head_t *wq, struct remote_event *event)
}
event->fired = 0;
- return 1;
+ return ret;
in general this patch looks good to me. But maybe we better return 0
directly and reduce the scope of ret.
}
/*
@@ -1140,6 +1145,7 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
struct vchiq_header *header;
ssize_t callback_result;
int svc_fourcc;
+ int ret;
local = state->local;
@@ -1147,7 +1153,9 @@ queue_message_sync(struct vchiq_state *state, struct vchiq_service *service,
mutex_lock_killable(&state->sync_mutex))
return -EAGAIN;
- remote_event_wait(&state->sync_release_event, &local->sync_release);
+ ret = remote_event_wait(&state->sync_release_event, &local->sync_release);
+ if (ret)
+ return ret;
/* Ensure that reads don't overtake the remote_event_wait. */
rmb();
@@ -1929,13 +1937,16 @@ slot_handler_func(void *v)
{
struct vchiq_state *state = v;
struct vchiq_shared_state *local = state->local;
+ int ret;
DEBUG_INITIALISE(local);
while (1) {
DEBUG_COUNT(SLOT_HANDLER_COUNT);
DEBUG_TRACE(SLOT_HANDLER_LINE);
- remote_event_wait(&state->trigger_event, &local->trigger);
+ ret = remote_event_wait(&state->trigger_event, &local->trigger);
+ if (ret)
+ return ret;
/* Ensure that reads don't overtake the remote_event_wait. */
rmb();
@@ -1966,6 +1977,7 @@ recycle_func(void *v)
struct vchiq_shared_state *local = state->local;
u32 *found;
size_t length;
+ int ret;
length = sizeof(*found) * BITSET_SIZE(VCHIQ_MAX_SERVICES);
@@ -1975,7 +1987,9 @@ recycle_func(void *v)
return -ENOMEM;
while (1) {
- remote_event_wait(&state->recycle_event, &local->recycle);
+ ret = remote_event_wait(&state->recycle_event, &local->recycle);
+ if (ret)
+ return ret;
process_free_queue(state, found, length);
}
@@ -1992,6 +2006,7 @@ sync_func(void *v)
(struct vchiq_header *)SLOT_DATA_FROM_INDEX(state,
state->remote->slot_sync);
int svc_fourcc;
+ int ret;
while (1) {
struct vchiq_service *service;
@@ -1999,7 +2014,9 @@ sync_func(void *v)
int type;
unsigned int localport, remoteport;
- remote_event_wait(&state->sync_trigger_event, &local->sync_trigger);
+ ret = remote_event_wait(&state->sync_trigger_event, &local->sync_trigger);
+ if (ret)
+ return ret;
/* Ensure that reads don't overtake the remote_event_wait. */
rmb();
