Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Qinyun Tan <qinyuntan@linux.alibaba.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: "Johannes Weiner" <hannes@cmpxchg.org>,
	"Michal Koutný" <mkoutny@suse.com>,
	"Lance Yang" <lance.yang@linux.dev>,
	"Qi Zheng" <qi.zheng@linux.dev>,
	"Roman Gushchin" <roman.gushchin@linux.dev>,
	"Muchun Song" <muchun.song@linux.dev>,
	"Dave Chinner" <david@fromorbit.com>,
	"Baolin Wang" <baolin.wang@linux.alibaba.com>,
	"David Hildenbrand" <david@kernel.org>,
	"Xunlei Pang" <xlpang@linux.alibaba.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/list_lru: don't copy stale shrinker id from non-memcg-aware shrinkers
Date: Wed, 2 Sep 2026 11:20:40 +0800	[thread overview]
Message-ID: <cc69f57b-47f3-4842-829e-df1d49158884@linux.alibaba.com> (raw)
In-Reply-To: <20260901102913.5c571bf153bc3830905c6770@linux-foundation.org>


Hi,

On 9/2/26 1:29 AM, Andrew Morton wrote:
> On Tue,  1 Sep 2026 19:51:04 +0800 Qinyun Tan <qinyuntan@linux.alibaba.com> wrote:
> 
>> With cgroup.memory=nokmem, shrinker_memcg_alloc() fails with -ENOSYS
>> for shrinkers without SHRINKER_NONSLAB, and shrinker_alloc() falls
>> back to a non-memcg-aware shrinker.  On this fallback path,
>> shrinker->id is never assigned and keeps 0 from kzalloc(), which is a
>> valid id belonging to whichever memcg-aware shrinker registers first.
>>
>> __list_lru_init() copies shrinker->id unconditionally, so every
>> list_lru backed by such a fallback shrinker (thp-deferred_split,
>> zswap-shrinker, workingset shadow nodes, superblock lrus, ...) ends
>> up with lru->shrinker_id == 0 instead of -1.
>>
>> Under nokmem the list_lru collapses to the shared per-node lists, but
>> __list_lru_add() still calls set_shrinker_bit() against the memcg of
>> the added object.  Most list_lru users are unaffected because their
>> objects resolve to a NULL memcg without kmem accounting, but the THP
>> deferred split queue holds user folios, which are charged regardless
>> of nokmem.  Since no memcg-aware shrinker can register under nokmem,
>> shrinker_nr_max stays 0 and every memcg's shrinker_info has
>> map_nr_max == 0, so the first folio added by khugepaged triggers on
>> every boot:
>>
>>   WARNING: mm/shrinker.c:212 at set_shrinker_bit+0x99/0xa0
>>
>> On systems where a SHRINKER_NONSLAB shrinker (btrfs, xfs) did register
>> and expand the maps, there is no warning; instead bit 0 is set
>> spuriously for an unrelated shrinker.
>>
>> shrinker->id is only meaningful while SHRINKER_MEMCG_AWARE is set,
>> and all readers inside mm/shrinker.c already check the flag before
>> using the id.  Make __list_lru_init() do the same and fall back to -1,
>> so set_shrinker_bit() is never reached with a bogus id.  The stale
>> shrinker->id itself is left as is; cleaning that up is a separate
>> topic.
> 
> Thanks.  I'll queue this for test and review.
> 
>> Fixes: 03375203e1da8 ("mm: do not allocate shrinker info with cgroup.memory=nokmem")
> 
> Worth a cc:stable, I assume.
> 
>> ---
>>
>> Verified on a machine booting with cgroup.memory=nokmem and
>> CONFIG_TRANSPARENT_HUGEPAGE=y: the warning fires once per boot from
>> khugepaged, disappears when nokmem is removed from the command line,
>> and no longer triggers with this fix applied and nokmem set.
> 
> That's useful info.  I'll move it into the changelog.
> 
> 
> Sashiko thinks there's a problem with cgroup_disable=memory as well:
> 	https://sashiko.dev/#/patchset/20260901115104.2944996-1-qinyuntan@linux.alibaba.com

Yes, the observation is valid, and it turned out to be a real crash
on mm-new, not just a semantic inconsistency.

__list_lru_init() only checks mem_cgroup_kmem_disabled(), which
covers cgroup.memory=nokmem but not cgroup_disable=memory.  With the
controller disabled entirely, the lru stays memcg aware while
folio_memcg() is always NULL.

On mainline this is unreachable: the only caller of
folio_memcg_list_lru_alloc(), folio_memcg_alloc_deferred(), bails out
on mem_cgroup_disabled() first.  But the shmem unused-huge shrinker
conversion in mm-new ("mm: shmem: make unused huge shrinker memcg
aware") calls folio_memcg_list_lru_alloc() without such a guard, so
booting with cgroup_disable=memory and writing to a huge=always tmpfs
oopses immediately:

  BUG: unable to handle page fault for address: 0000000000000488
  RIP: 0010:folio_memcg_list_lru_alloc+0x41/0xf0
  Call Trace:
   shmem_get_folio_gfp+0x1cd/0x7c0
   shmem_write_begin+0x5d/0x100
   generic_perform_write+0x89/0x2a0
   shmem_file_write_iter+0x82/0x90
   vfs_write+0x256/0x410
   ksys_write+0x61/0xe0

The faulting address is the offset of mem_cgroup->kmemcg_id,
dereferenced on the NULL memcg in memcg_list_lru_allocated().

I've verified that checking mem_cgroup_disabled() in
__list_lru_init() fixes this: with the lru collapsed to plain
per-node lists, folio_memcg_list_lru_alloc() returns early, the
inode is queued on the per-node list with a NULL objcg (which the
shmem code and obj_cgroup_memcg() handle fine), and the shrinker
still reclaims via the global scan.  Same reproducer runs cleanly.

I'll send the fix as a follow-up patch shortly.  Since the
triggering commit is only in mm-new, no stable backport is needed;
it may make sense to keep it ahead of (or folded near) the shmem
series.

Thanks,
Qinyun Tan


  reply	other threads:[~2026-09-02  3:20 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01 11:51 [PATCH] mm/list_lru: don't copy stale shrinker id from non-memcg-aware shrinkers Qinyun Tan
2026-09-01 17:29 ` Andrew Morton
2026-09-02  3:20   ` Qinyun Tan [this message]
2026-09-02  2:25 ` Muchun Song
2026-09-02  5:30 ` Baolin Wang
2026-09-02 17:17 ` Michal Koutný
2026-09-03  4:06   ` Qinyun Tan
2026-09-03  9:20     ` Michal Koutný

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cc69f57b-47f3-4842-829e-df1d49158884@linux.alibaba.com \
    --to=qinyuntan@linux.alibaba.com \
    --cc=akpm@linux-foundation.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@fromorbit.com \
    --cc=david@kernel.org \
    --cc=hannes@cmpxchg.org \
    --cc=lance.yang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mkoutny@suse.com \
    --cc=muchun.song@linux.dev \
    --cc=qi.zheng@linux.dev \
    --cc=roman.gushchin@linux.dev \
    --cc=xlpang@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox