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] [day] [month] [year] [list]

Message-ID: <5cb9453b-c507-4afa-9d48-1efcfb60be46@suse.de>
Date:   Fri, 28 Jan 2022 09:34:05 +0100
From:   Thomas Zimmermann <tzimmermann@...e.de>
To:     Lucas De Marchi <lucas.demarchi@...el.com>
Cc:     intel-gfx@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, linaro-mm-sig@...ts.linaro.org,
        Christian König <christian.koenig@....com>,
        linux-media@...r.kernel.org
Subject: Re: [Intel-gfx] [PATCH 02/19] dma-buf-map: Add helper to initialize
 second map

Hi

Am 28.01.22 um 09:15 schrieb Thomas Zimmermann:
...
> 
>>
>> While with the construct below
>>
>>          other_map;
>>          ...
>>          other_map = INITIALIZER()
>>
>> I can rely on the compiler complaining about uninitialized var. And
>> in most of the cases I can just have this single line in the beggining 
>> of the
>> function when the offset is constant:
>>
>>          struct dma_buf_map other_map = INITIALIZER(bla_map, 
>> offsetof(..));
>>
>>
>> This is useful when you have several small functions in charge of
>> updating/reading inner struct members.
> 
> You won't need an extra variable or the initializer macro if you add an 
> offset parameter to dma_buf_memcpy_{from,to}.  Simple pass offsetof(..) 
> to that parameter and it will do the right thing.
> 
> It avoids the problems of the current macro and is even more flexible. 
> On top of that, you can build whatever convenience macros you need for 
> i915.

And maybe put all changes to the dma_buf_map interface into a single 
patch. It makes it easier to review and discuss.

Best regards
Thomas

> 
> Best regards
> Thomas
> 
>>
>>>
>>> I've also been very careful to distinguish between .vaddr and 
>>> .vaddr_iomem, even in places where I wouldn't have to. This macro 
>>> breaks the assumption.
>>
>> That's one reason I think if we have this macro, it should be in the
>> dma_buf_map.h header (or whatever we rename these APIs to). It's the
>> only place where we can safely add code that relies on the implementation
>> of the "private" fields in struct dma_buf_map.
>>
>> Lucas De Marchi
>>
>>>
>>> Best regards
>>> Thomas
>>>
>>>>  /**
>>>>   * dma_buf_map_set_vaddr - Sets a dma-buf mapping structure to an 
>>>> address in system memory
>>>>   * @map:    The dma-buf mapping structure
>>>
>>> -- 
>>> Thomas Zimmermann
>>> Graphics Driver Developer
>>> SUSE Software Solutions Germany GmbH
>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>> (HRB 36809, AG Nürnberg)
>>> Geschäftsführer: Ivo Totev
>>
>>
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Download attachment "OpenPGP_signature" of type "application/pgp-signature" (841 bytes)