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

[Sean Paul]

On Tue, Dec 20, 2016 at 4:44 AM, Jani Nikula
<jani.nikula at linux.intel.com> wrote:
> 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.
>

I'll echo Laurent's concerns here, seems like the goal is easy to
merge, hard to change. I think the problem with this is that easy to
merge usually leads to designs which need to change :)

Secondly, I'm not sure there's value outside of i915, perhaps I'm
missing use cases for other drivers.

Sean


>
> 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
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Sean Paul, Software Engineer, Google / Chromium OS