diff mbox series

[v1,net] page_pool: Cap queue size to 32k.

Message ID 20230814060411.2401817-1-rkannoth@marvell.com
State New
Headers
Series [v1,net] page_pool: Cap queue size to 32k. |

Commit Message

Ratheesh Kannoth Aug. 14, 2023, 6:04 a.m. UTC 
  Clamp to 32k instead of returning error.

Please find discussion at
https://lore.kernel.org/lkml/
CY4PR1801MB1911E15D518A77535F6E51E2D308A@CY4PR1801MB1911.
namprd18.prod.outlook.com/T/

Fixes: ff7d6b27f894 ("page_pool: refurbish version of page_pool code")
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

---
ChangeLog:
v0 -> v1: Rebase && commit message changes
---
 net/core/page_pool.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Johannes Berg Aug. 14, 2023, 7:54 a.m. UTC | #1 
On Mon, 2023-08-14 at 11:34 +0530, Ratheesh Kannoth wrote:
> Clamp to 32k instead of returning error.
> 
> Please find discussion at
> https://lore.kernel.org/lkml/
> CY4PR1801MB1911E15D518A77535F6E51E2D308A@CY4PR1801MB1911.
> namprd18.prod.outlook.com/T/
> 

I'm not the one who's going to apply this, but honestly, I don't think
that will work as a commit message for such a change ...

Sure, link to it by all means, but also summarize it and make sense of
it for the commit message?

johannes
Ratheesh Kannoth Aug. 14, 2023, 8:05 a.m. UTC | #2 
> From: Johannes Berg <johannes@sipsolutions.net>
> Subject: [EXT] Re: [PATCH v1 net] page_pool: Cap queue size to 32k.
> > Please find discussion at
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_l
> >
> I'm not the one who's going to apply this, but honestly, I don't think that will
> work as a commit message for such a change ...
> Sure, link to it by all means, but also summarize it and make sense of it for
> the commit message?

Why do you think it will not work ?. There is a long discussion about pros and cons of different approaches by 
Page pool maintainers in the discussion link.  However I  summarize it here, as commit message, it will 
Lead to some more questions by reviewers. 

-Ratheesh
Ratheesh Kannoth Aug. 14, 2023, 8:55 a.m. UTC | #3 
> From: Jesper Dangaard Brouer <hawk@kernel.org>
> Subject: Re: [EXT] Re: [PATCH v1 net] page_pool: Cap queue size to 32k.


> I agree with Johannes, this commit message is too thin.
ACK.

 
> It makes sense to give a summary of the discussion, because it show us
> (page_pool maintainers) what you concluded for the discussion.
Got it. Thanks. 

> Further more, you also send another patch:
>   - "[PATCH net-next] page_pool: Set page pool size"
Okay. 

>   -
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lore.kernel.org_all_20230809021920.913324-2D1-2Drkannoth-
> 40marvell.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=aekcsyBCH00
> _LewrEDcQBzsRw8KCpUR0vZb_auTHk4M&m=uvV_vt_cNyQItTD90jF1LdKovP
> 7j7FYtnr7I38__nYY6wHtFHSozYoRSSvCI14nh&s=vGgt2ccGdiRTEhj3MoGVx-
> EXHmB03v6I3UIIY1fEb24&e=
> 
> That patch solves the issue for your driver marvell/octeontx2 and I like than
> change.
Okay. 

> Why did you conclude that PP core should also change?
I could not  answer Jacub's question at https://lore.kernel.org/netdev/20230810024422.1781312-1-rkannoth@marvell.com/T/ 

> (p.s. Cc/To list have gotten excessive with 89 recipients)
I added maintainters of all files which used page_pool_init().
diff mbox series

Patch

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index a3e12a61d456..e9dc8d8966ad 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -171,9 +171,10 @@  static int page_pool_init(struct page_pool *pool,
 	if (pool->p.pool_size)
 		ring_qsize = pool->p.pool_size;
 
-	/* Sanity limit mem that can be pinned down */
+	/* Cap queue size to 32k */
 	if (ring_qsize > 32768)
-		return -E2BIG;
+		ring_qsize = 32768;
+
 
 	/* DMA direction is either DMA_FROM_DEVICE or DMA_BIDIRECTIONAL.
 	 * DMA_BIDIRECTIONAL is for allowing page used for DMA sending,