On Thu, Apr 03, 2025 at 07:44:27AM +0200, Christoph Hellwig wrote:
> On Thu, Apr 03, 2025 at 10:52:08AM +0800, Ming Lei wrote:
> > Add blk_mq_enter_no_io() and blk_mq_exit_no_io() for preventing queue
> > from handling any FS or passthrough IO, meantime the queue is kept in
> > non-freeze state.
>
> How does that differ from the actual freeze? Please document that
> clearly in the commit log and in kerneldoc comments, and do an analysis
> of which callers should do the full freeze and which the limited I/O
> freeze.
>
> Also the name is really unfortunate - no_io has a very clear connotation
> for memory allocations, so this should be using something else.
>
> > Also add two variants of memsave version, since no fs_reclaim is allowed
> > in case of blk_mq_enter_no_io().
>
> Please explain why.
>
>
> > index ae8494d88897..d117fa18b394 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -222,8 +222,7 @@ bool __blk_mq_unfreeze_queue(struct request_queue *q, bool force_atomic)
> > bool unfreeze;
> >
> > mutex_lock(&q->mq_freeze_lock);
> > - if (force_atomic)
> > - q->q_usage_counter.data->force_atomic = true;
> > + q->q_usage_counter.data->force_atomic = force_atomic;
> > q->mq_freeze_depth--;
> > WARN_ON_ONCE(q->mq_freeze_depth < 0);
> > if (!q->mq_freeze_depth) {
>
> This is a completely unrelated cleanup.
>
> > +void blk_mq_enter_no_io(struct request_queue *q)
> > +{
> > + blk_mq_freeze_queue_nomemsave(q);
> > + q->no_io = true;
> > + if (__blk_mq_unfreeze_queue(q, true))
> > + blk_unfreeze_release_lock(q);
>
> So this freezes the queue, sets a flag to not do I/O then unfreezes
> it. So AFAIK it just is a freeze without the automatic recursion.
>
> But maybe I'm missing something?
Yeah, looks lockdep modeling for blk_mq_enter_no_io() is wrong, and the
part in bio_enter_queue() is missed.
So this approach doesn't work.
Now the dependency between freeze lock and elevator lock looks one trouble,
such as [1], which is one real deadlock risk.
And there should be more.
[1] https://lore.kernel.org/linux-block/7755.1743228130@turing-police/#tReviewed-by
Thanks,
Ming
