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

[Yunsheng Lin <linyunsheng@huawei.com>]

On 2025/3/27 12:59, Mina Almasry wrote:
> On Wed, Mar 26, 2025 at 8:54 PM Yunsheng Lin <linyunsheng@huawei.com> wrote:
>>>>
>>>> Since all the tracking added in this patch is performed on DMA
>>>> map/unmap, no additional code is needed in the fast path, meaning the
>>>> performance overhead of this tracking is negligible there. A
>>>> micro-benchmark shows that the total overhead of the tracking itself is
>>>> about 400 ns (39 cycles(tsc) 395.218 ns; sum for both map and unmap[2]).
>>>> Since this cost is only paid on DMA map and unmap, it seems like an
>>>> acceptable cost to fix the late unmap issue. Further optimisation can
>>>> narrow the cases where this cost is paid (for instance by eliding the
>>>> tracking when DMA map/unmap is a no-op).

See the above statement, and note the above optimisation was also discussed
before and it seemed unfeasible too.

> 
> what time_bench_page_pool03_slow actually does each iteration:
> - Allocates a page *from the fast path*
> - Frees a page to through the slow path (recycling disabled).
> 
> Notably it doesn't do anything in the slow path that I imagine is
> actually expensive: alloc_page, dma_map_page, & dma_unmap_page.

As above, for most arches, the DMA map/unmap seems to be almost no-op when
page_pool is created with PP_FLAG_DMA_MAP flag without IOMMU/swiotlb behind
the DMA MAPPING.

> 
> We do not have an existing benchmark case that actually tests the full
> cost of the slow path (i.e full cost of page_pool_alloc from slow path
> with dma-mapping and page_pool_put_page to the slow path with
> dma-unmapping). That test case would have given us the full picture in
> terms of % regression.
> 
> This is partly why I want to upstream the benchmark. Such cases can be
> added after it is upstreamed.

Why not add it now when you seemed to be arguing that exercising the code
path of dma_map_page() and dma_unmap_page() may change the full picture
here.