> From: Johannes Berg <[email protected]>
> 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
On 14/08/2023 10.05, Ratheesh Kannoth wrote:
>> From: Johannes Berg <[email protected]>
>> 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.
>
I agree with Johannes, this commit message is too thin.
It makes sense to give a summary of the discussion, because it show us
(page_pool maintainers) what you concluded for the discussion.
Further more, you also send another patch:
- "[PATCH net-next] page_pool: Set page pool size"
-
https://lore.kernel.org/all/[email protected]/
That patch solves the issue for your driver marvell/octeontx2 and I like
than change.
Why did you conclude that PP core should also change?
--Jesper
(p.s. Cc/To list have gotten excessive with 89 recipients)
> From: Jesper Dangaard Brouer <[email protected]>
> 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/[email protected]/T/
> (p.s. Cc/To list have gotten excessive with 89 recipients)
I added maintainters of all files which used page_pool_init().