Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mm/shrinker: fix bogus set_shrinker_bit() with cgroup.memory=nokmem
@ 2026-09-02  7:37 Jiayuan Chen
  2026-09-02 11:34 ` Usama Arif
  2026-09-02 15:40 ` Shakeel Butt
  0 siblings, 2 replies; 4+ messages in thread
From: Jiayuan Chen @ 2026-09-02  7:37 UTC (permalink / raw)
  To: linux-mm
  Cc: Jiayuan Chen, Andrew Morton, Dave Chinner, Qi Zheng,
	Roman Gushchin, Muchun Song, Shakeel Butt, Kairui Song,
	Johannes Weiner, Usama Arif, linux-kernel

With cgroup.memory=nokmem, shrinker_memcg_alloc() bails out early and
never allocates an id, so shrinker->id keeps the 0 it got from the
kzalloc() in shrinker_alloc(). __list_lru_init() then copies that 0
into lru->shrinker_id, where it looks like a valid bit index.

Nothing calls expand_shrinker_info() on nokmem either, so
shrinker_nr_max stays 0 and every memcg ends up with an empty map
(map_nr_max == 0).

deferred_split_folio() hands a real memcg to __list_lru_add()
regardless of whether the lru is memcg aware, so the first THP queued
in a cgroup does set_shrinker_bit(memcg, nid, 0) and trips the bounds
check:

WARNING: mm/shrinker.c:212 at set_shrinker_bit+0x7d/0x90, CPU#126
Call Trace:
 <TASK>
 deferred_split_folio+0x18c/0x220
 map_anon_folio_pmd_nopf+0xdd/0x130
 map_anon_folio_pmd_pf+0x14/0xb0
 do_huge_pmd_anonymous_page+0x1a1/0x620
 __handle_mm_fault+0xea9/0x10d0
 handle_mm_fault+0xe5/0x320
 do_user_addr_fault+0x1cc/0x870
 exc_page_fault+0x81/0x1b0
 asm_exc_page_fault+0x27/0x30
 </TASK>

Harmless, the WARN_ON_ONCE() is what keeps the out of bounds unit[]
read from happening, but the id should not look valid in the first
place. Clear it before returning.

Two other spots could paper over this: drop the id in
__list_lru_init() when nokmem turns memcg_aware off, or make
deferred_split_folio() pass NULL like list_lru_add_obj() does. Both
leave shrinker->id lying around for the next caller, so fix it where
the id is handed out.

Fixes: fafaeceb89a5 ("mm: switch deferred split shrinker to list_lru")
Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
---
 mm/shrinker.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/shrinker.c b/mm/shrinker.c
index a70aab124a0e..7ec2a9704f6f 100644
--- a/mm/shrinker.c
+++ b/mm/shrinker.c
@@ -227,6 +227,8 @@ static int shrinker_memcg_alloc(struct shrinker *shrinker)
 {
 	int id;
 
+	shrinker->id = -1;
+
 	if (mem_cgroup_disabled())
 		return -ENOSYS;
 	if (mem_cgroup_kmem_disabled() && !(shrinker->flags & SHRINKER_NONSLAB))
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/shrinker: fix bogus set_shrinker_bit() with cgroup.memory=nokmem
  2026-09-02  7:37 [PATCH] mm/shrinker: fix bogus set_shrinker_bit() with cgroup.memory=nokmem Jiayuan Chen
@ 2026-09-02 11:34 ` Usama Arif
  2026-09-02 11:44   ` Jiayuan Chen
  2026-09-02 15:40 ` Shakeel Butt
  1 sibling, 1 reply; 4+ messages in thread
From: Usama Arif @ 2026-09-02 11:34 UTC (permalink / raw)
  To: Jiayuan Chen, linux-mm
  Cc: Andrew Morton, Dave Chinner, Qi Zheng, Roman Gushchin,
	Muchun Song, Shakeel Butt, Kairui Song, Johannes Weiner,
	linux-kernel



On 02/09/2026 08:37, Jiayuan Chen wrote:
> With cgroup.memory=nokmem, shrinker_memcg_alloc() bails out early and
> never allocates an id, so shrinker->id keeps the 0 it got from the
> kzalloc() in shrinker_alloc(). __list_lru_init() then copies that 0
> into lru->shrinker_id, where it looks like a valid bit index.
> 
> Nothing calls expand_shrinker_info() on nokmem either, so
> shrinker_nr_max stays 0 and every memcg ends up with an empty map
> (map_nr_max == 0).
> 
> deferred_split_folio() hands a real memcg to __list_lru_add()
> regardless of whether the lru is memcg aware, so the first THP queued
> in a cgroup does set_shrinker_bit(memcg, nid, 0) and trips the bounds
> check:
> 
> WARNING: mm/shrinker.c:212 at set_shrinker_bit+0x7d/0x90, CPU#126
> Call Trace:
>  <TASK>
>  deferred_split_folio+0x18c/0x220
>  map_anon_folio_pmd_nopf+0xdd/0x130
>  map_anon_folio_pmd_pf+0x14/0xb0
>  do_huge_pmd_anonymous_page+0x1a1/0x620
>  __handle_mm_fault+0xea9/0x10d0
>  handle_mm_fault+0xe5/0x320
>  do_user_addr_fault+0x1cc/0x870
>  exc_page_fault+0x81/0x1b0
>  asm_exc_page_fault+0x27/0x30
>  </TASK>
> 
> Harmless, the WARN_ON_ONCE() is what keeps the out of bounds unit[]
> read from happening, but the id should not look valid in the first
> place. Clear it before returning.
> 
> Two other spots could paper over this: drop the id in
> __list_lru_init() when nokmem turns memcg_aware off, or make
> deferred_split_folio() pass NULL like list_lru_add_obj() does. Both
> leave shrinker->id lying around for the next caller, so fix it where
> the id is handed out.
> 
> Fixes: fafaeceb89a5 ("mm: switch deferred split shrinker to list_lru")
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> ---
>  mm/shrinker.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/shrinker.c b/mm/shrinker.c
> index a70aab124a0e..7ec2a9704f6f 100644
> --- a/mm/shrinker.c
> +++ b/mm/shrinker.c
> @@ -227,6 +227,8 @@ static int shrinker_memcg_alloc(struct shrinker *shrinker)
>  {
>  	int id;
>  
> +	shrinker->id = -1;
> +
>  	if (mem_cgroup_disabled())
>  		return -ENOSYS;
>  	if (mem_cgroup_kmem_disabled() && !(shrinker->flags & SHRINKER_NONSLAB))


Thanks for the fix!

I think the initialization would look better in shrinker_alloc()?



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/shrinker: fix bogus set_shrinker_bit() with cgroup.memory=nokmem
  2026-09-02 11:34 ` Usama Arif
@ 2026-09-02 11:44   ` Jiayuan Chen
  0 siblings, 0 replies; 4+ messages in thread
From: Jiayuan Chen @ 2026-09-02 11:44 UTC (permalink / raw)
  To: Usama Arif, linux-mm
  Cc: Andrew Morton, Dave Chinner, Qi Zheng, Roman Gushchin,
	Muchun Song, Shakeel Butt, Kairui Song, Johannes Weiner,
	linux-kernel


on 9/2/26 7:34 PM, Usama Arif wrote:
>
> On 02/09/2026 08:37, Jiayuan Chen wrote:
>> With cgroup.memory=nokmem, shrinker_memcg_alloc() bails out early and
>> never allocates an id, so shrinker->id keeps the 0 it got from the
>> kzalloc() in shrinker_alloc(). __list_lru_init() then copies that 0
>> into lru->shrinker_id, where it looks like a valid bit index.
>>
>> Nothing calls expand_shrinker_info() on nokmem either, so
>> shrinker_nr_max stays 0 and every memcg ends up with an empty map
>> (map_nr_max == 0).
>>
>> deferred_split_folio() hands a real memcg to __list_lru_add()
>> regardless of whether the lru is memcg aware, so the first THP queued
>> in a cgroup does set_shrinker_bit(memcg, nid, 0) and trips the bounds
>> check:
>>
>> WARNING: mm/shrinker.c:212 at set_shrinker_bit+0x7d/0x90, CPU#126
>> Call Trace:
>>   <TASK>
>>   deferred_split_folio+0x18c/0x220
>>   map_anon_folio_pmd_nopf+0xdd/0x130
>>   map_anon_folio_pmd_pf+0x14/0xb0
>>   do_huge_pmd_anonymous_page+0x1a1/0x620
>>   __handle_mm_fault+0xea9/0x10d0
>>   handle_mm_fault+0xe5/0x320
>>   do_user_addr_fault+0x1cc/0x870
>>   exc_page_fault+0x81/0x1b0
>>   asm_exc_page_fault+0x27/0x30
>>   </TASK>
>>
>> Harmless, the WARN_ON_ONCE() is what keeps the out of bounds unit[]
>> read from happening, but the id should not look valid in the first
>> place. Clear it before returning.
>>
>> Two other spots could paper over this: drop the id in
>> __list_lru_init() when nokmem turns memcg_aware off, or make
>> deferred_split_folio() pass NULL like list_lru_add_obj() does. Both
>> leave shrinker->id lying around for the next caller, so fix it where
>> the id is handed out.
>>
>> Fixes: fafaeceb89a5 ("mm: switch deferred split shrinker to list_lru")
>> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>> ---
>>   mm/shrinker.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/mm/shrinker.c b/mm/shrinker.c
>> index a70aab124a0e..7ec2a9704f6f 100644
>> --- a/mm/shrinker.c
>> +++ b/mm/shrinker.c
>> @@ -227,6 +227,8 @@ static int shrinker_memcg_alloc(struct shrinker *shrinker)
>>   {
>>   	int id;
>>   
>> +	shrinker->id = -1;
>> +
>>   	if (mem_cgroup_disabled())
>>   		return -ENOSYS;
>>   	if (mem_cgroup_kmem_disabled() && !(shrinker->flags & SHRINKER_NONSLAB))
>
> Thanks for the fix!
>
> I think the initialization would look better in shrinker_alloc()?


Well, we have to add macro if we move it to shrinker_alloc()


     shrinker->flags = flags | SHRINKER_ALLOCATED;
     shrinker->seeks = DEFAULT_SEEKS;

+#ifdef CONFIG_MEMCG
+        shrinker->id = -1;
+#endif


No strong preference for me :)



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mm/shrinker: fix bogus set_shrinker_bit() with cgroup.memory=nokmem
  2026-09-02  7:37 [PATCH] mm/shrinker: fix bogus set_shrinker_bit() with cgroup.memory=nokmem Jiayuan Chen
  2026-09-02 11:34 ` Usama Arif
@ 2026-09-02 15:40 ` Shakeel Butt
  1 sibling, 0 replies; 4+ messages in thread
From: Shakeel Butt @ 2026-09-02 15:40 UTC (permalink / raw)
  To: Jiayuan Chen
  Cc: linux-mm, Andrew Morton, Dave Chinner, Qi Zheng, Roman Gushchin,
	Muchun Song, Kairui Song, Johannes Weiner, Usama Arif,
	linux-kernel

On Wed, Sep 02, 2026 at 03:37:59PM +0800, Jiayuan Chen wrote:
> With cgroup.memory=nokmem, shrinker_memcg_alloc() bails out early and
> never allocates an id, so shrinker->id keeps the 0 it got from the
> kzalloc() in shrinker_alloc(). __list_lru_init() then copies that 0
> into lru->shrinker_id, where it looks like a valid bit index.
> 
> Nothing calls expand_shrinker_info() on nokmem either, so
> shrinker_nr_max stays 0 and every memcg ends up with an empty map
> (map_nr_max == 0).
> 
> deferred_split_folio() hands a real memcg to __list_lru_add()
> regardless of whether the lru is memcg aware, so the first THP queued
> in a cgroup does set_shrinker_bit(memcg, nid, 0) and trips the bounds
> check:
> 
> WARNING: mm/shrinker.c:212 at set_shrinker_bit+0x7d/0x90, CPU#126
> Call Trace:
>  <TASK>
>  deferred_split_folio+0x18c/0x220
>  map_anon_folio_pmd_nopf+0xdd/0x130
>  map_anon_folio_pmd_pf+0x14/0xb0
>  do_huge_pmd_anonymous_page+0x1a1/0x620
>  __handle_mm_fault+0xea9/0x10d0
>  handle_mm_fault+0xe5/0x320
>  do_user_addr_fault+0x1cc/0x870
>  exc_page_fault+0x81/0x1b0
>  asm_exc_page_fault+0x27/0x30
>  </TASK>
> 
> Harmless, the WARN_ON_ONCE() is what keeps the out of bounds unit[]
> read from happening, but the id should not look valid in the first
> place. Clear it before returning.
> 
> Two other spots could paper over this: drop the id in
> __list_lru_init() when nokmem turns memcg_aware off, or make
> deferred_split_folio() pass NULL like list_lru_add_obj() does. Both
> leave shrinker->id lying around for the next caller, so fix it where
> the id is handed out.
> 
> Fixes: fafaeceb89a5 ("mm: switch deferred split shrinker to list_lru")
> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>

Acked-by: Shakeel Butt <shakeel.butt@linux.dev>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-02 15:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-02  7:37 [PATCH] mm/shrinker: fix bogus set_shrinker_bit() with cgroup.memory=nokmem Jiayuan Chen
2026-09-02 11:34 ` Usama Arif
2026-09-02 11:44   ` Jiayuan Chen
2026-09-02 15:40 ` Shakeel Butt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox