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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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();




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux