Re: [PATCH v2 2/3] mm: use PF_NO_TAIL for PG_lru

[Yu Zhao <yuzhao@xxxxxxxxxx>]

On Fri, Feb 26, 2021 at 02:17:17AM -0700, Yu Zhao wrote:
> Trying to set or clear PG_lru on tail pages has been considered buggy.
> Enforce this rule by changing the policy for PG_lru from PF_HEAD to
> PF_NO_TAIL. This means setting or clearing PG_lru on tail pages won't
> be "corrected" by compound_page(). Such "correction" isn't helpful --
> even if a piece of buggy code has gotten away with
> compound_page(tail)->flags, it will run into trouble with lru list
> addition and deletion because they use tail->lru rather than
> compound_page(tail)->lru.
> 
> bloat-o-meter result:
>   add/remove: 0/0 grow/shrink: 0/11 up/down: 0/-535 (-535)
> 
> Signed-off-by: Yu Zhao <yuzhao@xxxxxxxxxx>
> ---
>  include/linux/page-flags.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 1995208a3763..c9626e54df8d 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -333,8 +333,8 @@ PAGEFLAG(Referenced, referenced, PF_HEAD)
>  	__SETPAGEFLAG(Referenced, referenced, PF_HEAD)
>  PAGEFLAG(Dirty, dirty, PF_HEAD) TESTSCFLAG(Dirty, dirty, PF_HEAD)
>  	__CLEARPAGEFLAG(Dirty, dirty, PF_HEAD)
> -PAGEFLAG(LRU, lru, PF_HEAD) __CLEARPAGEFLAG(LRU, lru, PF_HEAD)
> -	TESTCLEARFLAG(LRU, lru, PF_HEAD)

As a side note, IMO, testing PG_lru on compound_head(tail)->flags is
a bug because it defeats the purpose of the following pattern when,
e.g., racing with compound page creations.

This pattern is intended to avoid dirtying struct page cache line when
scanning PFNs speculatively in isolate_migratepages_block() and
page_idle_get_page(). Without compound_head(), it works well when it
misses head pages. But with compound_head(), get_page_unless_zero()
will run unnecessarily on tail pages.

  if (!PageLRU(page) || !get_page_unless_zero(page))
    continue;

  if (!PageLRU(page)) {
    put_page(page);
    continue;
  }

  do_something();