From ca3df03b83f2abaf0c50d777eccf4d12bc4d4dec Mon Sep 17 00:00:00 2001 From: Phil Elwell Date: Tue, 17 Jan 2017 20:56:14 +0000 Subject: [PATCH] staging: vc04_services: Fix messages appearing twice An issue was observed when flushing openmax components which generate a large number of messages returning buffers to host. We occasionally found a duplicate message from 16 messages prior, resulting in a buffer returned twice. So fix the issue by adding more memory barriers. Signed-off-by: Phil Elwell Signed-off-by: Stefan Wahren Signed-off-by: Greg Kroah-Hartman --- .../interface/vchiq_arm/vchiq_arm.c | 45 +++++++++++-------- .../interface/vchiq_arm/vchiq_core.c | 24 +++++++--- 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 72945f9a90ad..b02dc4b1e76c 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -208,10 +208,11 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason, void *bulk_userdata) { VCHIQ_COMPLETION_DATA_T *completion; + int insert; DEBUG_INITIALISE(g_state.local) - while (instance->completion_insert == - (instance->completion_remove + MAX_COMPLETIONS)) { + insert = instance->completion_insert; + while ((insert - instance->completion_remove) >= MAX_COMPLETIONS) { /* Out of space - wait for the client */ DEBUG_TRACE(SERVICE_CALLBACK_LINE); vchiq_log_trace(vchiq_arm_log_level, @@ -229,9 +230,7 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason, DEBUG_TRACE(SERVICE_CALLBACK_LINE); } - completion = - &instance->completions[instance->completion_insert & - (MAX_COMPLETIONS - 1)]; + completion = &instance->completions[insert & (MAX_COMPLETIONS - 1)]; completion->header = header; completion->reason = reason; @@ -252,9 +251,10 @@ add_completion(VCHIQ_INSTANCE_T instance, VCHIQ_REASON_T reason, wmb(); if (reason == VCHIQ_MESSAGE_AVAILABLE) - user_service->message_available_pos = - instance->completion_insert; - instance->completion_insert++; + user_service->message_available_pos = insert; + + insert++; + instance->completion_insert = insert; up(&instance->insert_event); @@ -895,24 +895,27 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) } DEBUG_TRACE(AWAIT_COMPLETION_LINE); - /* A read memory barrier is needed to stop prefetch of a stale - ** completion record - */ - rmb(); - if (ret == 0) { int msgbufcount = args.msgbufcount; + int remove = instance->completion_remove; + for (ret = 0; ret < args.count; ret++) { VCHIQ_COMPLETION_DATA_T *completion; VCHIQ_SERVICE_T *service; USER_SERVICE_T *user_service; VCHIQ_HEADER_T *header; - if (instance->completion_remove == - instance->completion_insert) + + if (remove == instance->completion_insert) break; + completion = &instance->completions[ - instance->completion_remove & - (MAX_COMPLETIONS - 1)]; + remove & (MAX_COMPLETIONS - 1)]; + + /* + * A read memory barrier is needed to stop + * prefetch of a stale completion record + */ + rmb(); service = completion->service_userdata; user_service = service->base.userdata; @@ -987,7 +990,13 @@ vchiq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) break; } - instance->completion_remove++; + /* + * Ensure that the above copy has completed + * before advancing the remove pointer. + */ + mb(); + remove++; + instance->completion_remove = remove; } if (msgbufcount != args.msgbufcount) { 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 9867e647f707..d587097b261c 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c @@ -607,15 +607,17 @@ process_free_queue(VCHIQ_STATE_T *state) BITSET_T service_found[BITSET_SIZE(VCHIQ_MAX_SERVICES)]; int slot_queue_available; - /* Use a read memory barrier to ensure that any state that may have - ** been modified by another thread is not masked by stale prefetched - ** values. */ - rmb(); - /* Find slots which have been freed by the other side, and return them ** to the available queue. */ slot_queue_available = state->slot_queue_available; + /* + * Use a memory barrier to ensure that any state that may have been + * modified by another thread is not masked by stale prefetched + * values. + */ + mb(); + while (slot_queue_available != local->slot_queue_recycle) { unsigned int pos; int slot_index = local->slot_queue[slot_queue_available++ & @@ -623,6 +625,12 @@ process_free_queue(VCHIQ_STATE_T *state) char *data = (char *)SLOT_DATA_FROM_INDEX(state, slot_index); int data_found = 0; + /* + * Beware of the address dependency - data is calculated + * using an index written by the other side. + */ + rmb(); + vchiq_log_trace(vchiq_core_log_level, "%d: pfq %d=%pK %x %x", state->id, slot_index, data, local->slot_queue_recycle, slot_queue_available); @@ -721,6 +729,12 @@ process_free_queue(VCHIQ_STATE_T *state) up(&state->data_quota_event); } + /* + * Don't allow the slot to be reused until we are no + * longer interested in it. + */ + mb(); + state->slot_queue_available = slot_queue_available; up(&state->slot_available_event); } -- 2.20.1