netdev - Re: [PATCH net-next v3 1/3] page_pool: Rename pp_frag_count to pp_ref

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKhg4tL6TQD+xABr_hd6knQrMwfWjPm_L_SUuJ1WLaBbpss+0g@mail.gmail.com>
Date: Mon, 27 Nov 2023 12:21:32 +0800
From: Liang Chen <liangchen.linux@...il.com>
To: Yunsheng Lin <linyunsheng@...wei.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org, 
	pabeni@...hat.com, hawk@...nel.org, ilias.apalodimas@...aro.org, 
	netdev@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH net-next v3 1/3] page_pool: Rename pp_frag_count to pp_ref_count

On Sat, Nov 25, 2023 at 7:53 PM Yunsheng Lin <linyunsheng@...wei.com> wrote:
>
> On 2023/11/24 15:34, Liang Chen wrote:
>
> >  static inline void page_pool_fragment_page(struct page *page, long nr)
>
> It seems page_pool_fragment_page() might not be a appropriate name too?
>
> Perhaps it might be better to grep defrag/frag to see if there is other
> function name might need changing.
>

Our understanding is that the concept of fragmenting exists before the
page is drained, and all related functions should retain their current
names. However, once the page is drained, its management shifts to
being governed by pp_ref_count, and there's no longer a need to
consider fragmentation. Therefore, all functions associated with that
lifecycle stage of a pp page will be renamed. With that in mind, the
following functions have been renamed.

page_pool_defrag_page -> page_pool_deref_page
page_pool_is_last_frag -> page_pool_is_last_ref
page_pool_put_defragged_page -> page_pool_put_derefed_page

> >  {
> > -     atomic_long_set(&page->pp_frag_count, nr);
> > +     atomic_long_set(&page->pp_ref_count, nr);
> >  }
> >
> > -static inline long page_pool_defrag_page(struct page *page, long nr)
> > +static inline long page_pool_deref_page(struct page *page, long nr)
>
> page_pool_defrag_page() related function is called by mlx5 driver directly,
> we need to change it to use the new function too.
>

Yeah, that change is right at the start of the patch.

> I assume that deref is short for dereference? According to:
>
> https://stackoverflow.com/questions/4955198/what-does-dereferencing-a-pointer-mean-in-c-c
>
> 'dereferencing means accessing the value from a certain memory location
> against which that pointer is pointing'.
>
> So I am not sure if 'deref' is the right word here as I am not a native
> english speaker, But it seems 'unref' is more appropriate here if we mirror
> the napi_frag_unref() function name?

That sounds better to me as well. Thanks!