[Intel-gfx] [PATCH 3/6] drm/i915: Limit number of capture objects

[Chris Wilson]

Quoting Mika Kuoppala (2018-09-10 14:14:56)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > If we fail to allocate an array for a large number of user requested
> > capture objects, reduce the array size and try to grab at least some of
> > the objects!
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gpu_error.c | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index f7f2aa71d8d9..2835cacd0d08 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -1365,15 +1365,20 @@ static void request_record_user_bo(struct i915_request *request,
> >  {
> >       struct i915_capture_list *c;
> >       struct drm_i915_error_object **bo;
> > -     long count;
> > +     long count, max;
> >  
> > -     count = 0;
> > +     max = 0;
> >       for (c = request->capture_list; c; c = c->next)
> > -             count++;
> > +             max++;
> > +     if (!max)
> > +             return;
> >  
> > -     bo = NULL;
> > -     if (count)
> > -             bo = kcalloc(count, sizeof(*bo), GFP_ATOMIC);
> > +     bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);
> 
> There seems to be no need to zero the array.
> 
> > +     if (!bo) {
> > +             /* If we can't capture everything, try to capture something. */
> > +             max = min_t(long, max, PAGE_SIZE / sizeof(*bo));
> 
> Perhaps there is some bookkeeping in slab so this would spill to 2
> pages eventually?

Perhaps, my understanding is that kmalloc < 8192 is efficient. It was
more about defining an arbitrary number to try in case the user is
asking for several million captures like the igt did.

We could say halve the number until it allocates, but then we'll
probably just fail latter in capturing pages.

So a page worth of pointers seemed like as good a number as any to use.
We could just capture 1 user buffer, so long as it was the right one ;)
0 just meant the test was a flop (didn't actually test anything other
than the capture malloc failure). What we really need on top of this is
to record when we didn't capture everything :(
-Chris