netdev - Re: [PATCH net-next v7 1/6] page_pool: frag API support for 32-bit arch with 64-bit DMA

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]

Date: Thu, 17 Aug 2023 16:57:44 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Ilias Apalodimas <ilias.apalodimas@...aro.org>
Cc: Mina Almasry <almasrymina@...gle.com>, Yunsheng Lin
 <linyunsheng@...wei.com>, davem@...emloft.net, pabeni@...hat.com,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org, Lorenzo Bianconi
 <lorenzo@...nel.org>, Alexander Duyck <alexander.duyck@...il.com>, Liang
 Chen <liangchen.linux@...il.com>, Alexander Lobakin
 <aleksander.lobakin@...el.com>, Saeed Mahameed <saeedm@...dia.com>, Leon
 Romanovsky <leon@...nel.org>, Eric Dumazet <edumazet@...gle.com>, Jesper
 Dangaard Brouer <hawk@...nel.org>
Subject: Re: [PATCH net-next v7 1/6] page_pool: frag API support for 32-bit
 arch with 64-bit DMA

On Thu, 17 Aug 2023 19:59:37 +0300 Ilias Apalodimas wrote:
> > Can we assume the DMA mapping of page pool is page aligned? We should
> > be, right?  
> 
> Yes
> 
> > That means we're storing 12 bits of 0 at the lower end.
> > So even with 32b of space we can easily store addresses for 32b+12b =>
> > 16TB of memory. "Ought to be enough" to paraphrase Bill G, and the
> > problem is only in our heads?  
> 
> Do you mean moving the pp_frag_count there? 

Right, IIUC we don't have enough space to fit dma_addr_t and the
refcount, but if we store the dma addr on a shifted u32 instead 
of using dma_addr_t explicitly - the refcount should fit?

> I was questioning the need to have PP_FLAG_PAGE_SPLIT_IN_DRIVER
> overall.  With Yunshengs patches such a platform would allocate a
> page, so why should we prevent it from splitting it internally?

Splitting it is fine, the problem is that the refcount AKA
page->pp_frag_count** counts outstanding PP-aware references
and page->refcount counts PP-unaware references.

If we want to use page->refcount directly we'd need to unmap 
the page whenever drivers calls page_pool_defrag_page().
But the driver may assume the page is still mapped afterwards.

We can change the API to make this behavior explicit. Although
IMHO that's putting the burden of rare platforms on non-rare
platforms which we should avoid.

** I said it before and I will keep saying this until someone gets 
   angry at me - I really think we should rename this field because
   the association with frags is a coincidence.