Re: [PATCH v3 01/10] file: add callback for creating long-term dmabuf maps

["Christian König" <christian.koenig@amd.com>]

On 4/30/26 20:33, Pavel Begunkov wrote:
> On 4/30/26 07:03, Christian König wrote:
>> On 4/29/26 17:25, Pavel Begunkov wrote:
>>> Introduce a new file callback that allows creating long-term dma
>>> mapping. All necessary information together with a dmabuf will be passed
>>> in the second argument of type struct io_dmabuf_token, which will be
>>> defined in following patches.
>>
>> Well first of all the naming is probably not the best. Maybe rather call that dma-buf attachment or context or mappping.
> 
> "Mapping" or "attachment" would be confusing as maps are created lazily
> together with struct io_dmabuf_map. I can name it create_dmabuf_ctx(),
> but I decided to use "token" not to collide with dmabuf terminology.
> e.g. I wouldn't be surprised to see some dmabuf ctx in the dmabuf
> implementation code. Maybe "*io_ctx" would be better.

Context or ctx sounds fine to me. IIRC we don't have a context in the DMA-buf subsystem yet.

But we do have the terminology context in other subsystems and components which build on top of DMA-buf similar to this patch set here. So I think that is a pretty good match.

> 
>> Then the patch should probably define the full interface and not just add the callback here and then the structure in a follow up patch.
> 
> I strongly prefer splitting patches so that they touch one tree at
> a time whenever possible.

Exactly that is what you should *not* do and is the background reason why I ask.

Making changes in a core header like include/linux/fs.h to add a new interface and then only later on explaining how that interface works is usually a pretty clear no-go for upstreaming.

Each patch should make one consistent change and upstream maintainers sometimes even require that you give an user for the interface in the same patch.

> tbh, I don't see much of a problem it being
> not defined as it's just forwarded in first patches, but I can shuffle
> it around in the series so that definitions come first.

That is not really a good idea either.

As far as I can see a good organization of the patches would look something like this:

1. The API between higher level and filesystem. Including all the functions, structures, enums etc.. necessary to give everybody reviewing it a solid picture of the general idea.

2. The higher level/frontend/uAPI. Again including all the stuff necessary to get a solid picture.

3. Eventually the glue code between #1 and #2. Depends on if you need it or not to understand those patches individually.

4. The backend implementation, which enables the new feature for a specific fs and/or storage device.

5. Updating Documentation/filesystems/api-summary.rst and eventually adding a new file to explain how the DMA-buf interaction with the fs layer works.

Regards,
Christian.