Re: [PATCH] sched: flush plug in schedule_preempt_disabled() to prevent deadlock

[Ming Lei <tom.leiming@gmail.com>]

On Tue, May 12, 2026 at 02:40:21PM +0200, Peter Zijlstra wrote:
> On Tue, May 12, 2026 at 02:04:32PM +0200, Peter Zijlstra wrote:
> > On Tue, May 12, 2026 at 04:59:39PM +0800, Ming Lei wrote:
> > > On preemptible kernels, a deadlock can occur when a task with plugged IO
> > > calls schedule_preempt_disabled():
> > > 
> > >   schedule_preempt_disabled()
> > >     sched_preempt_enable_no_resched()  // preemption now enabled
> > >     schedule()                         // <-- preemption can happen here
> > >       sched_submit_work()
> > >         blk_flush_plug()
> > > 
> > > After sched_preempt_enable_no_resched() re-enables preemption, the task
> > > can be preempted (e.g., by a higher-priority RT task) before reaching
> > > blk_flush_plug() in sched_submit_work(). Since the task's state is
> > > already TASK_UNINTERRUPTIBLE (set by the mutex/rwsem slowpath caller),
> > > requests in current->plug remain unflushed for an unbounded time.
> > > 
> > > If another task depends on those plugged requests to make progress (e.g.,
> > > to release a lock the sleeping task needs), a deadlock results:
> > > 
> > >   - Task A (writeback worker): holds plugged IO, preempted before
> > >     flushing, stuck on run queue behind higher-priority work
> > >   - Task B: waiting for IO completion from Task A's plug, holds a lock
> > >     that Task A needs to be woken up
> > > 
> > > Both reported deadlocks involve mutex/rwsem slowpaths, which are the
> > > primary callers of schedule_preempt_disabled() with non-running task
> > > state.
> > > 
> > > Fix by flushing the plug in schedule_preempt_disabled() while
> > > preemption is still disabled. This ensures the plug is empty before the
> > > preemption window opens.
> > 
> > How is this different from any path calling schedule()? That would be
> > subject to exactly the same issue.
> > 
> > The patch cannot be correct.
> 
> Also, is there a reason io_schedule_prepare() has a blk_flush_plug()
> call?

It is added in Tejun's "[PATCHSET RFC] sched, jbd2: mark sleeps on journal->j_checkpoint_mutex as iowait":

https://lore.kernel.org/all/1477673892-28940-1-git-send-email-tj@kernel.org/#t

which fixes iowait accounting for ext4, meantime adds the model
"io_schedule_prepare() + schedule() + io_schedule_finish()", which actually
can avoid this kind issue easily because io_schedule_prepare() is called
in task running state.

For this f2fs issue, maybe it can be addressed by adding rwsem io variant
just like mutex_lock_io(), meantime iowait accounting is covered too.

> io_schedule()
>   token = io_schedule_prepare()
>     blk_flush_plug(current->plug, true);
>   schedule()
>     if (!task_is_running(tsk))
>       sched_submit_work()
>         blk_flush_plug(tsk->plug, true);
> 
> Why isn't the one in sched_submit_work() sufficient? This thing either
> needs a comment justifying its existence, or get removed.

This plug is originally added in 73c101011926 "block: initial patch for
on-stack per-task plugging") and commit a237c1c5bc5d "block: let io_schedule()
flush the plug inline" by Jens, when there isn't such preempt issue.

But it is moved out to sched_submit_work() latter in commit 9c40cef2b799
"sched: Move blk_schedule_flush_plug() out of __schedule()") by Thomas
Gleixner, when this issue starts to become likely.

If io_schedule_prepare() can be called in every iowait context, looks
blk_flush_plug() from sched_submit_work() may be removed.


Thanks,
Ming