target/linux/brcm2708/patches-4.4/0427-vchiq_arm-Avoid-use-of-mutex-in-add_completion.patch

   1 From 00d6172964200c5d72cef2264ff09d38f764bd69 Mon Sep 17 00:00:00 2001
   2 From: Phil Elwell <phil@raspberrypi.org>
   3 Date: Mon, 20 Jun 2016 13:51:44 +0100
   4 Subject: [PATCH] vchiq_arm: Avoid use of mutex in add_completion
   5
   6 Claiming the completion_mutex within add_completion did prevent some
   7 messages appearing twice, but provokes a deadlock caused by vcsm using
   8 vchiq within a page fault handler.
   9
  10 Revert the use of completion_mutex, and instead fix the original
  11 problem using more memory barriers.
  12
  13 Signed-off-by: Phil Elwell <phil@raspberrypi.org>
  14 ---
  15  .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 55 +++++++++++-----------
  16  .../vc04_services/interface/vchiq_arm/vchiq_core.c | 14 ++++--
  17  2 files changed, 37 insertions(+), 32 deletions(-)
  18
  19 --- a/drivers/misc/vc04_services/interface/vchiq_arm/vchiq_arm.c
  20 +++ b/drivers/misc/vc04_services/interface/vchiq_arm/vchiq_arm.c
  21 @@ -64,10 +64,10 @@
  22  #define VCHIQ_MINOR 0
  23
  24  /* Some per-instance constants */
  25 -#define MAX_COMPLETIONS 16
  26 +#define MAX_COMPLETIONS 128
  27  #define MAX_SERVICES 64
  28  #define MAX_ELEMENTS 8
  29 -#define MSG_QUEUE_SIZE 64
  30 +#define MSG_QUEUE_SIZE 128
  31
  32  #define KEEPALIVE_VER 1
  33  #define KEEPALIVE_VER_MIN KEEPALIVE_VER
  34 @@ -208,28 +208,24 @@ add_completion(VCHIQ_INSTANCE_T instance
  35         void *bulk_userdata)
  36  {
  37         VCHIQ_COMPLETION_DATA_T *completion;
  38 +       int insert;
  39         DEBUG_INITIALISE(g_state.local)
  40
  41 -       mutex_lock(&instance->completion_mutex);
  42 -
  43 -       while (instance->completion_insert ==
  44 -               (instance->completion_remove + MAX_COMPLETIONS)) {
  45 +       insert = instance->completion_insert;
  46 +       while ((insert - instance->completion_remove) >= MAX_COMPLETIONS) {
  47                 /* Out of space - wait for the client */
  48                 DEBUG_TRACE(SERVICE_CALLBACK_LINE);
  49                 vchiq_log_trace(vchiq_arm_log_level,
  50                         "add_completion - completion queue full");
  51                 DEBUG_COUNT(COMPLETION_QUEUE_FULL_COUNT);
  52
  53 -               mutex_unlock(&instance->completion_mutex);
  54                 if (down_interruptible(&instance->remove_event) != 0) {
  55                         vchiq_log_info(vchiq_arm_log_level,
  56                                 "service_callback interrupted");
  57                         return VCHIQ_RETRY;
  58                 }
  59
  60 -               mutex_lock(&instance->completion_mutex);
  61                 if (instance->closing) {
  62 -                       mutex_unlock(&instance->completion_mutex);
  63                         vchiq_log_info(vchiq_arm_log_level,
  64                                 "service_callback closing");
  65                         return VCHIQ_SUCCESS;
  66 @@ -237,9 +233,7 @@ add_completion(VCHIQ_INSTANCE_T instance
  67                 DEBUG_TRACE(SERVICE_CALLBACK_LINE);
  68         }
  69
  70 -       completion =
  71 -                &instance->completions[instance->completion_insert &
  72 -                (MAX_COMPLETIONS - 1)];
  73 +       completion = &instance->completions[insert & (MAX_COMPLETIONS - 1)];
  74
  75         completion->header = header;
  76         completion->reason = reason;
  77 @@ -260,12 +254,9 @@ add_completion(VCHIQ_INSTANCE_T instance
  78         wmb();
  79
  80         if (reason == VCHIQ_MESSAGE_AVAILABLE)
  81 -               user_service->message_available_pos =
  82 -                       instance->completion_insert;
  83 -
  84 -       instance->completion_insert++;
  85 +               user_service->message_available_pos = insert;
  86
  87 -       mutex_unlock(&instance->completion_mutex);
  88 +       instance->completion_insert = ++insert;
  89
  90         up(&instance->insert_event);
  91
  92 @@ -795,6 +786,7 @@ vchiq_ioctl(struct file *file, unsigned
  93                         instance->completion_insert)
  94                         && !instance->closing) {
  95                         int rc;
  96 +
  97                         DEBUG_TRACE(AWAIT_COMPLETION_LINE);
  98                         mutex_unlock(&instance->completion_mutex);
  99                         rc = down_interruptible(&instance->insert_event);
 100 @@ -809,24 +801,29 @@ vchiq_ioctl(struct file *file, unsigned
 101                 }
 102                 DEBUG_TRACE(AWAIT_COMPLETION_LINE);
 103
 104 -               /* A read memory barrier is needed to stop prefetch of a stale
 105 -               ** completion record
 106 -               */
 107 -               rmb();
 108 -
 109                 if (ret == 0) {
 110                         int msgbufcount = args.msgbufcount;
 111 +                       int remove;
 112 +
 113 +                       remove = instance->completion_remove;
 114 +
 115                         for (ret = 0; ret < args.count; ret++) {
 116                                 VCHIQ_COMPLETION_DATA_T *completion;
 117                                 VCHIQ_SERVICE_T *service;
 118                                 USER_SERVICE_T *user_service;
 119                                 VCHIQ_HEADER_T *header;
 120 -                               if (instance->completion_remove ==
 121 -                                       instance->completion_insert)
 122 +
 123 +                               if (remove == instance->completion_insert)
 124                                         break;
 125 +
 126                                 completion = &instance->completions[
 127 -                                       instance->completion_remove &
 128 -                                       (MAX_COMPLETIONS - 1)];
 129 +                                       remove & (MAX_COMPLETIONS - 1)];
 130 +
 131 +
 132 +                               /* A read memory barrier is needed to prevent
 133 +                               ** the prefetch of a stale completion record
 134 +                               */
 135 +                               rmb();
 136
 137                                 service = completion->service_userdata;
 138                                 user_service = service->base.userdata;
 139 @@ -903,7 +900,11 @@ vchiq_ioctl(struct file *file, unsigned
 140                                         break;
 141                                 }
 142
 143 -                               instance->completion_remove++;
 144 +                               /* Ensure that the above copy has completed
 145 +                               ** before advancing the remove pointer. */
 146 +                               mb();
 147 +
 148 +                               instance->completion_remove = ++remove;
 149                         }
 150
 151                         if (msgbufcount != args.msgbufcount) {
 152 --- a/drivers/misc/vc04_services/interface/vchiq_arm/vchiq_core.c
 153 +++ b/drivers/misc/vc04_services/interface/vchiq_arm/vchiq_core.c
 154 @@ -610,15 +610,15 @@ process_free_queue(VCHIQ_STATE_T *state)
 155         BITSET_T service_found[BITSET_SIZE(VCHIQ_MAX_SERVICES)];
 156         int slot_queue_available;
 157
 158 -       /* Use a read memory barrier to ensure that any state that may have
 159 -       ** been modified by another thread is not masked by stale prefetched
 160 -       ** values. */
 161 -       rmb();
 162 -
 163         /* Find slots which have been freed by the other side, and return them
 164         ** to the available queue. */
 165         slot_queue_available = state->slot_queue_available;
 166
 167 +       /* Use a memory barrier to ensure that any state that may have been
 168 +       ** modified by another thread is not masked by stale prefetched
 169 +       ** values. */
 170 +       mb();
 171 +
 172         while (slot_queue_available != local->slot_queue_recycle) {
 173                 unsigned int pos;
 174                 int slot_index = local->slot_queue[slot_queue_available++ &
 175 @@ -626,6 +626,8 @@ process_free_queue(VCHIQ_STATE_T *state)
 176                 char *data = (char *)SLOT_DATA_FROM_INDEX(state, slot_index);
 177                 int data_found = 0;
 178
 179 +               rmb();
 180 +
 181                 vchiq_log_trace(vchiq_core_log_level, "%d: pfq %d=%x %x %x",
 182                         state->id, slot_index, (unsigned int)data,
 183                         local->slot_queue_recycle, slot_queue_available);
 184 @@ -741,6 +743,8 @@ process_free_queue(VCHIQ_STATE_T *state)
 185                                 up(&state->data_quota_event);
 186                 }
 187
 188 +               mb();
 189 +
 190                 state->slot_queue_available = slot_queue_available;
 191                 up(&state->slot_available_event);
 192         }