Re: [Intel-gfx] [RFC 4/6] drm: Add simple fdinfo memory helpers

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 18/04/2023 18:18, Rob Clark wrote:
> On Mon, Apr 17, 2023 at 8:56 AM Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>>
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> For drivers who only wish to show one memory region called 'system,
>> and only account the GEM buffer object handles under it.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> ---
>>   drivers/gpu/drm/drm_file.c | 45 ++++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_file.h     |  6 +++++
>>   2 files changed, 51 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
>> index e202f79e816d..1e70669dddf7 100644
>> --- a/drivers/gpu/drm/drm_file.c
>> +++ b/drivers/gpu/drm/drm_file.c
>> @@ -872,6 +872,51 @@ void drm_send_event(struct drm_device *dev, struct drm_pending_event *e)
>>   }
>>   EXPORT_SYMBOL(drm_send_event);
>>
>> +static void
>> +add_obj(struct drm_gem_object *obj, struct drm_fdinfo_memory_stat *stats)
>> +{
>> +       u64 sz = obj->size;
>> +
>> +       stats[0].size += sz;
>> +
>> +       if (obj->handle_count > 1)
>> +               stats[0].shared += sz;
>> +
>> +       if (!dma_resv_test_signaled(obj->resv, dma_resv_usage_rw(true)))
>> +               stats[0].active += sz;
>> +
>> +       /* Not supported. */
>> +       stats[0].resident = ~0ull;
>> +       stats[0].purgeable = ~0ull;
> 
> Hmm, this kinda makes the simple helper not very useful.  In my
> version, you get something that is useful for all UMA drivers (which
> all, IIRC, have some form of madv ioctl).  I was kinda imagining that
> for ttm drivers, my print_memory_stats() would just become a helper
> and that TTM (or "multi-region") drivers would have their own thing.

Hm how? Your version also needed a driver specific vfunc:

+static enum drm_gem_object_status msm_gem_status(struct drm_gem_object *obj)
+{
+	struct msm_gem_object *msm_obj = to_msm_bo(obj);
+	enum drm_gem_object_status status = 0;
+
+	if (msm_obj->pages)
+		status |= DRM_GEM_OBJECT_RESIDENT;
+
+	if (msm_obj->madv == MSM_MADV_DONTNEED)
+		status |= DRM_GEM_OBJECT_PURGEABLE;
+
+	return status;
+}

Regards,

Tvrtko

> 
> BR,
> -R
> 
>> +}
>> +
>> +char **
>> +drm_query_fdinfo_system_region(struct drm_device *dev, unsigned int *num)
>> +{
>> +       static char *region[] = {
>> +               "system",
>> +       };
>> +
>> +       *num = 1;
>> +
>> +       return region;
>> +}
>> +EXPORT_SYMBOL(drm_query_fdinfo_system_region);
>> +
>> +void
>> +drm_query_fdinfo_system_memory(struct drm_file *file,
>> +                              struct drm_fdinfo_memory_stat *stats)
>> +{
>> +       struct drm_gem_object *obj;
>> +       int id;
>> +
>> +       spin_lock(&file->table_lock);
>> +       idr_for_each_entry(&file->object_idr, obj, id)
>> +               add_obj(obj, stats);
>> +       spin_unlock(&file->table_lock);
>> +}
>> +EXPORT_SYMBOL(drm_query_fdinfo_system_memory);
>> +
>>   static void
>>   print_stat(struct drm_printer *p, const char *stat, const char *region, u64 sz)
>>   {
>> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
>> index 00d48beeac5c..dd7c6fb2c975 100644
>> --- a/include/drm/drm_file.h
>> +++ b/include/drm/drm_file.h
>> @@ -383,6 +383,12 @@ struct drm_fdinfo_memory_stat {
>>          u64 active;
>>   };
>>
>> +char **drm_query_fdinfo_system_region(struct drm_device *dev,
>> +                                     unsigned int *num);
>> +void drm_query_fdinfo_system_memory(struct drm_file *file,
>> +                                   struct drm_fdinfo_memory_stat *stats);
>> +
>> +
>>   /**
>>    * drm_is_primary_client - is this an open file of the primary node
>>    * @file_priv: DRM file
>> --
>> 2.37.2
>>