Re: [PATCH v2] dma-buf: Move sysfs work out of DMA-BUF export path

From: Christian König
Date: Wed May 18 2022 - 08:03:47 EST


Am 18.05.22 um 01:09 schrieb T.J. Mercier:
[SNIP]
Perhaps we should go just one step further and make a misc device node
for dmabug debugging information to be in and just have userspace
poll/read on the device node and we spit the info that used to be in
debugfs out through that? That way this only affects systems when they
want to read the information and not normal code paths? Yeah that's a
hack, but this whole thing feels overly complex now.
Yeah, totally agree on the complexity note. I'm just absolutely not keen
to add hack over hack over hack to make something work which from my
point of view has some serious issues with it's design.

Why is this patch a hack? We found a problem with the initial design
which nobody saw when it was originally created, and now we're trying
to address it within the constraints that exist.

Well the issue is that you don't try to tackle the underlying problem, but rather just mitigate the unforeseen consequences. That is pretty clearly a hack to me.

Is there some other
solution to the problem of exports getting blocked that you would
suggest here?

Well pretty much the same as Greg outlined as well. Go back to your drawing board and come back with a solution which does not need such workarounds.

Alternatively you can give me a full overview of the design and explain why exactly that interface here is necessary in exactly that form.

For example trying to do accounting based on DMA-bufs is extremely
questionable to begin with. See a modern game for example can have
between 10k and 100k of different buffers, reserving one file descriptor
for each of those objects is absolutely not going to work.

So my request is to please describe your full use case and not just why
you think this patch is justified.

The use case was described in the commit message when the feature was
initially added (after discussion about it on the list) including
links to code that uses the feature:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fall%2F20210603214758.2955251-1-hridya%40google.com%2F&data=05%7C01%7Cchristian.koenig%40amd.com%7C3f6e3e98fc6f45ead80d08da385a59e6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637884257979664387%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=i%2BSfpB%2B6iBolBHu6KrKH3njq0zo1SBbrKDHZOjpsIks%3D&reserved=0

Yeah and to be honest I have the strong feeling now that this was absolutely not well thought through. This description as well as the in kernel documentation are not even remotely sufficient to explain what you guys are doing with this.

So please come up with a complete design description for both userspace and kernel why this interface here is necessary inside the upstream kernel.

If you can't convince me that this is a good idea I will just bluntly mark this DMA-buf sysfs interface as deprecated.

Regards,
Christian.



Regards,
Christian.

thanks,

greg k-h