Re: [RFC PATCH v3 19/20] net: page pool: generalise ppiov dma address get

Mina Almasry
2023-12-21T19:51:26Z

On Tue, Dec 19, 2023 at 1:04 PM David Wei <dw@davidwei.uk> wrote:
>
> From: Pavel Begunkov <asml.silence@gmail.com>
>
> io_uring pp memory provider doesn't have contiguous dma addresses,
> implement page_pool_iov_dma_addr() via callbacks.
>
> Note: it might be better to stash dma address into struct page_pool_iov.
>

This is the approach already taken in v1 & RFC v5. I suspect you'd be
able to take advantage when you rebase.

> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: David Wei <dw@davidwei.uk>
> ---
> include/net/page_pool/helpers.h | 5 +----
> include/net/page_pool/types.h | 2 ++
> io_uring/zc_rx.c | 8 ++++++++
> net/core/page_pool.c | 9 +++++++++
> 4 files changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
> index aca3a52d0e22..10dba1f2aa0c 100644
> --- a/include/net/page_pool/helpers.h
> +++ b/include/net/page_pool/helpers.h
> @@ -105,10 +105,7 @@ static inline unsigned int page_pool_iov_idx(const struct page_pool_iov *ppiov)
> static inline dma_addr_t
> page_pool_iov_dma_addr(const struct page_pool_iov *ppiov)
> {
> - struct dmabuf_genpool_chunk_owner *owner = page_pool_iov_owner(ppiov);
> -
> - return owner->base_dma_addr +
> - ((dma_addr_t)page_pool_iov_idx(ppiov) << PAGE_SHIFT);
> + return ppiov->pp->mp_ops->ppiov_dma_addr(ppiov);
> }
>
> static inline unsigned long
> diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
> index f54ee759e362..1b9266835ab6 100644
> --- a/include/net/page_pool/types.h
> +++ b/include/net/page_pool/types.h
> @@ -125,6 +125,7 @@ struct page_pool_stats {
> #endif
>
> struct mem_provider;
> +struct page_pool_iov;
>
> enum pp_memory_provider_type {
> __PP_MP_NONE, /* Use system allocator directly */
> @@ -138,6 +139,7 @@ struct pp_memory_provider_ops {
> void (*scrub)(struct page_pool *pool);
> struct page *(*alloc_pages)(struct page_pool *pool, gfp_t gfp);
> bool (*release_page)(struct page_pool *pool, struct page *page);
> + dma_addr_t (*ppiov_dma_addr)(const struct page_pool_iov *ppiov);
> };
>
> extern const struct pp_memory_provider_ops dmabuf_devmem_ops;
> diff --git a/io_uring/zc_rx.c b/io_uring/zc_rx.c
> index f7d99d569885..20fb89e6bad7 100644
> --- a/io_uring/zc_rx.c
> +++ b/io_uring/zc_rx.c
> @@ -600,12 +600,20 @@ static void io_pp_zc_destroy(struct page_pool *pp)
> percpu_ref_put(&ifq->ctx->refs);
> }
>
> +static dma_addr_t io_pp_zc_ppiov_dma_addr(const struct page_pool_iov *ppiov)
> +{
> + struct io_zc_rx_buf *buf = io_iov_to_buf((struct page_pool_iov *)ppiov);
> +
> + return buf->dma;
> +}
> +
> const struct pp_memory_provider_ops io_uring_pp_zc_ops = {
> .alloc_pages = io_pp_zc_alloc_pages,
> .release_page = io_pp_zc_release_page,
> .init = io_pp_zc_init,
> .destroy = io_pp_zc_destroy,
> .scrub = io_pp_zc_scrub,
> + .ppiov_dma_addr = io_pp_zc_ppiov_dma_addr,
> };
> EXPORT_SYMBOL(io_uring_pp_zc_ops);
>
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index ebf5ff009d9d..6586631ecc2e 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -1105,10 +1105,19 @@ static bool mp_dmabuf_devmem_release_page(struct page_pool *pool,
> return true;
> }
>
> +static dma_addr_t mp_dmabuf_devmem_ppiov_dma_addr(const struct page_pool_iov *ppiov)
> +{
> + struct dmabuf_genpool_chunk_owner *owner = page_pool_iov_owner(ppiov);
> +
> + return owner->base_dma_addr +
> + ((dma_addr_t)page_pool_iov_idx(ppiov) << PAGE_SHIFT);
> +}
> +
> const struct pp_memory_provider_ops dmabuf_devmem_ops = {
> .init = mp_dmabuf_devmem_init,
> .destroy = mp_dmabuf_devmem_destroy,
> .alloc_pages = mp_dmabuf_devmem_alloc_pages,
> .release_page = mp_dmabuf_devmem_release_page,
> + .ppiov_dma_addr = mp_dmabuf_devmem_ppiov_dma_addr,
> };
> EXPORT_SYMBOL(dmabuf_devmem_ops);
> --
> 2.39.3
>

--
Thanks,
Mina