linux-kernel - Re: [PATCH RFC 3/8] memory-provider: dmabuf devmem memory provider

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: <8b7d25eb-1f10-3e37-8753-92b42da3fb34@huawei.com>
Date:   Wed, 15 Nov 2023 17:29:17 +0800
From:   Yunsheng Lin <linyunsheng@...wei.com>
To:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        Mina Almasry <almasrymina@...gle.com>
CC:     Jakub Kicinski <kuba@...nel.org>, <davem@...emloft.net>,
        <pabeni@...hat.com>, <netdev@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>,
        Willem de Bruijn <willemb@...gle.com>,
        Kaiyuan Zhang <kaiyuanz@...gle.com>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Christian König <christian.koenig@....com>,
        Jason Gunthorpe <jgg@...dia.com>,
        Matthew Wilcox <willy@...radead.org>,
        Linux-MM <linux-mm@...ck.org>
Subject: Re: [PATCH RFC 3/8] memory-provider: dmabuf devmem memory provider

On 2023/11/14 23:41, Willem de Bruijn wrote:
>>
>> I am not sure dma-buf maintainer's concern is still there with this patchset.
>>
>> Whatever name you calling it for the struct, however you arrange each field
>> in the struct, some metadata is always needed for dmabuf to intergrate into
>> page pool.
>>
>> If the above is true, why not utilize the 'struct page' to have more unified
>> handling?
> 
> My understanding is that there is a general preference to simplify struct
> page, and at the least not move in the other direction by overloading the
> struct in new ways.

As my understanding, the new struct is just mirroring the struct page pool
is already using, see:
https://elixir.free-electrons.com/linux/v6.7-rc1/source/include/linux/mm_types.h#L119

If there is simplifying to the struct page_pool is using, I think the new
stuct the devmem memory provider is using can adjust accordingly.

As a matter of fact, I think the way 'struct page' for devmem is decoupled
from mm subsystem may provide a way to simplify or decoupled the already
existing 'struct page' used in netstack from mm subsystem, before this
patchset, it seems we have the below types of 'struct page':
1. page allocated in the netstack using page pool.
2. page allocated in the netstack using buddy allocator.
3. page allocated in other subsystem and passed to the netstack, such as
   zcopy or spliced page?

If we can decouple 'struct page' for devmem from mm subsystem, we may be able
to decouple the above 'struct page' from mm subsystem one by one.

> 
> If using struct page for something that is not memory, there is ZONE_DEVICE.
> But using that correctly is non-trivial:
> 
> https://lore.kernel.org/all/ZKyZBbKEpmkFkpWV@ziepe.ca/ 
> 
> Since all we need is a handle that does not leave the network stack,
> a network specific struct like page_pool_iov entirely avoids this issue.

Yes, I am agree about the network specific struct.
I am wondering if we can make the struct more generic if we want to
intergrate it into page_pool and use it in net stack.

> RFC v3 seems like a good simplification over RFC v1 in that regard to me.
> I was also pleasantly surprised how minimal the change to the users of
> skb_frag_t actually proved to be.

Yes, I am agreed about that too. Maybe we can make it simpler by using
a more abstract struct as page_pool, and utilize some features of
page_pool too.

For example, from the page_pool doc, page_pool have fast cache and
ptr-ring cache as below, but if napi_frag_unref() call
page_pool_page_put_many() and return the dmabuf chunk directly to
gen_pool in the memory provider, then it seems we are bypassing the
below caches in the page_pool.

    +------------------+
    |       Driver     |
    +------------------+
            ^
            |
            |
            |
            v
    +--------------------------------------------+
    |                request memory              |
    +--------------------------------------------+
        ^                                  ^
        |                                  |
        | Pool empty                       | Pool has entries
        |                                  |
        v                                  v
    +-----------------------+     +------------------------+
    | alloc (and map) pages |     |  get page from cache   |
    +-----------------------+     +------------------------+
                                    ^                    ^
                                    |                    |
                                    | cache available    | No entries, refill
                                    |                    | from ptr-ring
                                    |                    |
                                    v                    v
                          +-----------------+     +------------------+
                          |   Fast cache    |     |  ptr-ring cache  |
                          +-----------------+     +------------------+


> 
> .
>