| Message ID | 20241030124240.230610-6-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block: freeze/unfreeze lockdep fixes | expand |
On Wed, Oct 30, 2024 at 08:42:37PM +0800, Ming Lei wrote: > --- a/block/elevator.c > +++ b/block/elevator.c > @@ -598,13 +598,17 @@ void elevator_init_mq(struct request_queue *q) > * drain any dispatch activities originated from passthrough > * requests, then no need to quiesce queue which may add long boot > * latency, especially when lots of disks are involved. > + * > + * Disk isn't added yet, so verifying queue lock only manually. > */ > - blk_mq_freeze_queue(q); > + blk_mq_freeze_queue_non_owner(q); > + blk_freeze_acquire_lock(q, true, false); > blk_mq_cancel_work_sync(q); > > err = blk_mq_init_sched(q, e); > > - blk_mq_unfreeze_queue(q); > + blk_unfreeze_release_lock(q, true, false); > + blk_mq_unfreeze_queue_non_owner(q); Why do we need to free at all from the add_disk case? The passthrough command should never hit the elevator, or am I missing something?
On Wed, Oct 30, 2024 at 03:46:52PM +0100, Christoph Hellwig wrote: > On Wed, Oct 30, 2024 at 08:42:37PM +0800, Ming Lei wrote: > > --- a/block/elevator.c > > +++ b/block/elevator.c > > @@ -598,13 +598,17 @@ void elevator_init_mq(struct request_queue *q) > > * drain any dispatch activities originated from passthrough > > * requests, then no need to quiesce queue which may add long boot > > * latency, especially when lots of disks are involved. > > + * > > + * Disk isn't added yet, so verifying queue lock only manually. > > */ > > - blk_mq_freeze_queue(q); > > + blk_mq_freeze_queue_non_owner(q); > > + blk_freeze_acquire_lock(q, true, false); > > blk_mq_cancel_work_sync(q); > > > > err = blk_mq_init_sched(q, e); > > > > - blk_mq_unfreeze_queue(q); > > + blk_unfreeze_release_lock(q, true, false); > > + blk_mq_unfreeze_queue_non_owner(q); > > Why do we need to free at all from the add_disk case? The passthrough > command should never hit the elevator, or am I missing something? In theory the queue needn't to be frozen here, but both FS IO and PT req share common blk-mq code, in which q->elevator is often referenced. Thanks, Ming
diff --git a/block/elevator.c b/block/elevator.c index f169f4bae917..a02bf911d3ca 100644 --- a/block/elevator.c +++ b/block/elevator.c @@ -598,13 +598,17 @@ void elevator_init_mq(struct request_queue *q) * drain any dispatch activities originated from passthrough * requests, then no need to quiesce queue which may add long boot * latency, especially when lots of disks are involved. + * + * Disk isn't added yet, so verifying queue lock only manually. */ - blk_mq_freeze_queue(q); + blk_mq_freeze_queue_non_owner(q); + blk_freeze_acquire_lock(q, true, false); blk_mq_cancel_work_sync(q); err = blk_mq_init_sched(q, e); - blk_mq_unfreeze_queue(q); + blk_unfreeze_release_lock(q, true, false); + blk_mq_unfreeze_queue_non_owner(q); if (err) { pr_warn("\"%s\" elevator initialization failed, "
elevator_init_mq() is only called at the entry of add_disk_fwnode() when disk IO isn't allowed yet. So not verify io lock(q->io_lockdep_map) for freeze & unfreeze in elevator_init_mq(). Reported-by: Marek Szyprowski <m.szyprowski@samsung.com> Reported-by: Lai Yi <yi1.lai@linux.intel.com> Fixes: f1be1788a32e ("block: model freeze & enter queue as lock for supporting lockdep") Signed-off-by: Ming Lei <ming.lei@redhat.com> --- block/elevator.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)