netdev - Re: [PATCH net-next v5 2/2] page_pool: Track DMA-mapped pages and unmap them when destroying the pool

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: <7488e6cf-e68b-4404-aaa9-f4892b2ff94b@redhat.com>
Date: Tue, 1 Apr 2025 10:56:00 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>,
 Toke Høiland-Jørgensen <toke@...hat.com>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski
 <kuba@...nel.org>, Jesper Dangaard Brouer <hawk@...nel.org>,
 Saeed Mahameed <saeedm@...dia.com>, Leon Romanovsky <leon@...nel.org>,
 Tariq Toukan <tariqt@...dia.com>, Andrew Lunn <andrew+netdev@...n.ch>,
 Eric Dumazet <edumazet@...gle.com>,
 Ilias Apalodimas <ilias.apalodimas@...aro.org>,
 Simon Horman <horms@...nel.org>, Andrew Morton <akpm@...ux-foundation.org>,
 Mina Almasry <almasrymina@...gle.com>, Yonglong Liu
 <liuyonglong@...wei.com>, Yunsheng Lin <linyunsheng@...wei.com>,
 Pavel Begunkov <asml.silence@...il.com>, Matthew Wilcox
 <willy@...radead.org>, netdev@...r.kernel.org, bpf@...r.kernel.org,
 linux-rdma@...r.kernel.org, linux-mm@...ck.org, Qiuling Ren
 <qren@...hat.com>, Yuying Ma <yuma@...hat.com>
Subject: Re: [PATCH net-next v5 2/2] page_pool: Track DMA-mapped pages and
 unmap them when destroying the pool

On 3/31/25 6:35 PM, Alexander Lobakin wrote:
> From: Toke Høiland-Jørgensen <toke@...hat.com>
> Date: Fri, 28 Mar 2025 13:19:09 +0100
> 
>> When enabling DMA mapping in page_pool, pages are kept DMA mapped until
>> they are released from the pool, to avoid the overhead of re-mapping the
>> pages every time they are used. This causes resource leaks and/or
>> crashes when there are pages still outstanding while the device is torn
>> down, because page_pool will attempt an unmap through a non-existent DMA
>> device on the subsequent page return.
> 
> [...]
> 
>> @@ -173,10 +212,10 @@ struct page_pool {
>>  	int cpuid;
>>  	u32 pages_state_hold_cnt;
>>  
>> -	bool has_init_callback:1;	/* slow::init_callback is set */
>> +	bool dma_sync;			/* Perform DMA sync for device */
> 
> Have you seen my comment under v3 (sorry but I missed that there was v4
> already)? Can't we just test the bit atomically?

My understanding is that to make such operation really atomic, we will
need to access all the other bits within the same bitfield with atomic
bit ops, leading to a significant code churn (and possibly some overhead).

I think that using a full bool field is a better option.

Thanks,

Paolo