Messages in this thread | | | Subject | Re: [PATCH net-next] page_pool: Rename frag_users to frag_cnt | From | Yunsheng Lin <> | Date | Thu, 21 Dec 2023 15:59:32 +0800 |
| |
On 2023/12/21 14:37, Ilias Apalodimas wrote: > Hi Yunsheng, > > On Thu, 21 Dec 2023 at 04:07, Yunsheng Lin <linyunsheng@huawei.com> wrote: >> >> On 2023/12/20 15:56, Ilias Apalodimas wrote: >>> Hi Yunsheng, >>>>>>> #ifdef CONFIG_PAGE_POOL_STATS >>>>>>> /* these stats are incremented while in softirq context */ >>>>>>> diff --git a/net/core/page_pool.c b/net/core/page_pool.c >>>>>>> index 9b203d8660e4..19a56a52ac8f 100644 >>>>>>> --- a/net/core/page_pool.c >>>>>>> +++ b/net/core/page_pool.c >>>>>>> @@ -659,7 +659,7 @@ EXPORT_SYMBOL(page_pool_put_page_bulk); >>>>>>> static struct page *page_pool_drain_frag(struct page_pool *pool, >>>>>>> struct page *page) >>>>>>> { >>>>>>> - long drain_count = BIAS_MAX - pool->frag_users; >>>>>>> + long drain_count = BIAS_MAX - pool->frag_cnt; >>>>>> >>>>>> drain_count = pool->refcnt_bais; >>>>> >>>>> I think this is a typo right? This still remains >>>> >>>> It would be better to invert logic too, as it is mirroring: >>>> >>>> https://elixir.bootlin.com/linux/v6.7-rc5/source/mm/page_alloc.c#L4745 >>> >>> This is still a bit confusing for me since the actual bias is the >>> number of fragments that you initially split the page. But I am fine >> Acctually there are two bais numbers for a page used by >> page_pool_alloc_frag(). >> the one for page->pp_ref_count, which already use the BIAS_MAX, which >> indicates the initial bais number: >> https://elixir.bootlin.com/linux/latest/source/net/core/page_pool.c#L779 >> >> Another one for pool->frag_users indicating the runtime bais number, which >> need changing when a page is split into more fragments: >> https://elixir.bootlin.com/linux/latest/source/net/core/page_pool.c#L776 >> https://elixir.bootlin.com/linux/latest/source/net/core/page_pool.c#L783 > > I know, and that's exactly what my commit message explains. Also, > that's the reason that the rename was 'frag_cnt' on v1. >
Yes, I think we do not need to invert logic when the naming is frag_users or frag_cnt.
But if we use 'bias' as part of the name, isn't that more reasonable to set both of the bias number to BIAS_MAX initially, and decrement the runtime bais number every time the page is split to more fragmemts?
>
| |