Re: [RFC PATCH 5/7] drm/ttm: add range busy check for range manager

[Robert Beckett]

On 16/03/2022 09:54, Christian König wrote:
> Am 15.03.22 um 19:04 schrieb Robert Beckett:
> > RFC: do we want this to become a generic interface in
> > ttm_resource_manager_func?
> >
> > RFC: would we prefer a different interface? e.g.
> > for_each_resource_in_range or for_each_bo_in_range
>
> Well completely NAK to that. Why do you need that?
>
> The long term goal is to completely remove the range checks from TTM 
> instead.

ah, I did not know that.
I wanted it just to enable parity with a selftest that checks whether a 
range is allocated before initializing a given range with test data 
behind the allocator's back. It needs to check the range so that it 
doesn't destroy in use data.

I suppose we could add another drm_mm range tracker just for testing and 
shadow track each allocation in the range, but that seemed like a lot of 
extra infrastructure for no general runtime use.

would you mind explaining the rationale for removing range checks? It 
seems to me like a natural fit for a memory manager

> Regards,
> Christian.
>
> > Signed-off-by: Robert Beckett <bob.beckett@xxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/ttm/ttm_range_manager.c | 21 +++++++++++++++++++++
> >   include/drm/ttm/ttm_range_manager.h     |  3 +++
> >   2 files changed, 24 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/ttm/ttm_range_manager.c 
> > b/drivers/gpu/drm/ttm/ttm_range_manager.c
> > index 8cd4f3fb9f79..5662627bb933 100644
> > --- a/drivers/gpu/drm/ttm/ttm_range_manager.c
> > +++ b/drivers/gpu/drm/ttm/ttm_range_manager.c
> > @@ -206,3 +206,24 @@ int ttm_range_man_fini_nocheck(struct ttm_device 
> > *bdev,
> >       return 0;
> >   }
> >   EXPORT_SYMBOL(ttm_range_man_fini_nocheck);
> > +
> > +/**
> > + * ttm_range_man_range_busy - Check whether anything is allocated 
> > with a range
> > + *
> > + * @man: memory manager to check
> > + * @fpfn: first page number to check
> > + * @lpfn: last page number to check
> > + *
> > + * Return: true if anything allocated within the range, false otherwise.
> > + */
> > +bool ttm_range_man_range_busy(struct ttm_resource_manager *man,
> > +                  unsigned fpfn, unsigned lpfn)
> > +{
> > +    struct ttm_range_manager *rman = to_range_manager(man);
> > +    struct drm_mm *mm = &rman->mm;
> > +
> > +    if (__drm_mm_interval_first(mm, PFN_PHYS(fpfn), PFN_PHYS(lpfn + 
> > 1) - 1))
> > +        return true;
> > +    return false;
> > +}
> > +EXPORT_SYMBOL(ttm_range_man_range_busy);
> > diff --git a/include/drm/ttm/ttm_range_manager.h 
> > b/include/drm/ttm/ttm_range_manager.h
> > index 7963b957e9ef..86794a3f9101 100644
> > --- a/include/drm/ttm/ttm_range_manager.h
> > +++ b/include/drm/ttm/ttm_range_manager.h
> > @@ -53,4 +53,7 @@ static __always_inline int ttm_range_man_fini(struct 
> > ttm_device *bdev,
> >       BUILD_BUG_ON(__builtin_constant_p(type) && type >= 
> > TTM_NUM_MEM_TYPES);
> >       return ttm_range_man_fini_nocheck(bdev, type);
> >   }
> > +
> > +bool ttm_range_man_range_busy(struct ttm_resource_manager *man,
> > +                  unsigned fpfn, unsigned lpfn);
> >   #endif