[Intel-gfx] [RFC 0/4] Introduce drmfs pseudo filesystem for drm subsystem

[Jani Nikula]

On Tue, 20 Dec 2016, Laurent Pinchart <laurent.pinchart at ideasonboard.com> wrote:
> Hi Swati,
>
> On Monday 19 Dec 2016 16:12:22 swati.dhingra at intel.com wrote:
>> From: Swati Dhingra <swati.dhingra at intel.com>
>> 
>> Currently, we don't have a stable ABI which can be used for the purpose of
>> providing output debug/loggging/crc and other such data from DRM.
>> The ABI in current use (filesystems, ioctls, et al.) have their own
>> constraints and are intended to output a particular type of data.
>> Few cases in point:
>> sysfs	- stable ABI, but constrained to one textual value per file
>> debugfs	- unstable ABI, free-for-all
>> ioctls	- not as suitable to many single purpose continuous data
>> 	  dumping, we would very quickly run out ioctl space; requires more
>> 	  userspace support than "cat"
>> device nodes -  a real possibilty, kernel instantiation is more tricky,
>> 		requires udev (+udev.rules) or userspace discovery of the
>> 		dynamic major:minor (via sysfs) [mounting a registered
>> 		filesystem is easy in comparison]
>> netlink	- stream based, therefore involves numerous copies.
>> 
>> Debugfs is the lesser among the evils here, thereby we have grown used to
>> the convenience and flexibility in presentation that debugfs gives us
>> (including relayfs inodes) that we want some of that hierachy in stable user
>> ABI form.
>
> Seriously, why ? A subsystem growing its own file system sounds so wrong. It 
> seems that you want to have all the benefits of a stable ABI without going 
> through the standardization effort that this requires. I can see so many ways 
> that drmfs could be abused, with drivers throwing in new data with little or 
> no review. You'll need very compelling arguments to convince me.

This is not unlike my sentiments on the first version posted
[1]. There's also the distinct feeling of [2]. Suffice it to say at this
time that I'm dubious, not convinced enough to defend this.

Swati, please refrain from posting new versions of the patches until
there's some consensus one way or the other; it's counter-productive to
keep splitting off the discussion into several patch series threads at
this stage. Let's have the discussion here.


BR,
Jani.


[1] http://mid.mail-archive.com/87lgw0xcf4.fsf@intel.com
[2] https://xkcd.com/927/

>
>> Due to these limitations, there is a need for a new pseudo filesytem, that
>> would act as a stable 'free-for-all' ABI, with the heirarchial structure and
>> thus convenience of debugfs. This will be managed by drm, thus named
>> 'drmfs'. DRM would register this filesystem to manage a canonical
>> mountpoint, but this wouldn't limit everyone to only using that pseudofs
>> underneath.
>> 
>> This can serve to hold various kinds of output data from Linux DRM
>> subsystems, for the files which can't truely fit anywhere else with
>> existing ABI's but present so, for the lack of a better place.
>> 
>> In this patch series, we have introduced a pseudo filesystem named as
>> 'drmfs' for now. The filesystem is introduced in the first patch, and the
>> subsequent patches make use of the filesystem interfaces, in drm driver,
>> and making them available for use by the drm subsystem components, one of
>> which is i915. We've moved the location of i915 GuC logs from debugfs to
>> drmfs in the last patch. Subsequently, more such files such as pipe_crc,
>> error states, memory stats, etc. can be move to this filesystem, if the
>> idea introduced here is acceptable per se. The filesystem introduced is
>> being used to house the data generated by i915 driver in this patch series,
>> but will hopefully be generic enough to provide scope for usage by any
>> other drm subsystem component.
>> 
>> The patch series is being floated as RFC to gather feedback on the idea and
>> infrastructure proposed here and it's suitability to address the specific
>> problem statement/use case.
>> 
>> v2: fix the bat failures caused due to missing config check
>> 
>> v3: Changes made:
>>     - Move the location of drmfs from fs/ to drivers/gpu/drm/ (Chris)
>>     - Moving config checks to header (Chris,Daniel)
>> 
>> v4: Added the kernel Documentaion (using Sphinx).
>> 
>> Sourab Gupta (4):
>>   drm: Introduce drmfs pseudo filesystem interfaces
>>   drm: Register drmfs filesystem from drm init
>>   drm: Create driver specific root directory inside drmfs
>>   drm/i915: Creating guc log file in drmfs instead of debugfs
>> 
>>  Documentation/gpu/drm-uapi.rst             |  76 ++++
>>  drivers/gpu/drm/Kconfig                    |   9 +
>>  drivers/gpu/drm/Makefile                   |   1 +
>>  drivers/gpu/drm/drm_drv.c                  |  26 ++
>>  drivers/gpu/drm/drmfs.c                    | 566 ++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/i915_guc_submission.c |  33 +-
>>  include/drm/drm_drv.h                      |   3 +
>>  include/drm/drmfs.h                        |  77 ++++
>>  include/uapi/linux/magic.h                 |   3 +
>>  9 files changed, 773 insertions(+), 21 deletions(-)
>>  create mode 100644 drivers/gpu/drm/drmfs.c
>>  create mode 100644 include/drm/drmfs.h

-- 
Jani Nikula, Intel Open Source Technology Center