linux-kernel - Re: [Intel-gfx] [PATCH 02/19] dma-buf-map: Add helper to initialize second map

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3c6a9126-bf88-0f29-425f-36748271c179@amd.com>
Date:   Thu, 27 Jan 2022 09:55:05 +0100
From:   Christian König <christian.koenig@....com>
To:     Lucas De Marchi <lucas.demarchi@...el.com>
Cc:     linaro-mm-sig@...ts.linaro.org, intel-gfx@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-media@...r.kernel.org
Subject: Re: [Intel-gfx] [PATCH 02/19] dma-buf-map: Add helper to initialize
 second map

Am 27.01.22 um 09:18 schrieb Lucas De Marchi:
> On Thu, Jan 27, 2022 at 09:02:54AM +0100, Christian König wrote:
>> Am 27.01.22 um 08:57 schrieb Lucas De Marchi:
>>> On Thu, Jan 27, 2022 at 08:27:11AM +0100, Christian König wrote:
>>>> Am 26.01.22 um 21:36 schrieb Lucas De Marchi:
>>>>> [SNIP]
>>> humn... not sure if I was  clear. There is no importer and exporter 
>>> here.
>>
>> Yeah, and exactly that's what I'm pointing out as problem here.
>>
>> You are using the inter driver framework for something internal to 
>> the driver. That is an absolutely clear NAK!
>>
>> We could discuss that, but you guys are just sending around patches 
>> to do this without any consensus that this is a good idea.
>
> s/you guys/you/ if you have to blame anyone - I'm the only s-o-b in
> these patches. I'm sending these to _build consensus_ on what may be a 
> good
> use for it showing a real problem it's helping to fix.

Well a cover letter would have been helpful, my impression was that you 
have a larger set and just want to upstream some minor DMA-buf changes 
necessary for it.

Now I know why people are bugging me all the time to add cover letters 
to add more context to my sets.

>
> From its documentation:
>
>  * The type :c:type:`struct dma_buf_map <dma_buf_map>` and its helpers 
> are
>  * actually independent from the dma-buf infrastructure. When sharing 
> buffers
>  * among devices, drivers have to know the location of the memory to 
> access
>  * the buffers in a safe way. :c:type:`struct dma_buf_map <dma_buf_map>`
>  * solves this problem for dma-buf and its users. If other drivers or
>  * sub-systems require similar functionality, the type could be 
> generalized
>  * and moved to a more prominent header file.
>
> if there is no consensus and a better alternative, I'm perfectly fine in
> throwing it out and using the better approach.

When Thomas Zimmermann upstreamed the dma_buf_map work we had a 
discussion if that shouldn't be independent of the DMA-buf framework.

The consensus was that as soon as we have more widely use for it this 
should be made independent. So basically that is what's happening now.

I suggest the following approach:
1. Find a funky name for this, something like iomem_, kiomap_ or similar.
2. Separate this from all you driver dependent work and move the 
dma_buf_map structure out of DMA-buf into this new whatever_ prefix.
3. Ping Thomas, LKML, me and probably a couple of other core people if 
this is the right idea or not.
4. Work on dropping the map parameter from dma_buf_vunmap(). This is 
basically why we can't modify the pointers returned from dma_buf_vmap() 
and has already cause a few problems with dma_buf_map_incr().

Regards,
Christian.

>
> Lucas De Marchi