Re: [zswap?] BUG: sleeping function called from invalid context at kernel/cgroup/rstat.c:421

[Shakeel Butt <shakeel.butt@linux.dev>]

On Thu, Jun 11, 2026 at 06:34:15PM +0000, Roman Gushchin wrote:
> Shakeel Butt <shakeel.butt@linux.dev> writes:
> 
> > On Wed, Jun 10, 2026 at 11:38:29AM -0700, Nhat Pham wrote:
> >> On Wed, Jun 10, 2026 at 10:31 AM Shakeel Butt <shakeel.butt@linux.dev> wrote:
> >> >
> >> > +Roman, Qi
> >> >
> >> > On Wed, Jun 10, 2026 at 09:38:03AM -0700, Nhat Pham wrote:
> >> > > On Wed, Jun 10, 2026 at 9:05 AM Zenghui Yu <zenghui.yu@linux.dev> wrote:
> >> > >
> >> > > Thanks for reporting, Zenghui.
> >> > >
> >> > >
> >> > > >
> >> > > > Hi all,
> >> > > >
> >> > > > The following splat was triggered on the mainline kernel:
> >> > > >
> >> > > >  BUG: sleeping function called from invalid context at kernel/cgroup/rstat.c:421
> >> > > >  in_atomic(): 0, irqs_disabled(): 0, non_block: 0, pid: 1126, name: cat
> >> > > >  preempt_count: 0, expected: 0
> >> > > >  RCU nest depth: 1, expected: 0
> >> > > >  CPU: 7 UID: 0 PID: 1126 Comm: cat Kdump: loaded Not tainted 7.1.0-rc7-00056-gacb7500801e9-dirty #304 PREEMPT
> >> > > >  Hardware name: QEMU QEMU Virtual Machine, BIOS edk2-stable202408-prebuilt.qemu.org 08/13/2024
> >> > > >  Call trace:
> >> > > >   show_stack+0x18/0x24 (C)
> >> > > >   dump_stack_lvl+0x78/0x90
> >> > > >   dump_stack+0x18/0x24
> >> > > >   __might_resched+0x114/0x170
> >> > > >   __might_sleep+0x48/0x98
> >> > > >   css_rstat_flush+0x54/0x564
> >> > > >   mem_cgroup_flush_stats+0x9c/0xb0
> >> > > >   zswap_shrinker_count+0xe4/0x1e4
> >> > > >   shrinker_debugfs_count_show+0xd8/0x268
> >> > >
> >> > > Ah, this seems a bit tricky.
> >> > >
> >> > > Seems like shrinker_debugfs_count_show() is invoking
> >> > > zswap_shrinker_count() in rcu_read_section(). zswap_shrinker_count()
> >> > > triggers a stats flushing, which might sleep. Not ideal.
> >> > >
> >> > > Is the rcu_read_section() here to protect memcg or shrinker? For
> >> > > memcg, i dont think it's necessary, no? mem_cgroup_iter() pins the
> >> > > memcg before returning.
> >> > >
> >> > > (memcg maintainers please fact check me).
> >> >
> >> > mem_cgroup_iter() handles the lifetime of memcg, so there is no need for rcu
> >> > read section for memcg.
> >> >
> >> > >
> >> > > If this is for the shrinker think this needs to follow shrink_slab()'s pattern.:
> >> > >
> >> > > rcu_read_lock();
> >> > > list_for_each_entry_rcu(shrinker, &shrinker_list, list)
> >> > > {
> >> > >     if (!shrinker_try_get(shrinker))
> >> > >         continue;
> >> > >     rcu_read_unlock();
> >> > > }
> >> > >
> >> > > But OTOH, doesn't seem like rcu_read_section() is what keeping it safe:
> >> >
> >> > Shouldn't the caller already holds the reference to the shrinker which it is
> >> > giving to this function? Does debugfs file entry holds a reference to the
> >> > shrinker which it is giving.
> >> >
> >> > After looking at shrinker_free(), it has call_rcu(&shrinker->rcu,
> >> > shrinker_free_rcu_cb), so this rcu read section is against that.
> >> >
> >> > I think we can simply use shrinker_try_get() here as Nhat said.
> >> 
> >> Hmm, so is this unsafe even with the current rcu shennanigans? What's
> >> stopping shrinker to be freed by that callback before we enter
> >> rcu_read_section()?
> >> 
> >> Seems like this is just implicitly correct - shrinker_debugfs_detach()
> >> and shrinker_debugfs_remove() happens before call_rcu(&shrinker->rcu,
> >> shrinker_free_rcu_cb);, so if you're reading this file, then it's
> >> before shrinker_free_rcu_cb() is even registered?
> >> 
> >> Do we still need rcu or shrinker_try_get() here?
> >
> > I think you are right that we don't need rcu or shrinker_try_get() but it is
> > more about an active debugfs file reader. Suppose we are sleeping within rstat
> > flush from shrinker_debugfs_count_show() and there is a parallel
> > shrinker_debugfs_remove() call.
> >
> > shrinker_debugfs_remove calls debugfs_remove_recursive and deep in the stack
> > there is a call wait_for_completion(&fsd->active_users_drained) which will wait
> > for active users, one of which is sleeping within rstat flush.
> >
> > So, let's simply remove rcu read here.
> 
> +1 to this. How about this version?
> 

Thanks, Andrew already picked up
https://lore.kernel.org/all/20260610232048.62930-1-shakeel.butt@linux.dev/