[PATCH v5 17/20] drm/gpusvm: Introduce drm_gpusvm_range_find_or_insert_start() function

[Matthew Brost]

On Wed, Apr 30, 2025 at 09:10:33AM +0530, Ghimiray, Himal Prasad wrote:
> 
> 
> On 30-04-2025 00:05, Matthew Brost wrote:
> > On Tue, Apr 29, 2025 at 04:12:30PM +0530, Himal Prasad Ghimiray wrote:
> > > The drm_gpusvm_range_find_or_insert_start() function is used to
> > > determine the starting address of a CPU VMA within a specified user
> > > range. If the range does not contain any VMA, the function returns
> > > ULONG_MAX.
> > > 
> > > Signed-off-by: Himal Prasad Ghimiray <himal.prasad.ghimiray at intel.com>
> > > ---
> > >   drivers/gpu/drm/drm_gpusvm.c | 29 +++++++++++++++++++++++++++++
> > >   include/drm/drm_gpusvm.h     |  5 +++++
> > >   2 files changed, 34 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> > > index 40a56f38ff8e..bb7c589b8d15 100644
> > > --- a/drivers/gpu/drm/drm_gpusvm.c
> > > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > > @@ -980,6 +980,35 @@ static void drm_gpusvm_driver_lock_held(struct drm_gpusvm *gpusvm)
> > >   }
> > >   #endif
> > > +/**
> > > + * drm_gpusvm_range_find_or_insert_start() - Find or insert start address for range
> > > + * @gpusvm: Pointer to the GPU SVM structure
> > > + * @start: The inclusive start user address.
> > > + * @end: The exclusive end user address.
> > > + *
> > > + * Returns: The start address of first VMA within the provided range,
> > > + * ULONG_MAX otherwise. Assumes start_addr < end_addr.
> > > + */
> > > +unsigned long
> > > +drm_gpusvm_range_find_or_insert_start(struct drm_gpusvm *gpusvm,
> > > +				      unsigned long start,
> > > +				      unsigned long end)
> > 
> > We don't really do an insertion here, so maybe:
> > 
> > s/drm_gpusvm_range_find_or_insert_start/drm_gpusvm_find_vma_start
> 
> Sure.
> 
> > 
> > > +{
> > > +	struct mm_struct *mm = gpusvm->mm;
> > > +	struct vm_area_struct *vma;
> > > +	unsigned long addr = ULONG_MAX;
> 
> Does this return in case of no vma found makes sense to you ?
> Initially I thought of returning 0, but wasn't sure whether under any
> scenario cpu vma can start at 0 or not.
> 

Yes, I think ULONG_MAX is better than zero - mmap could produce an
address of zero but not ULONG_MAX. 

Matt

> > > +
> > 
> > Hmm, do you think we need a mmget here? I'd expect this be called from a
> > user context where we'd have a MM ref but maybe to be safe add a
> > mmget/put?
> Yup, mmget/mmput makes it more reliable. Will add it
> 
> > 
> > Matt
> > 
> > > +	mmap_read_lock(mm);
> > > +
> > > +	vma = find_vma_intersection(mm, start, end);
> > > +	if (vma)
> > > +		addr =  vma->vm_start;
> > > +
> > > +	mmap_read_unlock(mm);
> > > +	return addr;
> > > +}
> > > +EXPORT_SYMBOL_GPL(drm_gpusvm_range_find_or_insert_start);
> > > +
> > >   /**
> > >    * drm_gpusvm_range_find_or_insert() - Find or insert GPU SVM range
> > >    * @gpusvm: Pointer to the GPU SVM structure
> > > diff --git a/include/drm/drm_gpusvm.h b/include/drm/drm_gpusvm.h
> > > index cce217bc136f..b0e9ee5c6226 100644
> > > --- a/include/drm/drm_gpusvm.h
> > > +++ b/include/drm/drm_gpusvm.h
> > > @@ -315,6 +315,11 @@ void drm_gpusvm_fini(struct drm_gpusvm *gpusvm);
> > >   void drm_gpusvm_free(struct drm_gpusvm *gpusvm);
> > > +unsigned long
> > > +drm_gpusvm_range_find_or_insert_start(struct drm_gpusvm *gpusvm,
> > > +				      unsigned long start,
> > > +				      unsigned long end);
> > > +
> > >   struct drm_gpusvm_range *
> > >   drm_gpusvm_range_find_or_insert(struct drm_gpusvm *gpusvm,
> > >   				unsigned long fault_addr,
> > > -- 
> > > 2.34.1
> > > 
>