On Tue, 22 Jul 2025 12:11:36 +0500 Mikhail Gavrilov wrote:
> On Tue, Jul 22, 2025 at 5:51 AM Hillf Danton <hdanton@xxxxxxxx> wrote:
> >
> > Try the diff that serializes elevator_change() with q->elevator_lock if
> > reproducer is available.
> >
> > --- x/block/elevator.c
> > +++ y/block/elevator.c
> > @@ -661,6 +661,7 @@ static int elevator_change(struct reques
> > unsigned int memflags;
> > int ret = 0;
> >
> > + /* updaters should be serialized */
> > lockdep_assert_held(&q->tag_set->update_nr_hwq_lock);
> >
> > memflags = blk_mq_freeze_queue(q);
> > @@ -674,11 +675,11 @@ static int elevator_change(struct reques
> > * Disk isn't added yet, so verifying queue lock only manually.
> > */
> > blk_mq_cancel_work_sync(q);
> > + blk_mq_unfreeze_queue(q, memflags);
> > mutex_lock(&q->elevator_lock);
> > if (!(q->elevator && elevator_match(q->elevator->type, ctx->name)))
> > ret = elevator_switch(q, ctx);
> > mutex_unlock(&q->elevator_lock);
> > - blk_mq_unfreeze_queue(q, memflags);
> > if (!ret)
> > ret = elevator_change_done(q, ctx);
> >
>
> Hi Hillf,
>
> Thanks for the patch.
>
> I tested your proposed diff that serializes elevator_change() with
> q->elevator_lock. Unfortunately, instead of the previous lockdep
> warning, I'm now seeing a soft lockup warning.
>
> Here is the relevant excerpt from the kernel log:
>
> [ 78.573292] sd 6:0:0:0: [sda] Assuming drive cache: write through
> [ 78.581496] ------------[ cut here ]------------
> [ 78.581507] WARNING: CPU: 7 PID: 300 at block/elevator.c:578
> elevator_switch+0x512/0x630
>
> This happens after plugging in a USB flash stick (sd 6:0:0:0) with the
> patched kernel.
>
> Full dmesg trace is attached below.
>
> Let me know if you'd like me to try additional debugging or patches.
>
> Thanks for looking into this!
In order to cure the deadlock, queue is thawed before switching elevator,
so lets see what comes out with that warning ignored.
--- x/block/elevator.c
+++ y/block/elevator.c
@@ -575,7 +575,6 @@ static int elevator_switch(struct reques
struct elevator_type *new_e = NULL;
int ret = 0;
- WARN_ON_ONCE(q->mq_freeze_depth == 0);
lockdep_assert_held(&q->elevator_lock);
if (strncmp(ctx->name, "none", 4)) {
@@ -661,6 +660,7 @@ static int elevator_change(struct reques
unsigned int memflags;
int ret = 0;
+ /* updaters should be serialized */
lockdep_assert_held(&q->tag_set->update_nr_hwq_lock);
memflags = blk_mq_freeze_queue(q);
@@ -674,11 +674,11 @@ static int elevator_change(struct reques
* Disk isn't added yet, so verifying queue lock only manually.
*/
blk_mq_cancel_work_sync(q);
+ blk_mq_unfreeze_queue(q, memflags);
mutex_lock(&q->elevator_lock);
if (!(q->elevator && elevator_match(q->elevator->type, ctx->name)))
ret = elevator_switch(q, ctx);
mutex_unlock(&q->elevator_lock);
- blk_mq_unfreeze_queue(q, memflags);
if (!ret)
ret = elevator_change_done(q, ctx);
--
