linux-kernel - Re: 6.15/regression/bisected - lockdep warning: circular locking dependency detected when plugging USB stick after ffa1e7ada456

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250723010336.2793-1-hdanton@sina.com>
Date: Wed, 23 Jul 2025 09:03:34 +0800
From: Hillf Danton <hdanton@...a.com>
To: Mikhail Gavrilov <mikhail.v.gavrilov@...il.com>
Cc: thomas.hellstrom@...ux.intel.com,
	axboe@...nel.dk,
	linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Ming Lei <ming.lei@...hat.com>,
	regressions@...ts.linux.dev
Subject: Re: 6.15/regression/bisected - lockdep warning: circular locking dependency detected when plugging USB stick after ffa1e7ada456

On Tue, 22 Jul 2025 12:11:36 +0500 Mikhail Gavrilov wrote:
> On Tue, Jul 22, 2025 at 5:51 AM Hillf Danton <hdanton@...a.com> 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);
 
--