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 <phil@raspberrypi.org>
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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 72945f9a90ad09e4a6c206c72499750e0fa59302..b02dc4b1e76cebffa7bc4f666eced3c4c97068fd 100644 (file)
--- 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 9867e647f7070e7200c63bd115ac545965bdc1bb..d587097b261c845921725479346add1c2807156d 100644 (file)
--- 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);
        }