Since [0] got merged, it's clear that 'pp_ref_count' is used to track
the number of users for each page. On struct page_pool though we have
a member called 'frag_users'. Despite of what the name suggests this is
not the number of users. It instead represents the number of fragments of
the current page. When we split the page this is set to the actual number
of frags and later used in page_pool_drain_frag() to infer the real number
of users.
So let's rename it to something that matches the description above
[0]
Link: https://lore.kernel.org/netdev/[email protected]/
Signed-off-by: Ilias Apalodimas <[email protected]>
---
Changes since v1:
- rename to pagecnt_bias instead of frag_cnt to match the mm subsystem
- rebase on top of -main
include/net/page_pool/types.h | 2 +-
net/core/page_pool.c | 8 ++++----
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/page_pool/types.h b/include/net/page_pool/types.h
index 76481c465375..d47491ba973d 100644
--- a/include/net/page_pool/types.h
+++ b/include/net/page_pool/types.h
@@ -130,7 +130,7 @@ struct page_pool {
bool has_init_callback;
- long frag_users;
+ long pagecnt_bias;
struct page *frag_page;
unsigned int frag_offset;
u32 pages_state_hold_cnt;
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 4933762e5a6b..0e64d6b8e748 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -760,7 +760,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->pagecnt_bias;
/* Some user is still using the page frag */
if (likely(page_pool_unref_page(page, drain_count)))
@@ -779,7 +779,7 @@ static struct page *page_pool_drain_frag(struct page_pool *pool,
static void page_pool_free_frag(struct page_pool *pool)
{
- long drain_count = BIAS_MAX - pool->frag_users;
+ long drain_count = BIAS_MAX - pool->pagecnt_bias;
struct page *page = pool->frag_page;
pool->frag_page = NULL;
@@ -821,14 +821,14 @@ struct page *page_pool_alloc_frag(struct page_pool *pool,
pool->frag_page = page;
frag_reset:
- pool->frag_users = 1;
+ pool->pagecnt_bias = 1;
*offset = 0;
pool->frag_offset = size;
page_pool_fragment_page(page, BIAS_MAX);
return page;
}
- pool->frag_users++;
+ pool->pagecnt_bias++;
pool->frag_offset = *offset + size;
alloc_stat_inc(pool, fast);
return page;
--
2.37.2
On Wed, 20 Dec 2023 10:01:46 +0200 Ilias Apalodimas wrote:
> - long frag_users;
> + long pagecnt_bias;
IDK :(
pagecnt to mean suggests this is related to page refcount,
not page pool specific refcount.
More importantly bias is the large number by which we increment.
This counter counts how many of the bias references we actually
consumed. So how about bias_consumed? bias_used? bias_issued?
frags_alloced?
Hi Jakub,
On Wed, 3 Jan 2024 at 01:35, Jakub Kicinski <[email protected]> wrote:
>
> On Wed, 20 Dec 2023 10:01:46 +0200 Ilias Apalodimas wrote:
> > - long frag_users;
> > + long pagecnt_bias;
>
> IDK :(
>
> pagecnt to mean suggests this is related to page refcount,
> not page pool specific refcount.
>
> More importantly bias is the large number by which we increment.
> This counter counts how many of the bias references we actually
> consumed. So how about bias_consumed? bias_used? bias_issued?
> frags_alloced?
I had similar concerns, that's why v1 used 'frag_cnt'.
I am fine with either frags_alloced or frag_cnt
Thanks
/Ilias
On 2024/1/9 15:02, Ilias Apalodimas wrote:
> Hi Jakub,
>
> On Wed, 3 Jan 2024 at 01:35, Jakub Kicinski <[email protected]> wrote:
>>
>> On Wed, 20 Dec 2023 10:01:46 +0200 Ilias Apalodimas wrote:
>>> - long frag_users;
>>> + long pagecnt_bias;
>>
>> IDK :(
>>
>> pagecnt to mean suggests this is related to page refcount,
>> not page pool specific refcount.
>>
>> More importantly bias is the large number by which we increment.
>> This counter counts how many of the bias references we actually
>> consumed. So how about bias_consumed? bias_used? bias_issued?
>> frags_alloced?
>
> I had similar concerns, that's why v1 used 'frag_cnt'.
> I am fine with either frags_alloced or frag_cnt
I would suggest that we stick with the *_bias version, and invert the
logic of draining, so that:
1. Aovid one subtraction operation in the data path.
2. Align with other frag implementation.
I guess I can do the above if that is ok with Ilias.
>
> Thanks
> /Ilias
> .
>