Re: [PATCH v4] dma-buf: Add a capabilities directory

[Simon Ser <contact@emersion.fr>]

On Thursday, June 2nd, 2022 at 08:25, Greg KH <greg@kroah.com> wrote:

> On Thu, Jun 02, 2022 at 06:17:31AM +0000, Simon Ser wrote:
>
> > On Thursday, June 2nd, 2022 at 07:40, Greg KH greg@kroah.com wrote:
> >
> > > On Wed, Jun 01, 2022 at 04:13:14PM +0000, Simon Ser wrote:
> > >
> > > > To discover support for new DMA-BUF IOCTLs, user-space has no
> > > > choice but to try to perform the IOCTL on an existing DMA-BUF.
> > >
> > > Which is correct and how all kernel features work (sorry I missed the
> > > main goal of this patch earlier and focused only on the sysfs stuff).
> > >
> > > > However, user-space may want to figure out whether or not the
> > > > IOCTL is available before it has a DMA-BUF at hand, e.g. at
> > > > initialization time in a Wayland compositor.
> > >
> > > Why not just do the ioctl in a test way? That's how we determine kernel
> > > features, we do not poke around in sysfs to determine what is, or is
> > > not, present at runtime.
> > >
> > > > Add a /sys/kernel/dmabuf/caps directory which allows the DMA-BUF
> > > > subsystem to advertise supported features. Add a
> > > > sync_file_import_export entry which indicates that importing and
> > > > exporting sync_files from/to DMA-BUFs is supported.
> > >
> > > No, sorry, this is not a sustainable thing to do for all kernel features
> > > over time. Please just do the ioctl and go from there. sysfs is not
> > > for advertising what is and is not enabled/present in a kernel with
> > > regards to functionality or capabilities of the system.
> > >
> > > If sysfs were to export this type of thing, it would have to do it for
> > > everything, not just some random tiny thing of one kernel driver.
> >
> > I'd argue that DMA-BUF is a special case here.
>
> So this is special and unique just like everything else? :)
>
> > To check whether the import/export IOCTLs are available, user-space
> > needs a DMA-BUF to try to perform the IOCTL. To get a DMA-BUF,
> > user-space needs to enumerate GPUs, pick one at random, load GBM or
> > Vulkan, use that heavy-weight API to allocate a "fake" buffer on the
> > GPU, export that buffer into a DMA-BUF, try the IOCTL, then teardown
> > all of this. There is no other way.
> >
> > This sounds like a roundabout way to answer the simple question "is the
> > IOCTL available?". Do you have another suggestion to address this
> > problem?
>
> What does userspace do differently if the ioctl is present or not?

Globally enable a synchronization API for Wayland clients, for instance
in the case of a Wayland compositor.

> And why is this somehow more special than of the tens of thousands of
> other ioctl calls where you have to do exactly the same thing you list
> above to determine if it is present or not?

For other IOCTLs it's not as complicated to obtain a FD to do the test
with.

> And how have you specifically tied this sysfs to the ioctl so that if it
> changes or is ported elsewhere, that sysfs attribute will also know to
> be added?

What do you mean by "ported elsewhere"?

> You already have shipping kernels today without this attribute, you
> can't go back in time and add the attribute to those kernels just to
> reflect the ioctl being present or not, so you have to handle this case
> in userspace today, making this not needed at all. Do you want to have
> two test cases in your userspace code, one that does "is the sysfs file
> there? No, ok, let's see if we are on an older kernel without it, yet
> the ioctl is present..." When really you can just do "let's see if the
> ioctl is present" logic as you already do that today.

The IOCTL has not been shipped yet.