From: Phil Elwell <phil@xxxxxxxxxxxxxxx>
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@xxxxxxxxxxxxxxx>
Signed-off-by: Stefan Wahren <stefan.wahren@xxxxxxxx>
---
.../vc04_services/interface/vchiq_arm/vchiq_arm.c | 45 ++++++++++++--------
.../vc04_services/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 72945f9..b02dc4b 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 @@ struct vchiq_instance_struct {
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 @@ struct vchiq_instance_struct {
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 @@ struct vchiq_instance_struct {
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 @@ struct vchiq_io_copy_callback_context {
}
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 @@ struct vchiq_io_copy_callback_context {
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 9867e64..d587097 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 @@ static const char *msg_type_str(unsigned int msg_type)
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 @@ static const char *msg_type_str(unsigned int msg_type)
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 @@ static const char *msg_type_str(unsigned int msg_type)
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);
}
--
1.7.9.5
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
