| Message ID | 20210121070906.25380-1-haokexin@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | db2805150a0f27c00ad286a29109397a7723adad |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: octeontx2: Make sure the buffer is 128 byte aligned | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 12 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 7 this patch: 7 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
Hi Kevin, Tested at my end and works fine. Thanks for the patch. Tested-by: Subbaraya Sundeep <sbhatta@marvell.com> Sundeep On Thu, Jan 21, 2021 at 12:51 PM Kevin Hao <haokexin@gmail.com> wrote: > > The octeontx2 hardware needs the buffer to be 128 byte aligned. > But in the current implementation of napi_alloc_frag(), it can't > guarantee the return address is 128 byte aligned even the request size > is a multiple of 128 bytes, so we have to request an extra 128 bytes and > use the PTR_ALIGN() to make sure that the buffer is aligned correctly. > > Fixes: 7a36e4918e30 ("octeontx2-pf: Use the napi_alloc_frag() to alloc the pool buffers") > Reported-by: Subbaraya Sundeep <sbhatta@marvell.com> > Signed-off-by: Kevin Hao <haokexin@gmail.com> > --- > drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > index bdfa2e293531..5ddedc3b754d 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > @@ -488,10 +488,11 @@ dma_addr_t __otx2_alloc_rbuf(struct otx2_nic *pfvf, struct otx2_pool *pool) > dma_addr_t iova; > u8 *buf; > > - buf = napi_alloc_frag(pool->rbsize); > + buf = napi_alloc_frag(pool->rbsize + OTX2_ALIGN); > if (unlikely(!buf)) > return -ENOMEM; > > + buf = PTR_ALIGN(buf, OTX2_ALIGN); > iova = dma_map_single_attrs(pfvf->dev, buf, pool->rbsize, > DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); > if (unlikely(dma_mapping_error(pfvf->dev, iova))) { > -- > 2.29.2 >
From: Kevin Hao > Sent: 21 January 2021 07:09 > > The octeontx2 hardware needs the buffer to be 128 byte aligned. > But in the current implementation of napi_alloc_frag(), it can't > guarantee the return address is 128 byte aligned even the request size > is a multiple of 128 bytes, so we have to request an extra 128 bytes and > use the PTR_ALIGN() to make sure that the buffer is aligned correctly. > > Fixes: 7a36e4918e30 ("octeontx2-pf: Use the napi_alloc_frag() to alloc the pool buffers") > Reported-by: Subbaraya Sundeep <sbhatta@marvell.com> > Signed-off-by: Kevin Hao <haokexin@gmail.com> > --- > drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > index bdfa2e293531..5ddedc3b754d 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > @@ -488,10 +488,11 @@ dma_addr_t __otx2_alloc_rbuf(struct otx2_nic *pfvf, struct otx2_pool *pool) > dma_addr_t iova; > u8 *buf; > > - buf = napi_alloc_frag(pool->rbsize); > + buf = napi_alloc_frag(pool->rbsize + OTX2_ALIGN); > if (unlikely(!buf)) > return -ENOMEM; > > + buf = PTR_ALIGN(buf, OTX2_ALIGN); > iova = dma_map_single_attrs(pfvf->dev, buf, pool->rbsize, > DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); > if (unlikely(dma_mapping_error(pfvf->dev, iova))) { > -- > 2.29.2 Doesn't that break the 'free' code ? Surely it needs the original pointer. It isn't obvious that page_frag_free() it correct when the allocator is napi_alloc_frag() either. I'd have thought it ought to be returned to the pool. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Thu, Jan 21, 2021 at 09:53:08AM +0000, David Laight wrote: > From: Kevin Hao > > Sent: 21 January 2021 07:09 > > > > The octeontx2 hardware needs the buffer to be 128 byte aligned. > > But in the current implementation of napi_alloc_frag(), it can't > > guarantee the return address is 128 byte aligned even the request size > > is a multiple of 128 bytes, so we have to request an extra 128 bytes and > > use the PTR_ALIGN() to make sure that the buffer is aligned correctly. > > > > Fixes: 7a36e4918e30 ("octeontx2-pf: Use the napi_alloc_frag() to alloc the pool buffers") > > Reported-by: Subbaraya Sundeep <sbhatta@marvell.com> > > Signed-off-by: Kevin Hao <haokexin@gmail.com> > > --- > > drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > > b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > > index bdfa2e293531..5ddedc3b754d 100644 > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > > @@ -488,10 +488,11 @@ dma_addr_t __otx2_alloc_rbuf(struct otx2_nic *pfvf, struct otx2_pool *pool) > > dma_addr_t iova; > > u8 *buf; > > > > - buf = napi_alloc_frag(pool->rbsize); > > + buf = napi_alloc_frag(pool->rbsize + OTX2_ALIGN); > > if (unlikely(!buf)) > > return -ENOMEM; > > > > + buf = PTR_ALIGN(buf, OTX2_ALIGN); > > iova = dma_map_single_attrs(pfvf->dev, buf, pool->rbsize, > > DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); > > if (unlikely(dma_mapping_error(pfvf->dev, iova))) { > > -- > > 2.29.2 > > Doesn't that break the 'free' code ? > Surely it needs the original pointer. Why do we care about the original pointer? The free code should work with the mangling poiner. Did I miss something? > > It isn't obvious that page_frag_free() it correct when the > allocator is napi_alloc_frag() either. > I'd have thought it ought to be returned to the pool. Sorry, I didn't get what you mean. Could you elaborate a bit more? Thanks, Kevin > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) >
On Thu, Jan 21, 2021 at 5:26 PM Kevin Hao <haokexin@gmail.com> wrote: > > On Thu, Jan 21, 2021 at 09:53:08AM +0000, David Laight wrote: > > From: Kevin Hao > > > Sent: 21 January 2021 07:09 > > > > > > The octeontx2 hardware needs the buffer to be 128 byte aligned. > > > But in the current implementation of napi_alloc_frag(), it can't > > > guarantee the return address is 128 byte aligned even the request size > > > is a multiple of 128 bytes, so we have to request an extra 128 bytes and > > > use the PTR_ALIGN() to make sure that the buffer is aligned correctly. > > > > > > Fixes: 7a36e4918e30 ("octeontx2-pf: Use the napi_alloc_frag() to alloc the pool buffers") > > > Reported-by: Subbaraya Sundeep <sbhatta@marvell.com> > > > Signed-off-by: Kevin Hao <haokexin@gmail.com> > > > --- > > > drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 3 ++- > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > > > b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > > > index bdfa2e293531..5ddedc3b754d 100644 > > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c > > > @@ -488,10 +488,11 @@ dma_addr_t __otx2_alloc_rbuf(struct otx2_nic *pfvf, struct otx2_pool *pool) > > > dma_addr_t iova; > > > u8 *buf; > > > > > > - buf = napi_alloc_frag(pool->rbsize); > > > + buf = napi_alloc_frag(pool->rbsize + OTX2_ALIGN); > > > if (unlikely(!buf)) > > > return -ENOMEM; > > > > > > + buf = PTR_ALIGN(buf, OTX2_ALIGN); > > > iova = dma_map_single_attrs(pfvf->dev, buf, pool->rbsize, > > > DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); > > > if (unlikely(dma_mapping_error(pfvf->dev, iova))) { > > > -- > > > 2.29.2 > > > > Doesn't that break the 'free' code ? > > Surely it needs the original pointer. > > Why do we care about the original pointer? The free code should work with > the mangling poiner. Did I miss something? > > > I too agree, put_page(buf) or put_page(buf + OTX2_ALIGN) is same.
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Thu, 21 Jan 2021 15:09:06 +0800 you wrote: > The octeontx2 hardware needs the buffer to be 128 byte aligned. > But in the current implementation of napi_alloc_frag(), it can't > guarantee the return address is 128 byte aligned even the request size > is a multiple of 128 bytes, so we have to request an extra 128 bytes and > use the PTR_ALIGN() to make sure that the buffer is aligned correctly. > > Fixes: 7a36e4918e30 ("octeontx2-pf: Use the napi_alloc_frag() to alloc the pool buffers") > Reported-by: Subbaraya Sundeep <sbhatta@marvell.com> > Signed-off-by: Kevin Hao <haokexin@gmail.com> > > [...] Here is the summary with links: - net: octeontx2: Make sure the buffer is 128 byte aligned https://git.kernel.org/netdev/net/c/db2805150a0f You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c index bdfa2e293531..5ddedc3b754d 100644 --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c @@ -488,10 +488,11 @@ dma_addr_t __otx2_alloc_rbuf(struct otx2_nic *pfvf, struct otx2_pool *pool) dma_addr_t iova; u8 *buf; - buf = napi_alloc_frag(pool->rbsize); + buf = napi_alloc_frag(pool->rbsize + OTX2_ALIGN); if (unlikely(!buf)) return -ENOMEM; + buf = PTR_ALIGN(buf, OTX2_ALIGN); iova = dma_map_single_attrs(pfvf->dev, buf, pool->rbsize, DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC); if (unlikely(dma_mapping_error(pfvf->dev, iova))) {
The octeontx2 hardware needs the buffer to be 128 byte aligned. But in the current implementation of napi_alloc_frag(), it can't guarantee the return address is 128 byte aligned even the request size is a multiple of 128 bytes, so we have to request an extra 128 bytes and use the PTR_ALIGN() to make sure that the buffer is aligned correctly. Fixes: 7a36e4918e30 ("octeontx2-pf: Use the napi_alloc_frag() to alloc the pool buffers") Reported-by: Subbaraya Sundeep <sbhatta@marvell.com> Signed-off-by: Kevin Hao <haokexin@gmail.com> --- drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)