Re: [Intel-gfx] [PATCH] drm/i915/gem: Avoid iterating an empty list

2020-05-22 Thread Matthew Auld
On Fri, 22 May 2020 at 13:02, Chris Wilson  wrote:
>
> Quoting Chris Wilson (2020-05-22 11:42:07)
> > Our __sgt_iter assumes that the scattergather list has at least one
> > element. But during construction we may fail in allocating the first
> > page, and so mark the first element as the terminator. This is
> > unexpected!
> >
> > [22555.524752] RIP: 0010:shmem_get_pages+0x506/0x710 [i915]
> > [22555.524759] Code: 49 8b 2c 24 31 c0 66 89 44 24 40 48 85 ed 0f 84 62 01 
> > 00 00 4c 8b 75 00 8b 5d 08 44 8b 7d 0c 48 8b 0d 7e 34 07 e2 49 83 e6 fc 
> > <49> 8b 16 41 01 df 48 89 cf 48 89 d0 48 c1 e8 2d 48 85 c9 0f 84 c8
> > [22555.524765] RSP: 0018:c953f9d0 EFLAGS: 00010246
> > [22555.524770] RAX:  RBX:  RCX: 
> > 8881a000
> > [22555.524774] RDX: fff4 RSI:  RDI: 
> > 821efe00
> > [22555.524778] RBP: 8881b099ab00 R08:  R09: 
> > fff4
> > [22555.524782] R10: 0002 R11: ffec0a02 R12: 
> > 8881cd3c8d60
> > [22555.524786] R13: fff4 R14:  R15: 
> > 
> > [22555.524790] FS:  7f4fbeb9b9c0() GS:8881f858() 
> > knlGS:
> > [22555.524795] CS:  0010 DS:  ES:  CR0: 80050033
> > [22555.524799] CR2:  CR3: 0001ec7f0004 CR4: 
> > 001606e0
> > [22555.524803] Call Trace:
> > [22555.524919]  __i915_gem_object_get_pages+0x4f/0x60 [i915]
> >
>
> Fixes: 85d1225ec066 ("drm/i915: Introduce & use new lightweight SGL 
> iterators")
> > Signed-off-by: Chris Wilson 
> > Cc: Matthew Auld 
> > Cc: Tvrtko Ursulin 
> Cc:  # v4.8+
Reviewed-by: Matthew Auld 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/gem: Avoid iterating an empty list

2020-05-22 Thread Chris Wilson
Quoting Chris Wilson (2020-05-22 11:42:07)
> Our __sgt_iter assumes that the scattergather list has at least one
> element. But during construction we may fail in allocating the first
> page, and so mark the first element as the terminator. This is
> unexpected!
> 
> [22555.524752] RIP: 0010:shmem_get_pages+0x506/0x710 [i915]
> [22555.524759] Code: 49 8b 2c 24 31 c0 66 89 44 24 40 48 85 ed 0f 84 62 01 00 
> 00 4c 8b 75 00 8b 5d 08 44 8b 7d 0c 48 8b 0d 7e 34 07 e2 49 83 e6 fc <49> 8b 
> 16 41 01 df 48 89 cf 48 89 d0 48 c1 e8 2d 48 85 c9 0f 84 c8
> [22555.524765] RSP: 0018:c953f9d0 EFLAGS: 00010246
> [22555.524770] RAX:  RBX:  RCX: 
> 8881a000
> [22555.524774] RDX: fff4 RSI:  RDI: 
> 821efe00
> [22555.524778] RBP: 8881b099ab00 R08:  R09: 
> fff4
> [22555.524782] R10: 0002 R11: ffec0a02 R12: 
> 8881cd3c8d60
> [22555.524786] R13: fff4 R14:  R15: 
> 
> [22555.524790] FS:  7f4fbeb9b9c0() GS:8881f858() 
> knlGS:
> [22555.524795] CS:  0010 DS:  ES:  CR0: 80050033
> [22555.524799] CR2:  CR3: 0001ec7f0004 CR4: 
> 001606e0
> [22555.524803] Call Trace:
> [22555.524919]  __i915_gem_object_get_pages+0x4f/0x60 [i915]
> 

Fixes: 85d1225ec066 ("drm/i915: Introduce & use new lightweight SGL iterators")
> Signed-off-by: Chris Wilson 
> Cc: Matthew Auld 
> Cc: Tvrtko Ursulin 
Cc:  # v4.8+

> ---
>  drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c 
> b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> index 5d5d7eef3f43..7aff3514d97a 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shmem.c
> @@ -39,7 +39,6 @@ static int shmem_get_pages(struct drm_i915_gem_object *obj)
> unsigned long last_pfn = 0; /* suppress gcc warning */
> unsigned int max_segment = i915_sg_segment_size();
> unsigned int sg_page_sizes;
> -   struct pagevec pvec;
> gfp_t noreclaim;
> int ret;
>  
> @@ -192,13 +191,17 @@ static int shmem_get_pages(struct drm_i915_gem_object 
> *obj)
> sg_mark_end(sg);
>  err_pages:
> mapping_clear_unevictable(mapping);
> -   pagevec_init();
> -   for_each_sgt_page(page, sgt_iter, st) {
> -   if (!pagevec_add(, page))
> +   if (sg != st->sgl) {
> +   struct pagevec pvec;
> +
> +   pagevec_init();
> +   for_each_sgt_page(page, sgt_iter, st) {
> +   if (!pagevec_add(, page))
> +   check_release_pagevec();
> +   }
> +   if (pagevec_count())
> check_release_pagevec();
> }
> -   if (pagevec_count())
> -   check_release_pagevec();
> sg_free_table(st);
> kfree(st);
>  
> -- 
> 2.20.1
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx