Re: [Intel-gfx] [PATCH] drm/i915/gem: Atomically invalidate userptr on mmu-notifier

2023-12-19 Thread Andi Shyti
Hi Jonathan,

On Mon, Dec 18, 2023 at 06:33:44PM +, Cavitt, Jonathan wrote:
...
> > On Tue, Nov 28, 2023 at 08:25:05AM -0800, Jonathan Cavitt wrote:
> > > Never block for outstanding work on userptr object upon receipt of a
> > > mmu-notifier. The reason we originally did so was to immediately unbind
> > > the userptr and unpin its pages, but since that has been dropped in
> > > commit b4b9731b02c3c ("drm/i915: Simplify userptr locking"), we never
> > > return the pages to the system i.e. never drop our page->mapcount and so
> > > do not allow the page and CPU PTE to be revoked. Based on this history,
> > > we know we are safe to drop the wait entirely.
> > > 
> > > Upon return from mmu-notifier, we will still have the userptr pages
> > > pinned preventing the following PTE operation (such as try_to_unmap)
> > > adjusting the vm_area_struct, so it is safe to keep the pages around for
> > > as long as we still have i/o pending.
> > > 
> > > We do not have any means currently to asynchronously revalidate the
> > > userptr pages, that is always prior to next use.
> > > 
> > > Signed-off-by: Chris Wilson 
> > > Signed-off-by: Jonathan Cavitt 
> > 
> > Reviewed-by: Andi Shyti 
> 
> Thank you for the review!  I don't think I have permission to push this 
> upstream,
> though, so you or someone else will have to complete the push.

pushed in drm-intel-gt-next.

Thanks and sorry for the late merge.

Andi


Re: [Intel-gfx] [PATCH] drm/i915/gem: Atomically invalidate userptr on mmu-notifier

2023-12-18 Thread Andi Shyti
Hi Jonathan,

On Tue, Nov 28, 2023 at 08:25:05AM -0800, Jonathan Cavitt wrote:
> Never block for outstanding work on userptr object upon receipt of a
> mmu-notifier. The reason we originally did so was to immediately unbind
> the userptr and unpin its pages, but since that has been dropped in
> commit b4b9731b02c3c ("drm/i915: Simplify userptr locking"), we never
> return the pages to the system i.e. never drop our page->mapcount and so
> do not allow the page and CPU PTE to be revoked. Based on this history,
> we know we are safe to drop the wait entirely.
> 
> Upon return from mmu-notifier, we will still have the userptr pages
> pinned preventing the following PTE operation (such as try_to_unmap)
> adjusting the vm_area_struct, so it is safe to keep the pages around for
> as long as we still have i/o pending.
> 
> We do not have any means currently to asynchronously revalidate the
> userptr pages, that is always prior to next use.
> 
> Signed-off-by: Chris Wilson 
> Signed-off-by: Jonathan Cavitt 

Reviewed-by: Andi Shyti 

Thanks,
Andi


RE: [Intel-gfx] [PATCH] drm/i915/gem: Atomically invalidate userptr on mmu-notifier

2023-12-18 Thread Cavitt, Jonathan
-Original Message-
From: Andi Shyti  
Sent: Monday, December 18, 2023 8:06 AM
To: Cavitt, Jonathan 
Cc: intel-gfx@lists.freedesktop.org; Gupta, saurabhg 
; chris.p.wil...@linux.intel.com
Subject: Re: [Intel-gfx] [PATCH] drm/i915/gem: Atomically invalidate userptr on 
mmu-notifier
> 
> Hi Jonathan,
> 
> On Tue, Nov 28, 2023 at 08:25:05AM -0800, Jonathan Cavitt wrote:
> > Never block for outstanding work on userptr object upon receipt of a
> > mmu-notifier. The reason we originally did so was to immediately unbind
> > the userptr and unpin its pages, but since that has been dropped in
> > commit b4b9731b02c3c ("drm/i915: Simplify userptr locking"), we never
> > return the pages to the system i.e. never drop our page->mapcount and so
> > do not allow the page and CPU PTE to be revoked. Based on this history,
> > we know we are safe to drop the wait entirely.
> > 
> > Upon return from mmu-notifier, we will still have the userptr pages
> > pinned preventing the following PTE operation (such as try_to_unmap)
> > adjusting the vm_area_struct, so it is safe to keep the pages around for
> > as long as we still have i/o pending.
> > 
> > We do not have any means currently to asynchronously revalidate the
> > userptr pages, that is always prior to next use.
> > 
> > Signed-off-by: Chris Wilson 
> > Signed-off-by: Jonathan Cavitt 
> 
> Reviewed-by: Andi Shyti 


Thank you for the review!  I don't think I have permission to push this 
upstream,
though, so you or someone else will have to complete the push.
-Jonathan Cavitt


> 
> Thanks,
> Andi
>