| Message ID | 20180907112856.28242-3-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/6] drm/i915: Missed interrupt simulation is no more, tell the world | expand |
Chris Wilson <chris@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@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? Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > + 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
Quoting Mika Kuoppala (2018-09-10 14:14:56) > Chris Wilson <chris@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@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
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;
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@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gpu_error.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)