On Mon, Jun 06, 2022 at 04:10:09PM +0100, Robin Murphy wrote: > On 2022-06-02 07:47, Daniel Vetter wrote: > > On Thu, 2 Jun 2022 at 08:34, Simon Ser <contact@xxxxxxxxxxx> wrote: > > > > > > On Thursday, June 2nd, 2022 at 08:25, Greg KH <greg@xxxxxxxxx> 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@xxxxxxxxx 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. > > > > Two expand on this: > > > > - compositor opens the drm render /dev node > > - compositor initializes the opengl or vulkan userspace driver on top of that > > - compositor asks that userspace driver to allocate some buffer, which > > can be pretty expensive > > - compositor asks the userspace driver to export that buffer into a dma-buf > > - compositor can finally do the test ioctl, realizes support isn't > > there and tosses the entire thing > > > > read() on a sysfs file is so much more reasonable it's not even funny. > > Just a drive-by observation, so apologies if I'm overlooking something > obvious, but it sounds like the ideal compromise would be to expose a sysfs > file which behaves as a dummy exported dma-buf. That way userspace could > just open() it and try ioctl() directly - assuming that supported operations > can fail distinctly from unsupported ones, or succeed as a no-op - which > seems even simpler still. ioctl() will not work on a sysfs file, sorry.
