The standard idiom for getting head page of a given folio is
'&folio->page'.  It is efficient and safe even if the folio is NULL,
because the offset of page field in folio is zero.  However, it makes
the code not that easy to understand at the first glance, especially the
NULL safety.  Also, sometimes people forget the idiom and use
'folio_page(folio, 0)' instead.  To make it easier to read and remember,
add a new macro function called 'folio_headpage()' with the NULL case
explanation.  Then, replace the 'folio_page(folio, 0)' calls with
'folio_headpage(folio)'.


SeongJae Park (3):
  include/linux/page-flags: add folio_headpage()
  mm: use folio_headpage() instead of folio_page()
  fs/ceph/addr: use folio_headpage() instead of folio_page()

 fs/ceph/addr.c             | 2 +-
 include/linux/page-flags.h | 8 ++++++++
 mm/shmem.c                 | 4 ++--
 mm/slab.c                  | 6 +++---
 mm/slab_common.c           | 4 ++--
 mm/slub.c                  | 4 ++--
 6 files changed, 18 insertions(+), 10 deletions(-)

-- 
2.25.1

Sorry I didn't see the [1/3] and [2/3] patches in my inbox, it seems you 
didn't CCed the ceph-devel@ mail list.

Thanks

On 07/01/2023 01:40, SeongJae Park wrote:
> The standard idiom for getting head page of a given folio is
> '&folio->page'.  It is efficient and safe even if the folio is NULL,
> because the offset of page field in folio is zero.  However, it makes
> the code not that easy to understand at the first glance, especially the
> NULL safety.  Also, sometimes people forget the idiom and use
> 'folio_page(folio, 0)' instead.  To make it easier to read and remember,
> add a new macro function called 'folio_headpage()' with the NULL case
> explanation.  Then, replace the 'folio_page(folio, 0)' calls with
> 'folio_headpage(folio)'.
>
>
> SeongJae Park (3):
>    include/linux/page-flags: add folio_headpage()
>    mm: use folio_headpage() instead of folio_page()
>    fs/ceph/addr: use folio_headpage() instead of folio_page()
>
>   fs/ceph/addr.c             | 2 +-
>   include/linux/page-flags.h | 8 ++++++++
>   mm/shmem.c                 | 4 ++--
>   mm/slab.c                  | 6 +++---
>   mm/slab_common.c           | 4 ++--
>   mm/slub.c                  | 4 ++--
>   6 files changed, 18 insertions(+), 10 deletions(-)
>
-- 
Best Regards,

Xiubo Li (李秀波)

Email: xiubli@redhat.com/xiubli@ibm.com
Slack: @Xiubo Li

On Fri, Jan 06, 2023 at 05:40:25PM +0000, SeongJae Park wrote:
> The standard idiom for getting head page of a given folio is
> '&folio->page'.  It is efficient and safe even if the folio is NULL,
> because the offset of page field in folio is zero.  However, it makes
> the code not that easy to understand at the first glance, especially the
> NULL safety.  Also, sometimes people forget the idiom and use
> 'folio_page(folio, 0)' instead.  To make it easier to read and remember,
> add a new macro function called 'folio_headpage()' with the NULL case
> explanation.  Then, replace the 'folio_page(folio, 0)' calls with
> 'folio_headpage(folio)'.

No.  Everywhere that uses &folio->page is a place that needs to be fixed.
It shouldn't have a nice convenience macro.  It should make you mildly
uncomfortable.

On Fri, 6 Jan 2023 19:21:40 +0000 Matthew Wilcox <willy@infradead.org> wrote:

> On Fri, Jan 06, 2023 at 05:40:25PM +0000, SeongJae Park wrote:
> > The standard idiom for getting head page of a given folio is
> > '&folio->page'.  It is efficient and safe even if the folio is NULL,
> > because the offset of page field in folio is zero.  However, it makes
> > the code not that easy to understand at the first glance, especially the
> > NULL safety.  Also, sometimes people forget the idiom and use
> > 'folio_page(folio, 0)' instead.  To make it easier to read and remember,
> > add a new macro function called 'folio_headpage()' with the NULL case
> > explanation.  Then, replace the 'folio_page(folio, 0)' calls with
> > 'folio_headpage(folio)'.
> 
> No.  Everywhere that uses &folio->page is a place that needs to be fixed.
> It shouldn't have a nice convenience macro.  It should make you mildly
> uncomfortable.

It's true that it's just a mild uncomfortableness.  I will respect your opinion
here.  Thanks for the input.


Thanks,
SJ