Re: [PATCH] drm/i915: Fix mismatch between misplaced vma check and vma insert

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Sent: Tuesday, December 15, 2020 2:02 PM
> To: Tang, CQ <cq.tang@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: stable@ <vger.kernel.org stable@xxxxxxxxxxxxxxx>
> Subject: Re:  [PATCH] drm/i915: Fix mismatch between misplaced
> vma check and vma insert
> 
> Quoting Tang, CQ (2020-12-15 21:50:53)
> >
> >
> > > -----Original Message-----
> > > From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > Sent: Tuesday, December 15, 2020 12:31 PM
> > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>; Tang, CQ
> > > <cq.tang@xxxxxxxxx>; stable@xxxxxxxxxxxxxxx
> > > Subject: [PATCH] drm/i915: Fix mismatch between misplaced vma check
> > > and vma insert
> > >
> > > When inserting a VMA, we restrict the placement to the low 4G unless
> > > the caller opts into using the full range. This was done to allow
> > > usersapce the opportunity to transition slowly from a 32b address
> > > space, and to avoid breaking inherent 32b assumptions of some
> commands.
> > >
> > > However, for insert we limited ourselves to 4G-4K, but on
> > > verification we allowed the full 4G. This causes some attempts to
> > > bind a new buffer to sporadically fail with -ENOSPC, but at other times be
> bound successfully.
> > >
> > > commit 48ea1e32c39d ("drm/i915/gen9: Set PIN_ZONE_4G end to 4GB - 1
> > > page") suggests that there is a genuine problem with stateless
> > > addressing that cannot utilize the last page in 4G and so we purposefully
> excluded it.
> > >
> > > Reported-by: CQ Tang <cq.tang@xxxxxxxxx>
> > > Fixes: 48ea1e32c39d ("drm/i915/gen9: Set PIN_ZONE_4G end to 4GB - 1
> > > page")
> > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > > Cc: CQ Tang <cq.tang@xxxxxxxxx>
> > > Cc: stable@xxxxxxxxxxxxxxx
> > > ---
> > >  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > > b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > > index 193996144c84..2ff32daa50bd 100644
> > > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > > @@ -382,7 +382,7 @@ eb_vma_misplaced(const struct
> > > drm_i915_gem_exec_object2 *entry,
> > >               return true;
> > >
> > >       if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS) &&
> > > -         (vma->node.start + vma->node.size - 1) >> 32)
> > > +         (vma->node.start + vma->node.size + 4095) >> 32)
> >
> > Why 4095 not 4096?
> 
> It's the nature of the test that we need an inclusive bound.
> 
> Consider an object of size 4G - 4K, that is allowed to fit within our 32b GTT.
> 
> 	4G - 4k + 4K = 4G == 1 << 32: => vma misplaced
> 
> 	4G - 4k + 4k - 1 = 4G -1 = 0xffffffff => vma ok

Yes, the checking in i915_vma_insert() is ">" not ">="
        if (size > end) {
                DRM_DEBUG("Attempting to bind an object larger than the aperture: request=%llu > %s aperture=%llu\n",
                          size, flags & PIN_MAPPABLE ? "mappable" : "total",
                          end);
                return -ENOSPC;
        }

Reviewed-by: CQ Tang <cq.tang@xxxxxxxxx>


> 
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux