[Mesa-dev] [PATCH] nouveau: avoid queueing too much work onto a single fence

[Ilia Mirkin]

On Fri, Nov 6, 2015 at 1:27 AM, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> On Fri, 2015-11-06 at 00:46 -0500, Ilia Mirkin wrote:
>> Force the fence to get kicked off, which won't actually wait for its
>> completion, but any additional work will be put onto a fresh list.
>>
>> This fixes crashes in teximage-colors --benchmark with too many
>> active
>> maps.
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>  src/gallium/drivers/nouveau/nouveau_fence.c | 68 ++++++++++++++++++-
>> ----------
>>  src/gallium/drivers/nouveau/nouveau_fence.h |  1 +
>>  2 files changed, 43 insertions(+), 26 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/nouveau_fence.c
>> b/src/gallium/drivers/nouveau/nouveau_fence.c
>> index d3a3406..691553a 100644
>> --- a/src/gallium/drivers/nouveau/nouveau_fence.c
>> +++ b/src/gallium/drivers/nouveau/nouveau_fence.c
>> @@ -59,26 +59,6 @@ nouveau_fence_trigger_work(struct nouveau_fence
>> *fence)
>>     }
>>  }
>>
>> -bool
>> -nouveau_fence_work(struct nouveau_fence *fence,
>> -                   void (*func)(void *), void *data)
>> -{
>> -   struct nouveau_fence_work *work;
>> -
>> -   if (!fence || fence->state == NOUVEAU_FENCE_STATE_SIGNALLED) {
>> -      func(data);
>> -      return true;
>> -   }
>> -
>> -   work = CALLOC_STRUCT(nouveau_fence_work);
>> -   if (!work)
>> -      return false;
>> -   work->func = func;
>> -   work->data = data;
>> -   LIST_ADD(&work->list, &fence->work);
>> -   return true;
>> -}
>> -
>>  void
>>  nouveau_fence_emit(struct nouveau_fence *fence)
>>  {
>> @@ -182,12 +162,10 @@ nouveau_fence_signalled(struct nouveau_fence
>> *fence)
>>     return fence->state == NOUVEAU_FENCE_STATE_SIGNALLED;
>>  }
>>
>> -bool
>> -nouveau_fence_wait(struct nouveau_fence *fence, struct
>> pipe_debug_callback *debug)
>> +static bool
>> +nouveau_fence_kick(struct nouveau_fence *fence)
>>  {
>>     struct nouveau_screen *screen = fence->screen;
>> -   uint32_t spins = 0;
>> -   int64_t start = 0;
>>
>>     /* wtf, someone is waiting on a fence in flush_notify handler? */
>>     assert(fence->state != NOUVEAU_FENCE_STATE_EMITTING);
>> @@ -208,12 +186,25 @@ nouveau_fence_wait(struct nouveau_fence *fence,
>> struct pipe_debug_callback *debu
>>     if (fence == screen->fence.current)
>>        nouveau_fence_next(screen);
>>
>> +   nouveau_fence_update(screen, false);
>> +
>> +   return true;
>> +}
>> +
>> +bool
>> +nouveau_fence_wait(struct nouveau_fence *fence, struct
>> pipe_debug_callback *debug)
>> +{
>> +   struct nouveau_screen *screen = fence->screen;
>> +   uint32_t spins = 0;
>> +   int64_t start = 0;
>> +
>>     if (debug && debug->debug_message)
>>        start = os_time_get_nano();
>>
>> -   do {
>> -      nouveau_fence_update(screen, false);
>> +   if (!nouveau_fence_kick(fence))
>> +      return false;
>>
>> +   do {
>>        if (fence->state == NOUVEAU_FENCE_STATE_SIGNALLED) {
>>           if (debug && debug->debug_message)
>>              pipe_debug_message(debug, PERF_INFO,
>> @@ -228,6 +219,8 @@ nouveau_fence_wait(struct nouveau_fence *fence,
>> struct pipe_debug_callback *debu
>>        if (!(spins % 8)) /* donate a few cycles */
>>           sched_yield();
>>  #endif
>> +
>> +      nouveau_fence_update(screen, false);
>>     } while (spins < NOUVEAU_FENCE_MAX_SPINS);
>>
>>     debug_printf("Wait on fence %u (ack = %u, next = %u) timed out
>> !\n",
>> @@ -259,3 +252,26 @@ nouveau_fence_unref_bo(void *data)
>>
>>     nouveau_bo_ref(NULL, &bo);
>>  }
>> +
>> +bool
>> +nouveau_fence_work(struct nouveau_fence *fence,
>> +                   void (*func)(void *), void *data)
>> +{
>> +   struct nouveau_fence_work *work;
>> +
>> +   if (!fence || fence->state == NOUVEAU_FENCE_STATE_SIGNALLED) {
>> +      func(data);
>> +      return true;
>> +   }
>> +
>> +   work = CALLOC_STRUCT(nouveau_fence_work);
>> +   if (!work)
>> +      return false;
>> +   work->func = func;
>> +   work->data = data;
>> +   LIST_ADD(&work->list, &fence->work);
>> +   p_atomic_inc(&fence->work_count);
>> +   if (fence->work_count > 64)
>
> Is 64 just an arbitrary number?

Yep!

>
>> +      nouveau_fence_kick(fence);
>> +   return true;
>> +}
>> diff --git a/src/gallium/drivers/nouveau/nouveau_fence.h
>> b/src/gallium/drivers/nouveau/nouveau_fence.h
>> index 0fa9d02..f10016d 100644
>> --- a/src/gallium/drivers/nouveau/nouveau_fence.h
>> +++ b/src/gallium/drivers/nouveau/nouveau_fence.h
>> @@ -25,6 +25,7 @@ struct nouveau_fence {
>>     int state;
>>     int ref;
>>     uint32_t sequence;
>> +   uint32_t work_count;
>>     struct list_head work;
>>  };
>>