[PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Josef Bacik]

cleanup_offline_cgwbs_workfn() drains a dying cgwb by calling
cleanup_offline_cgwb() until it returns false, with a cond_resched()
between passes.  On a CONFIG_PREEMPTION kernel that cond_resched() does
nothing: _cond_resched() is a plain "return 0", and under
PREEMPT_DYNAMIC the full and lazy modes disable it.  Since commit
7dadeaa6e851 ("sched: Further restrict the preemption modes") those are
the only two models on the architectures with PREEMPT_LAZY support,
arm64 and x86 among them, so the drain loop never reports a Tasks-RCU
quiescent state.

A worker draining a cgwb with millions of attached inodes runs for
minutes.  On a 6.18 arm64 host in lazy mode the cgwb worker drained one
dying cgroup's writeback domain for over 11 minutes.  A BPF program
unlink (bpf_trampoline_unlink_prog -> bpf_trampoline_update ->
unregister_ftrace_direct -> ftrace_shutdown -> synchronize_rcu_tasks())
waited on that grace period while holding the trampoline mutex, 42
tasks queued behind it in D state, and the hung task detector fired at
614 s and panicked the host.  Any BPF or ftrace detach during a long
drain inherits the drain's length.

Fix this by calling cond_resched_tasks_rcu_qs() so we do not stall out
anybody who calls sycnrhonize_rcu_tasks().  We put this in a do { } while
loop because if we have many small cgroups cleanup_offline_cgwb() will
return false and we will never call cond_resched_tasks_rcu_qs(), creating
the same problem.

Fixes: c22d70a162d3 ("writeback, cgroup: release dying cgwbs by switching attached inodes")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/bpf/9d444098-7c03-4163-af12-bd0a79a51443@paulmck-laptop/
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 mm/backing-dev.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index cecbcf9060a6..18e999053bae 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -910,8 +910,9 @@ static void cleanup_offline_cgwbs_workfn(struct work_struct *work)
 			continue;
 
 		spin_unlock_irq(&cgwb_lock);
-		while (cleanup_offline_cgwb(wb))
-			cond_resched();
+		do {
+			cond_resched_tasks_rcu_qs();
+		} while (cleanup_offline_cgwb(wb));
 		spin_lock_irq(&cgwb_lock);
 
 		wb_put(wb);

---
base-commit: 893e11787f78e43b534e252249ac3fff4d1333f8
change-id: 20260909-cgwb-tasks-rcu-qs-42341609f3e1

Best regards,
--  
Josef Bacik <josef@toxicpanda.com>

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Unbounded iteration under a spinlock in a list filtering loop.
- [High] O(N^2) list traversal and spinlock acquisitions when draining cgwbs.
- [High] Unbounded queueing of work items ignoring WB_FRN_MAX_IN_FLIGHT limit.
--

commit 6a4c1184ed4e555add5aa8d7af747c8f54a2713f
Author: Josef Bacik <josef@toxicpanda.com>

writeback: report a Tasks-RCU quiescent state per cgwb drain pass

This commit addresses an issue where draining a dying cgwb with many attached
inodes can run for minutes without reporting a Tasks-RCU quiescent state,
causing RCU stalls and watchdog panics. It fixes this by replacing a plain
cond_resched() with cond_resched_tasks_rcu_qs() in a loop as long as
cleanup_offline_cgwb() returns true.

Link: https://lore.kernel.org/bpf/9d444098-7c03-4163-af12-bd0a79a51443@paulmck-laptop/

> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index cecbcf9060a65..18e999053bae0 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -910,8 +910,9 @@ static void cleanup_offline_cgwbs_workfn(struct work_struct *work)
>  			continue;

[Severity: High]
This isn't a bug introduced by this patch, but doesn't this continue bypass
the lock-drop at the bottom of the loop?

If a user creates and destroys many cgroups, the offline_cgwbs list can grow
arbitrarily. Iterating over all of them under spin_lock_irq(&cgwb_lock) and
hitting this continue creates an unbounded IRQ-disabled critical section,
which could cause latency spikes and trigger the hard lockup watchdog.

>  
>  		spin_unlock_irq(&cgwb_lock);
> -		while (cleanup_offline_cgwb(wb))
> -			cond_resched();
> +		do {
> +			cond_resched_tasks_rcu_qs();
> +		} while (cleanup_offline_cgwb(wb));

[Severity: High]
This is a pre-existing issue, but does cleanup_offline_cgwb() suffer from an
algorithmic regression that causes the CPU livelocks this patch observes?

When cleanup_offline_cgwb() calls isw_prepare_wbs_switch():

fs/fs-writeback.c:isw_prepare_wbs_switch() {
	...
	list_for_each_entry(inode, list, i_io_list) {
		if (!inode_prepare_wbs_switch(inode, new_wb))
			continue;
	...
}

It processes inodes, sets I_WB_SWITCH, and queues them, but they are not
removed from the list synchronously. They are removed by an async worker
after a slow synchronize_rcu(). Because this worker is slow and we
tightly loop here, the next iteration traverses the same list from the
head, encountering all previously processed inodes again.

For each one, inode_prepare_wbs_switch() does this:

fs/fs-writeback.c:inode_prepare_wbs_switch() {
	spin_lock(&inode->i_lock);
	if (!(inode->i_sb->s_flags & SB_ACTIVE) ||
	    inode_state_read(inode) & (I_WB_SWITCH | I_FREEING | I_WILL_FREE) ||
	    inode_to_wb(inode) == new_wb) {
		spin_unlock(&inode->i_lock);
		goto out_unpin;
	}
	...
}

For N inodes, doesn't this result in an O(N^2) list traversal and O(N^2)
lock acquisitions, wasting CPU time during the drain?


[Severity: High]
This is also a pre-existing issue, but does this loop queue work items
without enforcing concurrency limits?

Inside cleanup_offline_cgwb():

fs/fs-writeback.c:cleanup_offline_cgwb() {
	...
	atomic_inc(&isw_nr_in_flight);
	...
	wb_queue_isw(new_wb, isw);
	...
}

This queues a new isw work item for every 125 inodes without checking if
isw_nr_in_flight exceeds WB_FRN_MAX_IN_FLIGHT (1024). For a large cgroup
with millions of inodes, doesn't this unconditionally queue thousands of
work items concurrently, bypassing the limit and potentially flooding the
workqueue?

>  		spin_lock_irq(&cgwb_lock);
>  
>  		wb_put(wb);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909-cgwb-tasks-rcu-qs-v1-1-967a7754771f@toxicpanda.com?part=1

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Tejun Heo]

(cc'ing Paul)

On Wed, Sep 09, 2026 at 06:01:07PM +0000, Josef Bacik wrote:
> cleanup_offline_cgwbs_workfn() drains a dying cgwb by calling
> cleanup_offline_cgwb() until it returns false, with a cond_resched()
> between passes.  On a CONFIG_PREEMPTION kernel that cond_resched() does
> nothing: _cond_resched() is a plain "return 0", and under
> PREEMPT_DYNAMIC the full and lazy modes disable it.  Since commit
> 7dadeaa6e851 ("sched: Further restrict the preemption modes") those are
> the only two models on the architectures with PREEMPT_LAZY support,
> arm64 and x86 among them, so the drain loop never reports a Tasks-RCU
> quiescent state.
> 
> A worker draining a cgwb with millions of attached inodes runs for
> minutes.  On a 6.18 arm64 host in lazy mode the cgwb worker drained one
> dying cgroup's writeback domain for over 11 minutes.  A BPF program
> unlink (bpf_trampoline_unlink_prog -> bpf_trampoline_update ->
> unregister_ftrace_direct -> ftrace_shutdown -> synchronize_rcu_tasks())
> waited on that grace period while holding the trampoline mutex, 42
> tasks queued behind it in D state, and the hung task detector fired at
> 614 s and panicked the host.  Any BPF or ftrace detach during a long
> drain inherits the drain's length.
> 
> Fix this by calling cond_resched_tasks_rcu_qs() so we do not stall out
> anybody who calls sycnrhonize_rcu_tasks().  We put this in a do { } while
> loop because if we have many small cgroups cleanup_offline_cgwb() will
> return false and we will never call cond_resched_tasks_rcu_qs(), creating
> the same problem.
> 
> Fixes: c22d70a162d3 ("writeback, cgroup: release dying cgwbs by switching attached inodes")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/bpf/9d444098-7c03-4163-af12-bd0a79a51443@paulmck-laptop/
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Acked-by: Tejun Heo <tj@kernel.org>

> ---
>  mm/backing-dev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index cecbcf9060a6..18e999053bae 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -910,8 +910,9 @@ static void cleanup_offline_cgwbs_workfn(struct work_struct *work)
>  			continue;
>  
>  		spin_unlock_irq(&cgwb_lock);
> -		while (cleanup_offline_cgwb(wb))
> -			cond_resched();
> +		do {
> +			cond_resched_tasks_rcu_qs();
> +		} while (cleanup_offline_cgwb(wb));

The patch looks fine but this overall seems fragile. cond_resched() was
already marking "stuff that can take too long" but we need to use
cond_resched_tasks_rcu_qs() if it can take *really* long. There gotta be a
way to make this more maintainable. If always doing tasks_rcu_qs from
cond_resched() is too expensive, can it be be gated behind something cheaper
e.g. some tick based test?

Thanks.

-- 
tejun

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Paul E. McKenney]

On Wed, Sep 09, 2026 at 08:16:54AM -1000, Tejun Heo wrote:
> (cc'ing Paul)
> 
> On Wed, Sep 09, 2026 at 06:01:07PM +0000, Josef Bacik wrote:
> > cleanup_offline_cgwbs_workfn() drains a dying cgwb by calling
> > cleanup_offline_cgwb() until it returns false, with a cond_resched()
> > between passes.  On a CONFIG_PREEMPTION kernel that cond_resched() does
> > nothing: _cond_resched() is a plain "return 0", and under
> > PREEMPT_DYNAMIC the full and lazy modes disable it.  Since commit
> > 7dadeaa6e851 ("sched: Further restrict the preemption modes") those are
> > the only two models on the architectures with PREEMPT_LAZY support,
> > arm64 and x86 among them, so the drain loop never reports a Tasks-RCU
> > quiescent state.
> > 
> > A worker draining a cgwb with millions of attached inodes runs for
> > minutes.  On a 6.18 arm64 host in lazy mode the cgwb worker drained one
> > dying cgroup's writeback domain for over 11 minutes.  A BPF program
> > unlink (bpf_trampoline_unlink_prog -> bpf_trampoline_update ->
> > unregister_ftrace_direct -> ftrace_shutdown -> synchronize_rcu_tasks())
> > waited on that grace period while holding the trampoline mutex, 42
> > tasks queued behind it in D state, and the hung task detector fired at
> > 614 s and panicked the host.  Any BPF or ftrace detach during a long
> > drain inherits the drain's length.
> > 
> > Fix this by calling cond_resched_tasks_rcu_qs() so we do not stall out
> > anybody who calls sycnrhonize_rcu_tasks().  We put this in a do { } while
> > loop because if we have many small cgroups cleanup_offline_cgwb() will
> > return false and we will never call cond_resched_tasks_rcu_qs(), creating
> > the same problem.
> > 
> > Fixes: c22d70a162d3 ("writeback, cgroup: release dying cgwbs by switching attached inodes")
> > Cc: stable@vger.kernel.org
> > Link: https://lore.kernel.org/bpf/9d444098-7c03-4163-af12-bd0a79a51443@paulmck-laptop/
> > Assisted-by: LLM
> > Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> 
> Acked-by: Tejun Heo <tj@kernel.org>
> 
> > ---
> >  mm/backing-dev.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> > index cecbcf9060a6..18e999053bae 100644
> > --- a/mm/backing-dev.c
> > +++ b/mm/backing-dev.c
> > @@ -910,8 +910,9 @@ static void cleanup_offline_cgwbs_workfn(struct work_struct *work)
> >  			continue;
> >  
> >  		spin_unlock_irq(&cgwb_lock);
> > -		while (cleanup_offline_cgwb(wb))
> > -			cond_resched();
> > +		do {
> > +			cond_resched_tasks_rcu_qs();
> > +		} while (cleanup_offline_cgwb(wb));
> 
> The patch looks fine but this overall seems fragile. cond_resched() was
> already marking "stuff that can take too long" but we need to use
> cond_resched_tasks_rcu_qs() if it can take *really* long. There gotta be a
> way to make this more maintainable. If always doing tasks_rcu_qs from
> cond_resched() is too expensive, can it be be gated behind something cheaper
> e.g. some tick based test?

This is the business end of cond_resched_tasks_rcu_qs() in preemptible
kernels (in which cond_resched() is nothingness):

# define rcu_tasks_classic_qs(t, preempt)				\
	do {								\
		if (!(preempt) && READ_ONCE((t)->rcu_tasks_holdout))	\
			WRITE_ONCE((t)->rcu_tasks_holdout, false);	\
	} while (0)

This is pretty lightweight.  Adding a jiffies check would likely make
it more expensive.

Or am I missing your point?

							Thanx, Paul

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Tejun Heo]

Hello,

On Wed, Sep 09, 2026 at 12:03:20PM -0700, Paul E. McKenney wrote:
> > The patch looks fine but this overall seems fragile. cond_resched() was
> > already marking "stuff that can take too long" but we need to use
> > cond_resched_tasks_rcu_qs() if it can take *really* long. There gotta be a
> > way to make this more maintainable. If always doing tasks_rcu_qs from
> > cond_resched() is too expensive, can it be be gated behind something cheaper
> > e.g. some tick based test?
> 
> This is the business end of cond_resched_tasks_rcu_qs() in preemptible
> kernels (in which cond_resched() is nothingness):
> 
> # define rcu_tasks_classic_qs(t, preempt)				\
> 	do {								\
> 		if (!(preempt) && READ_ONCE((t)->rcu_tasks_holdout))	\
> 			WRITE_ONCE((t)->rcu_tasks_holdout, false);	\
> 	} while (0)
> 
> This is pretty lightweight.  Adding a jiffies check would likely make
> it more expensive.
> 
> Or am I missing your point?

I found the following thread for why there is a separate variant for
cond_resched_tasks_rcu_qs():

  https://lkml.kernel.org/r/20180224151240.0d63a059@vmware.local.home

The rationale was that it'd make cond_resched() expensive, so I assumed it
was relatively heavy. If it already comes down to a single test, I'm not
sure having a separate interface makes a lot of sense. There isn't some
semantical difference between the two, right? Anything which takes long
enough needs to do the tasks rcu qs, and that is what we mark with
cond_resched().

Thanks.

-- 
tejun

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Paul E. McKenney]

On Wed, Sep 09, 2026 at 09:13:49AM -1000, Tejun Heo wrote:
> Hello,
> 
> On Wed, Sep 09, 2026 at 12:03:20PM -0700, Paul E. McKenney wrote:
> > > The patch looks fine but this overall seems fragile. cond_resched() was
> > > already marking "stuff that can take too long" but we need to use
> > > cond_resched_tasks_rcu_qs() if it can take *really* long. There gotta be a
> > > way to make this more maintainable. If always doing tasks_rcu_qs from
> > > cond_resched() is too expensive, can it be be gated behind something cheaper
> > > e.g. some tick based test?
> > 
> > This is the business end of cond_resched_tasks_rcu_qs() in preemptible
> > kernels (in which cond_resched() is nothingness):
> > 
> > # define rcu_tasks_classic_qs(t, preempt)				\
> > 	do {								\
> > 		if (!(preempt) && READ_ONCE((t)->rcu_tasks_holdout))	\
> > 			WRITE_ONCE((t)->rcu_tasks_holdout, false);	\
> > 	} while (0)
> > 
> > This is pretty lightweight.  Adding a jiffies check would likely make
> > it more expensive.
> > 
> > Or am I missing your point?
> 
> I found the following thread for why there is a separate variant for
> cond_resched_tasks_rcu_qs():
> 
>   https://lkml.kernel.org/r/20180224151240.0d63a059@vmware.local.home

Well, that was nine years ago.  ;-)

> The rationale was that it'd make cond_resched() expensive, so I assumed it
> was relatively heavy. If it already comes down to a single test, I'm not
> sure having a separate interface makes a lot of sense. There isn't some
> semantical difference between the two, right? Anything which takes long
> enough needs to do the tasks rcu qs, and that is what we mark with
> cond_resched().

And nine years later, I am still good with cond_resched() implying
cond_resched_tasks_rcu_qs().

The people wanting to get rid of cond_resched() might have a different
opinion, though.

							Thanx, Paul

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Josef Bacik]

On Wed, Sep 09, 2026 at 08:16:54AM -1000, Tejun Heo wrote:
> On Wed, Sep 09, 2026 at 06:01:07PM +0000, Josef Bacik wrote:
> > +		do {
> > +			cond_resched_tasks_rcu_qs();
> > +		} while (cleanup_offline_cgwb(wb));
> 
> The patch looks fine but this overall seems fragile. cond_resched() was
> already marking "stuff that can take too long" but we need to use
> cond_resched_tasks_rcu_qs() if it can take *really* long. There gotta be a
> way to make this more maintainable. If always doing tasks_rcu_qs from
> cond_resched() is too expensive, can it be be gated behind something cheaper
> e.g. some tick based test?

Yeah I agree, it is fragile.  Every long running loop in the kernel that
only does cond_resched() is a potential multi-minute synchronize_rcu_tasks()
stall now that cond_resched() is a no-op on the preemption models most
people actually run, and playing whack-a-mole with
cond_resched_tasks_rcu_qs() at each site as we trip over them isn't a great
long term answer.

I'm working on something more general so we don't have to sprinkle
cond_resched_tasks_rcu_qs() everywhere, but I expect it to be controversial
and it's going to take a while to shake out.  In the meantime these are real
bugs that are taking machines down today, so I'd like to get the targeted
fixes in and to stable while the broader discussion happens separately.

Thanks,

Josef

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Paul E. McKenney]

On Wed, Sep 09, 2026 at 07:38:20PM +0000, Josef Bacik wrote:
> On Wed, Sep 09, 2026 at 08:16:54AM -1000, Tejun Heo wrote:
> > On Wed, Sep 09, 2026 at 06:01:07PM +0000, Josef Bacik wrote:
> > > +		do {
> > > +			cond_resched_tasks_rcu_qs();
> > > +		} while (cleanup_offline_cgwb(wb));
> > 
> > The patch looks fine but this overall seems fragile. cond_resched() was
> > already marking "stuff that can take too long" but we need to use
> > cond_resched_tasks_rcu_qs() if it can take *really* long. There gotta be a
> > way to make this more maintainable. If always doing tasks_rcu_qs from
> > cond_resched() is too expensive, can it be be gated behind something cheaper
> > e.g. some tick based test?
> 
> Yeah I agree, it is fragile.  Every long running loop in the kernel that
> only does cond_resched() is a potential multi-minute synchronize_rcu_tasks()
> stall now that cond_resched() is a no-op on the preemption models most
> people actually run, and playing whack-a-mole with
> cond_resched_tasks_rcu_qs() at each site as we trip over them isn't a great
> long term answer.
> 
> I'm working on something more general so we don't have to sprinkle
> cond_resched_tasks_rcu_qs() everywhere, but I expect it to be controversial
> and it's going to take a while to shake out.  In the meantime these are real
> bugs that are taking machines down today, so I'd like to get the targeted
> fixes in and to stable while the broader discussion happens separately.

Looking forward to seeing what you come up with.

							Thanx, Paul

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Roman Gushchin]

Josef Bacik <josef@toxicpanda.com> writes:

> cleanup_offline_cgwbs_workfn() drains a dying cgwb by calling
> cleanup_offline_cgwb() until it returns false, with a cond_resched()
> between passes.  On a CONFIG_PREEMPTION kernel that cond_resched() does
> nothing: _cond_resched() is a plain "return 0", and under
> PREEMPT_DYNAMIC the full and lazy modes disable it.  Since commit
> 7dadeaa6e851 ("sched: Further restrict the preemption modes") those are
> the only two models on the architectures with PREEMPT_LAZY support,
> arm64 and x86 among them, so the drain loop never reports a Tasks-RCU
> quiescent state.
>
> A worker draining a cgwb with millions of attached inodes runs for
> minutes.  On a 6.18 arm64 host in lazy mode the cgwb worker drained one
> dying cgroup's writeback domain for over 11 minutes.  A BPF program
> unlink (bpf_trampoline_unlink_prog -> bpf_trampoline_update ->
> unregister_ftrace_direct -> ftrace_shutdown -> synchronize_rcu_tasks())
> waited on that grace period while holding the trampoline mutex, 42
> tasks queued behind it in D state, and the hung task detector fired at
> 614 s and panicked the host.  Any BPF or ftrace detach during a long
> drain inherits the drain's length.
>
> Fix this by calling cond_resched_tasks_rcu_qs() so we do not stall out
> anybody who calls sycnrhonize_rcu_tasks().  We put this in a do { } while
> loop because if we have many small cgroups cleanup_offline_cgwb() will
> return false and we will never call cond_resched_tasks_rcu_qs(), creating
> the same problem.
>
> Fixes: c22d70a162d3 ("writeback, cgroup: release dying cgwbs by switching attached inodes")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/bpf/9d444098-7c03-4163-af12-bd0a79a51443@paulmck-laptop/
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  mm/backing-dev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index cecbcf9060a6..18e999053bae 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -910,8 +910,9 @@ static void cleanup_offline_cgwbs_workfn(struct work_struct *work)
>  			continue;
>  
>  		spin_unlock_irq(&cgwb_lock);
> -		while (cleanup_offline_cgwb(wb))
> -			cond_resched();
> +		do {
> +			cond_resched_tasks_rcu_qs();
> +		} while (cleanup_offline_cgwb(wb));

Reviewed-by: Roman Gushchin <roman.gushchin@linux.dev>

Thanks!

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Jan Kara]

On Wed 09-09-26 18:01:07, Josef Bacik wrote:
> cleanup_offline_cgwbs_workfn() drains a dying cgwb by calling
> cleanup_offline_cgwb() until it returns false, with a cond_resched()
> between passes.  On a CONFIG_PREEMPTION kernel that cond_resched() does
> nothing: _cond_resched() is a plain "return 0", and under
> PREEMPT_DYNAMIC the full and lazy modes disable it.  Since commit
> 7dadeaa6e851 ("sched: Further restrict the preemption modes") those are
> the only two models on the architectures with PREEMPT_LAZY support,
> arm64 and x86 among them, so the drain loop never reports a Tasks-RCU
> quiescent state.
> 
> A worker draining a cgwb with millions of attached inodes runs for
> minutes.  On a 6.18 arm64 host in lazy mode the cgwb worker drained one
> dying cgroup's writeback domain for over 11 minutes.  A BPF program
> unlink (bpf_trampoline_unlink_prog -> bpf_trampoline_update ->
> unregister_ftrace_direct -> ftrace_shutdown -> synchronize_rcu_tasks())
> waited on that grace period while holding the trampoline mutex, 42
> tasks queued behind it in D state, and the hung task detector fired at
> 614 s and panicked the host.  Any BPF or ftrace detach during a long
> drain inherits the drain's length.
> 
> Fix this by calling cond_resched_tasks_rcu_qs() so we do not stall out
> anybody who calls sycnrhonize_rcu_tasks().  We put this in a do { } while
> loop because if we have many small cgroups cleanup_offline_cgwb() will
> return false and we will never call cond_resched_tasks_rcu_qs(), creating
> the same problem.
> 
> Fixes: c22d70a162d3 ("writeback, cgroup: release dying cgwbs by switching attached inodes")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/bpf/9d444098-7c03-4163-af12-bd0a79a51443@paulmck-laptop/
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>

Makes sense to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

And I fully agree about the fragility. It looks as an oversight that
cond_resched() doesn't imply RCU quiescent period after the recent
preemption overhaul.

								Honza

> ---
>  mm/backing-dev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index cecbcf9060a6..18e999053bae 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -910,8 +910,9 @@ static void cleanup_offline_cgwbs_workfn(struct work_struct *work)
>  			continue;
>  
>  		spin_unlock_irq(&cgwb_lock);
> -		while (cleanup_offline_cgwb(wb))
> -			cond_resched();
> +		do {
> +			cond_resched_tasks_rcu_qs();
> +		} while (cleanup_offline_cgwb(wb));
>  		spin_lock_irq(&cgwb_lock);
>  
>  		wb_put(wb);
> 
> ---
> base-commit: 893e11787f78e43b534e252249ac3fff4d1333f8
> change-id: 20260909-cgwb-tasks-rcu-qs-42341609f3e1
> 
> Best regards,
> --  
> Josef Bacik <josef@toxicpanda.com>
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Lorenzo Stoakes (ARM)]

+cc cgroup guys + paulmcr for rcu aspect + willy, pedro for fs stuff

On Wed, Sep 09, 2026 at 06:01:07PM +0000, Josef Bacik wrote:
> cleanup_offline_cgwbs_workfn() drains a dying cgwb by calling
> cleanup_offline_cgwb() until it returns false, with a cond_resched()
> between passes.  On a CONFIG_PREEMPTION kernel that cond_resched() does
> nothing: _cond_resched() is a plain "return 0", and under
> PREEMPT_DYNAMIC the full and lazy modes disable it.  Since commit
> 7dadeaa6e851 ("sched: Further restrict the preemption modes") those are
> the only two models on the architectures with PREEMPT_LAZY support,
> arm64 and x86 among them, so the drain loop never reports a Tasks-RCU
> quiescent state.

Hmm, I thought that for all intents and purposes cond_resched() was unimportant
and something we wanted to get rid of or it was silly to use?

So it seems very strange that it's so critical here.

I think there's probably a bigger algorithmic issue underlying this.

And I don't think cond_resched() is really relevant here:

Looking at my RCU notes it says that [rcu_tasks_kthread] polls and snapshots all
CPUs which have undergone _voluntary_ rescheduling, and quiescent state is
determined by that.

It tracks holdouts and then at 100-200ms it forces a context switch for those
holdouts.

But AFAIU cond_resched() isn't a _voluntary_ context switch from task RCU's
point of view.

So this whole bit is just wrong I think.

It's more so that no voluntary context switch occurs in a context where task RCU
is all you have for RCU.

Also actually I think the comment on cond_resched_rcu_qs() is out of date:

/**
 * cond_resched_tasks_rcu_qs - Report potential quiescent states to RCU
 *
 * This macro resembles cond_resched(), except that it is defined to
 * report potential quiescent states to RCU-tasks even if the cond_resched()
 * machinery were to be shut off, as some advocate for PREEMPTION kernels.
 */

Commit bde6c3aa9930 ("rcu: Provide cond_resched_rcu_qs() to force quiescent
states in long loops") references reluctance to 'instrument' cond_resched().

So it was provided as a means of actually achieving what you need here in a long
loop.

Paul - I think maybe it's worth updating this? And maybe even renaming it? :)

>
> A worker draining a cgwb with millions of attached inodes runs for
> minutes.  On a 6.18 arm64 host in lazy mode the cgwb worker drained one
> dying cgroup's writeback domain for over 11 minutes.  A BPF program

That really sounds like something bigger is broken here... why is so much queued
up by this point with no voluntary context switches to clear any of it down?

> unlink (bpf_trampoline_unlink_prog -> bpf_trampoline_update ->
> unregister_ftrace_direct -> ftrace_shutdown -> synchronize_rcu_tasks())
> waited on that grace period while holding the trampoline mutex, 42
> tasks queued behind it in D state, and the hung task detector fired at
> 614 s and panicked the host.  Any BPF or ftrace detach during a long
> drain inherits the drain's length.

Yikes...

>
> Fix this by calling cond_resched_tasks_rcu_qs() so we do not stall out
> anybody who calls sycnrhonize_rcu_tasks().  We put this in a do { } while
> loop because if we have many small cgroups cleanup_offline_cgwb() will
> return false and we will never call cond_resched_tasks_rcu_qs(), creating
> the same problem.

So I think this looks fine as an immediate, backportable fix.

But I think we need to look deeper into the real cause here.

I've not looked at the writeback code for a while, and am certainly not au-fait
with the cgroup side of it, so asking the LLM:

" - fs/fs-writeback.c:726 isw_prepare_wbs_switch() walks b_attached from the
    head on every pass. Inodes already marked I_WB_SWITCH stay on the old wb's
    list until the consumer moves them at fs/fs-writeback.c:465.

  - Each pass isolates at most about 120 inodes, per fs/fs-writeback.c:269, but
    revisits every already-prepared inode, each costing cgroup_writeback_pin()
    plus an i_lock round trip at fs/fs-writeback.c:624, all under wb->list_lock
    with preemption disabled.

  - The consumer at fs/fs-writeback.c:616 does a synchronize_rcu() per wave, so
    the producer outruns it and the rescans grow. With millions of inodes that
    is quadratic, and it explains minutes of CPU-bound worker time without any
    preemption-mode change.

  A follow-up that makes the producer wait for in-flight switches, or that has
  cleanup_offline_cgwb() resume from where it stopped, would fix both the
  runtime and the Tasks-RCU stall. "

So this DEFINITELY needs some follow up attention I think.

Anyway for the fix as written, it seems sensible and the right way to
immediately address this, so:

Acked-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

>
> Fixes: c22d70a162d3 ("writeback, cgroup: release dying cgwbs by switching attached inodes")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/bpf/9d444098-7c03-4163-af12-bd0a79a51443@paulmck-laptop/
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  mm/backing-dev.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index cecbcf9060a6..18e999053bae 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -910,8 +910,9 @@ static void cleanup_offline_cgwbs_workfn(struct work_struct *work)
>  			continue;
>
>  		spin_unlock_irq(&cgwb_lock);
> -		while (cleanup_offline_cgwb(wb))
> -			cond_resched();
> +		do {
> +			cond_resched_tasks_rcu_qs();
> +		} while (cleanup_offline_cgwb(wb));
>  		spin_lock_irq(&cgwb_lock);
>
>  		wb_put(wb);
>
> ---
> base-commit: 893e11787f78e43b534e252249ac3fff4d1333f8
> change-id: 20260909-cgwb-tasks-rcu-qs-42341609f3e1
>
> Best regards,
> --
> Josef Bacik <josef@toxicpanda.com>
>

--
Cheers, Lorenzo

Re: [PATCH] writeback: report a Tasks-RCU quiescent state per cgwb drain pass

[Lorenzo Stoakes (ARM)]

And of course neomutt managed to hide all the replies from me so I hadn't
realised it had been replied to :)

Anyway hopefully relatively in line with the assessment of others on the
thread...

--
Cheers, Lorenzo