| Message ID | 20210226091718.2927291-3-yuzhao@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | trim the uses of compound_head() | expand |
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@google.com> > --- > 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();
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) +PAGEFLAG(LRU, lru, PF_NO_TAIL) __CLEARPAGEFLAG(LRU, lru, PF_NO_TAIL) + TESTCLEARFLAG(LRU, lru, PF_NO_TAIL) PAGEFLAG(Active, active, PF_HEAD) __CLEARPAGEFLAG(Active, active, PF_HEAD) TESTCLEARFLAG(Active, active, PF_HEAD) PAGEFLAG(Workingset, workingset, PF_HEAD)
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@google.com> --- include/linux/page-flags.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)