| Message ID | 20241107212309.3097362-4-almasrymina@google.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | devmem TCP fixes | expand |
On 11/07, Mina Almasry wrote: > From: Samiullah Khawaja <skhawaja@google.com> > > Move the `dma_map` and `dma_sync` checks to `page_pool_init` to make > them generic. Set dma_sync to false for devmem memory provider because > the dma_sync APIs should not be used for dma_buf backed devmem memory > provider. > > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Signed-off-by: Samiullah Khawaja <skhawaja@google.com> > Signed-off-by: Mina Almasry <almasrymina@google.com> Acked-by: Stanislav Fomichev <sdf@fomichev.me>
On 11/7/24 21:23, Mina Almasry wrote: > From: Samiullah Khawaja <skhawaja@google.com> > > Move the `dma_map` and `dma_sync` checks to `page_pool_init` to make > them generic. Set dma_sync to false for devmem memory provider because > the dma_sync APIs should not be used for dma_buf backed devmem memory > provider. > > Cc: Jason Gunthorpe <jgg@ziepe.ca> > Signed-off-by: Samiullah Khawaja <skhawaja@google.com> > Signed-off-by: Mina Almasry <almasrymina@google.com> > > --- > net/core/devmem.c | 9 ++++----- > net/core/page_pool.c | 3 +++ > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/net/core/devmem.c b/net/core/devmem.c > index 11b91c12ee11..826d0b00159f 100644 > --- a/net/core/devmem.c > +++ b/net/core/devmem.c > @@ -331,11 +331,10 @@ int mp_dmabuf_devmem_init(struct page_pool *pool) > if (!binding) > return -EINVAL; > > - if (!pool->dma_map) > - return -EOPNOTSUPP; > - > - if (pool->dma_sync) > - return -EOPNOTSUPP; > + /* dma-buf dma addresses should not be used with > + * dma_sync_for_cpu/device. Force disable dma_sync. > + */ > + pool->dma_sync = false; > > if (pool->p.order != 0) > return -E2BIG; > diff --git a/net/core/page_pool.c b/net/core/page_pool.c > index 77de79c1933b..528dd4d18eab 100644 > --- a/net/core/page_pool.c > +++ b/net/core/page_pool.c > @@ -287,6 +287,9 @@ static int page_pool_init(struct page_pool *pool, > } > > if (pool->mp_priv) { > + if (!pool->dma_map || !pool->dma_sync) Is there a reason why ->dma_sync is enforced for all providers? It sounds like a devmem fix, on the other hand I don't really care as we don't have drivers yet implementing queue api / etc. but not passing PP_FLAG_DMA_SYNC_DEV. Also, putting fixes first in the series or sending them separately reduces backporting headache.
On Thu, 7 Nov 2024 21:23:07 +0000 Mina Almasry wrote: > From: Samiullah Khawaja <skhawaja@google.com> > > Move the `dma_map` and `dma_sync` checks to `page_pool_init` to make > them generic. Set dma_sync to false for devmem memory provider because > the dma_sync APIs should not be used for dma_buf backed devmem memory > provider. Let's start from specifying the expectations from the drivers, and documenting them under Documentation/ We should require that drivers set PP_FLAG_DMA_SYNC_DEV on the page pool and always use (a new form of) page_pool_dma_sync_for_cpu() to sync for CPU. Behind the scenes we will configure the PP to act accordingly. For dmabuf backed PP I suspect we just say "syncing is the responsibility of userspace", and configure the PP to do no syncing at all. For other providers we can keep the syncing enabled. But drivers shouldn't be allowed to use any netmem if they don't set PP_FLAG_DMA_SYNC_DEV, just to keep things uniform.
diff --git a/net/core/devmem.c b/net/core/devmem.c index 11b91c12ee11..826d0b00159f 100644 --- a/net/core/devmem.c +++ b/net/core/devmem.c @@ -331,11 +331,10 @@ int mp_dmabuf_devmem_init(struct page_pool *pool) if (!binding) return -EINVAL; - if (!pool->dma_map) - return -EOPNOTSUPP; - - if (pool->dma_sync) - return -EOPNOTSUPP; + /* dma-buf dma addresses should not be used with + * dma_sync_for_cpu/device. Force disable dma_sync. + */ + pool->dma_sync = false; if (pool->p.order != 0) return -E2BIG; diff --git a/net/core/page_pool.c b/net/core/page_pool.c index 77de79c1933b..528dd4d18eab 100644 --- a/net/core/page_pool.c +++ b/net/core/page_pool.c @@ -287,6 +287,9 @@ static int page_pool_init(struct page_pool *pool, } if (pool->mp_priv) { + if (!pool->dma_map || !pool->dma_sync) + return -EOPNOTSUPP; + err = mp_dmabuf_devmem_init(pool); if (err) { pr_warn("%s() mem-provider init failed %d\n", __func__,