Somehow I was stupid enough to forget CC'ing Matthew :) +Matthew Wilcox On Mon, Mar 13, 2023 at 1:34 AM Yosry Ahmed <yosryahmed@xxxxxxxxxx> wrote: > > From: Hugh Dickins <hughd@xxxxxxxxxx> > > In a kernel with added WARN_ON_ONCE(PageTail) in page_memcg_check(), we > observed a warning from page_cgroup_ino() when reading > /proc/kpagecgroup. This warning was added to catch fragile reads of > a page memcg. Make page_cgroup_ino() get memcg from compound_head(page): > that gives it the correct memcg for each subpage of a compound page, > so is the right fix. > > I dithered between the right fix and the safer "fix": it's unlikely but > conceivable that some userspace has learnt that /proc/kpagecgroup gives > no memcg on tail pages, and compensates for that in some (racy) way: so > continuing to give no memcg on tails, without warning, might be safer. > > But hwpoison_filter_task(), the only other user of page_cgroup_ino(), > persuaded me. It looks as if it currently leaves out tail pages of the > selected memcg, by mistake: whereas hwpoison_inject() uses compound_head() > and expects the tails to be included. So hwpoison testing coverage has > probably been restricted by the wrong output from page_cgroup_ino() (if > that memcg filter is used at all): in the short term, it might be safer > not to enable wider coverage there, but long term we would regret that. > > Signed-off-by: Hugh Dickins <hughd@xxxxxxxxxx> > Signed-off-by: Yosry Ahmed <yosryahmed@xxxxxxxxxx> > --- > > (Yosry: Alternatively, we could modify page_memcg_check() to do > page_folio() like its sibling page_memcg(), as page_cgroup_ino() is the > only remaining caller other than print_page_owner_memcg(); and it already > excludes pages that have page->memcg_data = 0) > > --- > mm/memcontrol.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 5abffe6f8389..e3a55295725e 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -395,7 +395,7 @@ ino_t page_cgroup_ino(struct page *page) > unsigned long ino = 0; > > rcu_read_lock(); > - memcg = page_memcg_check(page); > + memcg = page_memcg_check(compound_head(page)); > > while (memcg && !(memcg->css.flags & CSS_ONLINE)) > memcg = parent_mem_cgroup(memcg); > -- > 2.40.0.rc1.284.g88254d51c5-goog >
