* [PATCH v2 net] octeontx2-pf: fix page_pool creation fail for rings > 32k
@ 2023-08-23 2:53 Ratheesh Kannoth
2023-08-23 7:14 ` Jesper Dangaard Brouer
2023-08-23 10:12 ` Alexander Lobakin
0 siblings, 2 replies; 6+ messages in thread
From: Ratheesh Kannoth @ 2023-08-23 2:53 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: sgoutham, gakula, sbhatta, hkelam, davem, edumazet, kuba, pabeni,
rkannoth, hawk, alexander.duyck, ilias.apalodimas, linyunsheng,
Alexander Lobakin
octeontx2 driver calls page_pool_create() during driver probe()
and fails if queue size > 32k. Page pool infra uses these buffers
as shock absorbers for burst traffic. These pages are pinned down
over time as working sets varies, due to the recycling nature
of page pool, given page pool (currently) don't have a shrinker
mechanism, the pages remain pinned down in ptr_ring.
Instead of clamping page_pool size to 32k at
most, limit it even more to 2k to avoid wasting memory.
This have been tested on octeontx2 CN10KA hardware.
TCP and UDP tests using iperf shows no performance regressions.
Fixes: b2e3406a38f0 ("octeontx2-pf: Add support for page pool")
Suggested-by: Alexander Lobakin <aleksander.lobakin@intel.com>
Reviewed-by: Sunil Goutham <sgoutham@marvell.com>
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
ChangeLogs:
v1->v2: Commit message changes and typo fixes
v0->v1: Commit message changes.
---
drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 2 +-
drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h | 2 ++
2 files changed, 3 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 77c8f650f7ac..3e1c70c74622 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -1432,7 +1432,7 @@ int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id,
}
pp_params.flags = PP_FLAG_PAGE_FRAG | PP_FLAG_DMA_MAP;
- pp_params.pool_size = numptrs;
+ pp_params.pool_size = min(OTX2_PAGE_POOL_SZ, numptrs);
pp_params.nid = NUMA_NO_NODE;
pp_params.dev = pfvf->dev;
pp_params.dma_dir = DMA_FROM_DEVICE;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index ba8091131ec0..f6fea43617ff 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -30,6 +30,8 @@
#include <rvu_trace.h>
#include "qos.h"
+#define OTX2_PAGE_POOL_SZ 2048
+
/* IPv4 flag more fragment bit */
#define IPV4_FLAG_MORE 0x20
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net] octeontx2-pf: fix page_pool creation fail for rings > 32k
2023-08-23 2:53 [PATCH v2 net] octeontx2-pf: fix page_pool creation fail for rings > 32k Ratheesh Kannoth
@ 2023-08-23 7:14 ` Jesper Dangaard Brouer
2023-08-23 10:12 ` Alexander Lobakin
1 sibling, 0 replies; 6+ messages in thread
From: Jesper Dangaard Brouer @ 2023-08-23 7:14 UTC (permalink / raw)
To: Ratheesh Kannoth, netdev, linux-kernel
Cc: sgoutham, gakula, sbhatta, hkelam, davem, edumazet, kuba, pabeni,
hawk, alexander.duyck, ilias.apalodimas, linyunsheng,
Alexander Lobakin
On 23/08/2023 04.53, Ratheesh Kannoth wrote:
> octeontx2 driver calls page_pool_create() during driver probe()
> and fails if queue size > 32k. Page pool infra uses these buffers
> as shock absorbers for burst traffic. These pages are pinned down
> over time as working sets varies, due to the recycling nature
> of page pool, given page pool (currently) don't have a shrinker
> mechanism, the pages remain pinned down in ptr_ring.
> Instead of clamping page_pool size to 32k at
> most, limit it even more to 2k to avoid wasting memory.
>
> This have been tested on octeontx2 CN10KA hardware.
> TCP and UDP tests using iperf shows no performance regressions.
>
> Fixes: b2e3406a38f0 ("octeontx2-pf: Add support for page pool")
> Suggested-by: Alexander Lobakin<aleksander.lobakin@intel.com>
> Reviewed-by: Sunil Goutham<sgoutham@marvell.com>
> Signed-off-by: Ratheesh Kannoth<rkannoth@marvell.com>
LGTM
Acked-by: Jesper Dangaard Brouer <hawk@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net] octeontx2-pf: fix page_pool creation fail for rings > 32k
2023-08-23 2:53 [PATCH v2 net] octeontx2-pf: fix page_pool creation fail for rings > 32k Ratheesh Kannoth
2023-08-23 7:14 ` Jesper Dangaard Brouer
@ 2023-08-23 10:12 ` Alexander Lobakin
2023-08-24 2:40 ` [EXT] " Ratheesh Kannoth
1 sibling, 1 reply; 6+ messages in thread
From: Alexander Lobakin @ 2023-08-23 10:12 UTC (permalink / raw)
To: Ratheesh Kannoth
Cc: netdev, linux-kernel, sgoutham, gakula, sbhatta, hkelam, davem,
edumazet, kuba, pabeni, hawk, alexander.duyck, ilias.apalodimas,
linyunsheng
From: Ratheesh Kannoth <rkannoth@marvell.com>
Date: Wed, 23 Aug 2023 08:23:25 +0530
> octeontx2 driver calls page_pool_create() during driver probe()
> and fails if queue size > 32k. Page pool infra uses these buffers
> as shock absorbers for burst traffic. These pages are pinned down
> over time as working sets varies, due to the recycling nature
> of page pool, given page pool (currently) don't have a shrinker
> mechanism, the pages remain pinned down in ptr_ring.
> Instead of clamping page_pool size to 32k at
> most, limit it even more to 2k to avoid wasting memory.
>
> This have been tested on octeontx2 CN10KA hardware.
> TCP and UDP tests using iperf shows no performance regressions.
See now? I told ya you don't need that many to provide good recycling :>
But nice to see it was confirmed 2k is enough.
>
> Fixes: b2e3406a38f0 ("octeontx2-pf: Add support for page pool")
> Suggested-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> Reviewed-by: Sunil Goutham <sgoutham@marvell.com>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---
>
> ChangeLogs:
>
> v1->v2: Commit message changes and typo fixes
> v0->v1: Commit message changes.
> ---
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 2 +-
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h | 2 ++
> 2 files changed, 3 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 77c8f650f7ac..3e1c70c74622 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> @@ -1432,7 +1432,7 @@ int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id,
> }
>
> pp_params.flags = PP_FLAG_PAGE_FRAG | PP_FLAG_DMA_MAP;
> - pp_params.pool_size = numptrs;
> + pp_params.pool_size = min(OTX2_PAGE_POOL_SZ, numptrs);
> pp_params.nid = NUMA_NO_NODE;
> pp_params.dev = pfvf->dev;
> pp_params.dma_dir = DMA_FROM_DEVICE;
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> index ba8091131ec0..f6fea43617ff 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
Isn't otx2_txrx.h a better place for this definition? For example, I'd
place it somewhere between CGX_CHAN_BASE and OTX2_DATA_ALIGN. Or after
MAX_XDP_MTU, doesn't matter much.
> @@ -30,6 +30,8 @@
> #include <rvu_trace.h>
> #include "qos.h"
>
> +#define OTX2_PAGE_POOL_SZ 2048
Don't forget about alignment. Otx2 code has very inconsistent alignment
of definitions -- just one space in some places and proper tab alignment
in others. I still believe we prefer the second for netdev code.
> +
> /* IPv4 flag more fragment bit */
> #define IPV4_FLAG_MORE 0x20
>
Thanks,
Olek
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [EXT] Re: [PATCH v2 net] octeontx2-pf: fix page_pool creation fail for rings > 32k
2023-08-23 10:12 ` Alexander Lobakin
@ 2023-08-24 2:40 ` Ratheesh Kannoth
0 siblings, 0 replies; 6+ messages in thread
From: Ratheesh Kannoth @ 2023-08-24 2:40 UTC (permalink / raw)
To: Alexander Lobakin
Cc: netdev, linux-kernel, Sunil Kovvuri Goutham,
Geethasowjanya Akula, Subbaraya Sundeep Bhatta, Hariprasad Kelam,
davem, edumazet, kuba, pabeni, hawk, alexander.duyck,
ilias.apalodimas, linyunsheng
> From: Alexander Lobakin <aleksander.lobakin@intel.com>
> Sent: Wednesday, August 23, 2023 3:43 PM
> Subject: [EXT] Re: [PATCH v2 net] octeontx2-pf: fix page_pool creation fail
> for rings > 32k
> See now? I told ya you don't need that many to provide good recycling :> But
> nice to see it was confirmed 2k is enough.
Thanks !
> Isn't otx2_txrx.h a better place for this definition? For example, I'd place it
> somewhere between CGX_CHAN_BASE and OTX2_DATA_ALIGN. Or after
> MAX_XDP_MTU, doesn't matter much.
ACK.
> Don't forget about alignment. Otx2 code has very inconsistent alignment of
> definitions -- just one space in some places and proper tab alignment in
> others. I still believe we prefer the second for netdev code.
ACK. Is it documented somewhere. I mean the coding std for netdev ?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 net] octeontx2-pf: fix page_pool creation fail for rings > 32k
2023-08-16 9:07 Ratheesh Kannoth
@ 2023-08-16 12:44 ` Alexander Lobakin
0 siblings, 0 replies; 6+ messages in thread
From: Alexander Lobakin @ 2023-08-16 12:44 UTC (permalink / raw)
To: Ratheesh Kannoth
Cc: netdev, linux-kernel, sgoutham, gakula, sbhatta, hkelam, davem,
edumazet, kuba, pabeni
From: Ratheesh Kannoth <rkannoth@marvell.com>
Date: Wed, 16 Aug 2023 14:37:18 +0530
> octeontx2 driver calls page_pool_create() during driver probe()
> and fails if queue size > 32k. Page pool infra uses these buffers
> as shock absorbers for burst traffic. These pages are pinned down
> over time as working sets varies, due to the recycling nature
> of page pool, given page pool (currently) don't have a shrinker
> mechanism, the pages remain pinned down in ptr_ring.
> Instead of clamping page_pool size to 32k at
> most, limit it even more to 2k to avoid wasting memory.
>
> This have been tested on octeontx2 CN10KA hardware.
> TCP and UDP tests using iperf shows not performance regressions.
>
> Fixes: b2e3406a38f0 ("octeontx2-pf: Add support for page pool")
> Suggested-by: Alexander Lobakin <aleksander.lobakin@intel.com>
> Reviewed-by: Sunil Goutham <sgoutham@marvell.com>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
> ---
>
> ChangeLogs:
>
> vi->v2: Commit message changes and typo fixes
> v0->v1: Commit message changes.
> ---
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 2 +-
> drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h | 2 ++
> 2 files changed, 3 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 77c8f650f7ac..fc8a1220eb39 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
> @@ -1432,7 +1432,7 @@ int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id,
> }
>
> pp_params.flags = PP_FLAG_PAGE_FRAG | PP_FLAG_DMA_MAP;
> - pp_params.pool_size = numptrs;
> + pp_params.pool_size = OTX2_PAGE_POOL_SZ;
You still didn't respond to my previous message or maybe I missed the
reply somewhere: why not min(numptrs, OTX2_PAGE_POOL_SZ)? Why create
page_pool with 2k elements for rings with 128 descriptors?
> pp_params.nid = NUMA_NO_NODE;
> pp_params.dev = pfvf->dev;
> pp_params.dma_dir = DMA_FROM_DEVICE;
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> index ba8091131ec0..f6fea43617ff 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> @@ -30,6 +30,8 @@
> #include <rvu_trace.h>
> #include "qos.h"
>
> +#define OTX2_PAGE_POOL_SZ 2048
> +
> /* IPv4 flag more fragment bit */
> #define IPV4_FLAG_MORE 0x20
>
Thanks,
Olek
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 net] octeontx2-pf: fix page_pool creation fail for rings > 32k
@ 2023-08-16 9:07 Ratheesh Kannoth
2023-08-16 12:44 ` Alexander Lobakin
0 siblings, 1 reply; 6+ messages in thread
From: Ratheesh Kannoth @ 2023-08-16 9:07 UTC (permalink / raw)
To: netdev, linux-kernel
Cc: sgoutham, gakula, sbhatta, hkelam, davem, edumazet, kuba, pabeni,
rkannoth, Alexander Lobakin
octeontx2 driver calls page_pool_create() during driver probe()
and fails if queue size > 32k. Page pool infra uses these buffers
as shock absorbers for burst traffic. These pages are pinned down
over time as working sets varies, due to the recycling nature
of page pool, given page pool (currently) don't have a shrinker
mechanism, the pages remain pinned down in ptr_ring.
Instead of clamping page_pool size to 32k at
most, limit it even more to 2k to avoid wasting memory.
This have been tested on octeontx2 CN10KA hardware.
TCP and UDP tests using iperf shows not performance regressions.
Fixes: b2e3406a38f0 ("octeontx2-pf: Add support for page pool")
Suggested-by: Alexander Lobakin <aleksander.lobakin@intel.com>
Reviewed-by: Sunil Goutham <sgoutham@marvell.com>
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
ChangeLogs:
vi->v2: Commit message changes and typo fixes
v0->v1: Commit message changes.
---
drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c | 2 +-
drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h | 2 ++
2 files changed, 3 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 77c8f650f7ac..fc8a1220eb39 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -1432,7 +1432,7 @@ int otx2_pool_init(struct otx2_nic *pfvf, u16 pool_id,
}
pp_params.flags = PP_FLAG_PAGE_FRAG | PP_FLAG_DMA_MAP;
- pp_params.pool_size = numptrs;
+ pp_params.pool_size = OTX2_PAGE_POOL_SZ;
pp_params.nid = NUMA_NO_NODE;
pp_params.dev = pfvf->dev;
pp_params.dma_dir = DMA_FROM_DEVICE;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index ba8091131ec0..f6fea43617ff 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -30,6 +30,8 @@
#include <rvu_trace.h>
#include "qos.h"
+#define OTX2_PAGE_POOL_SZ 2048
+
/* IPv4 flag more fragment bit */
#define IPV4_FLAG_MORE 0x20
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-24 2:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-23 2:53 [PATCH v2 net] octeontx2-pf: fix page_pool creation fail for rings > 32k Ratheesh Kannoth
2023-08-23 7:14 ` Jesper Dangaard Brouer
2023-08-23 10:12 ` Alexander Lobakin
2023-08-24 2:40 ` [EXT] " Ratheesh Kannoth
-- strict thread matches above, loose matches on Subject: below --
2023-08-16 9:07 Ratheesh Kannoth
2023-08-16 12:44 ` Alexander Lobakin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).