[Intel-xe] [RFC PATCH 0/2] Implement vma madvise ioctl

[Thomas Hellström]

+ Carl Zhang

On Tue, 2023-05-30 at 15:19 +0200, Thomas Hellström wrote:
> 
> On 5/24/23 22:12, Nirmoy Das wrote:
> > 
> > On 5/24/2023 8:30 PM, Matthew Brost wrote:
> > > On Wed, May 24, 2023 at 02:36:46PM +0200, Nirmoy Das wrote:
> > > > Sending this initial RFC patch series for vma madvise ioctl
> > > > to gether feedback if this the correct way to do that.
> > > > 
> > > > I am adding two expected options for userspace to pass
> > > > 
> > > > DRM_XE_VMA_MADVISE_WILLNEED:
> > > >     * Set ttm priority to normal/high(if cap permits)
> > > >     * Make sure VMAs are in allowed placement and bound.
> > > > 
> > > > DRM_XE_VMA_MADVISE_DONTNEED:
> > > >     * Set ttm priority to low so the BO belong to the vma
> > > >     become early target for eviction.
> > > >     * Make sure VMAs are not bound.
> > > > 
> > > > Questions:
> > > > Should this be part of DRM_IOCTL_XE_VM_MADVISE rather than
> > > > creating 
> > > > new ioctl?
> > > > 
> > > Def not a new IOCTL. Let's take a step back, what are you trying
> > > to
> > > implement that the current DRM_IOCTL_XE_VM_MADVISE IOCTL / VM
> > > bind IOCTL
> > > does not support?
> > 
> > AFAIU at this moment:
> > 
> > MADVISE_WILLNEED  ==  XE_VM_BIND_OP_PREFETCH +
> > DRM_XE_VM_MADVISE_PRIORITY
> > MADVISE_DONTNEED  ==  XE_VM_BIND_OP_UNMAP +
> > DRM_XE_VM_MADVISE_PRIORITY
> > 
> > So unless we need explicit madvise ioctl or vm_madvise ioctl
> > options 
> > for UMD,
> > I think we can have madvise equivalent with above vm bind and vm 
> > madvise ioctl.
> > 
> > Hi Thomas, Joonas,
> > 
> > What do you think ?
> 
> Hi, Nirmoy,
> 
> the functionality we need for DONTNEED and WILLNEED would, based on
> i915 
> and the move to vma-based would IMO be something along the lines of:
> 
> DONTNEED
> 1) if userptr, unbind (or perhaps -EINVAL)
> 2) If bo, mark the vma as dontneed. If all other vmas of the bo are 
> marked dontneed, mark the bo as dontneed and adjust its priority.
> 3) If a dontneed bo is marked for eviction, unbind its vmas, kill its
> storage and mark it as purged. Don't put on rebind list.
> 
> WILLNEED
> 1) If userptr, bind (or perhaps -EINVAL)
> 2) If bo, remove dontneed marker. If bo was purged, notify user-
> space. 
> (Need feedback from UMD whether they want an error message or just a 
> fresh backing store). Adjust its priority, put vma on rebind list.
> 
>   So in particular, if no eviction / shrinking happens between
> DONTNEED 
> and WILLNEED they will essentially be NOOPs.
> 
> It sounds like this is a bit different than what can be achieved with
> prefetch/unmap/priority.
> 
> /Thomas
> 
> > 
> > 
> > Regards,
> > 
> > Nirmoy
> > 
> > > 
> > > Matt
> > > 
> > > > Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> > > > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > > > Cc: Matthew Brost <matthew.brost at intel.com>
> > > > 
> > > > Nirmoy Das (2):
> > > >    drm/xe: Expose vma bind-unbind functions
> > > >    drm/xe: Implement madvise ioctl for vma
> > > > 
> > > >   drivers/gpu/drm/xe/Makefile         |   1 +
> > > >   drivers/gpu/drm/xe/xe_device.c      |   2 +
> > > >   drivers/gpu/drm/xe/xe_vm.c          |  52 +++----
> > > >   drivers/gpu/drm/xe/xe_vm.h          |   3 +
> > > >   drivers/gpu/drm/xe/xe_vma_madvise.c | 223 
> > > > ++++++++++++++++++++++++++++
> > > >   drivers/gpu/drm/xe/xe_vma_madvise.h |  15 ++
> > > >   include/uapi/drm/xe_drm.h           |  28 ++++
> > > >   7 files changed, 296 insertions(+), 28 deletions(-)
> > > >   create mode 100644 drivers/gpu/drm/xe/xe_vma_madvise.c
> > > >   create mode 100644 drivers/gpu/drm/xe/xe_vma_madvise.h
> > > > 
> > > > -- 
> > > > 2.39.0
> > > >