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

2018-09-10 Thread Chris Wilson
Quoting Mika Kuoppala (2018-09-10 14:14:56)
> Chris Wilson  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 
> > ---
> >  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
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2018-09-10 Thread Mika Kuoppala
Chris Wilson  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 
> ---
>  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?

Reviewed-by: Mika Kuoppala 

> + bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);
> + }
>   if (!bo)
>   return;
>  
> @@ -1382,7 +1387,8 @@ static void request_record_user_bo(struct i915_request 
> *request,
>   bo[count] = i915_error_object_create(request->i915, c->vma);
>   if (!bo[count])
>   break;
> - count++;
> + if (++count == max)
> + break;
>   }
>  
>   ee->user_bo = bo;
> -- 
> 2.19.0.rc2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2018-09-07 Thread Chris Wilson
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 
---
 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);
+   if (!bo) {
+   /* If we can't capture everything, try to capture something. */
+   max = min_t(long, max, PAGE_SIZE / sizeof(*bo));
+   bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);
+   }
if (!bo)
return;
 
@@ -1382,7 +1387,8 @@ static void request_record_user_bo(struct i915_request 
*request,
bo[count] = i915_error_object_create(request->i915, c->vma);
if (!bo[count])
break;
-   count++;
+   if (++count == max)
+   break;
}
 
ee->user_bo = bo;
-- 
2.19.0.rc2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx