From: Mike Snitzer <snitzer@redhat.com>
To: Bart Van Assche <Bart.VanAssche@wdc.com>
Cc: "axboe@kernel.dk" <axboe@kernel.dk>,
"dm-devel@redhat.com" <dm-devel@redhat.com>,
"hare@suse.de" <hare@suse.de>,
"tom.leiming@gmail.com" <tom.leiming@gmail.com>,
"djeffery@redhat.com" <djeffery@redhat.com>,
"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>
Subject: Re: [for-4.16 PATCH v5 0/4] block/dm: allow DM to defer blk_register_queue() until ready
Date: Mon, 15 Jan 2018 17:15:11 -0500 [thread overview]
Message-ID: <20180115221511.GA25858@redhat.com> (raw)
In-Reply-To: <1516036612.10386.2.camel@wdc.com>
On Mon, Jan 15 2018 at 12:16pm -0500,
Bart Van Assche <Bart.VanAssche@wdc.com> wrote:
> On Fri, 2018-01-12 at 10:06 -0500, Mike Snitzer wrote:
> > I'm submitting this v5 with more feeling now ;)
>
> Hello Mike,
>
> Have these patches been tested with lockdep enabled? The following appeared in
> the kernel log when after I started testing Jens' for-next tree of this morning:
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 4.15.0-rc7-dbg+ #1 Not tainted
> ------------------------------------------------------
> 02-mq/1211 is trying to acquire lock:
> (&q->sysfs_lock){+.+.}, at: [<000000008b65bdad>] queue_attr_store+0x35/0x80
>
> but task is already holding lock:
> (kn->count#213){++++}, at: [<000000007a18ad18>] kernfs_fop_write+0xe5/0x190
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #1 (kn->count#213){++++}:
> kernfs_remove+0x1a/0x30
> kobject_del.part.3+0xe/0x40
> blk_unregister_queue+0xa7/0xe0
> del_gendisk+0x12f/0x260
> sd_remove+0x58/0xc0 [sd_mod]
> device_release_driver_internal+0x15a/0x220
> bus_remove_device+0xf4/0x170
> device_del+0x12f/0x330
> __scsi_remove_device+0xef/0x120 [scsi_mod]
> scsi_forget_host+0x1b/0x60 [scsi_mod]
> scsi_remove_host+0x6f/0x110 [scsi_mod]
> 0xffffffffc09ed6e4
> process_one_work+0x21c/0x6d0
> worker_thread+0x35/0x380
> kthread+0x117/0x130
> ret_from_fork+0x24/0x30
>
> -> #0 (&q->sysfs_lock){+.+.}:
> __mutex_lock+0x6c/0x9e0
> queue_attr_store+0x35/0x80
> kernfs_fop_write+0x109/0x190
> __vfs_write+0x1e/0x130
> vfs_write+0xb9/0x1b0
> SyS_write+0x40/0xa0
> entry_SYSCALL_64_fastpath+0x23/0x9a
>
> other info that might help us debug this:
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(kn->count#213);
> lock(&q->sysfs_lock);
> lock(kn->count#213);
> lock(&q->sysfs_lock);
>
> *** DEADLOCK ***
>
> 3 locks held by 02-mq/1211:
> #0: (sb_writers#6){.+.+}, at: [<00000000afdb61d3>] vfs_write+0x17f/0x1b0
> #1: (&of->mutex){+.+.}, at: [<00000000b291cabb>] kernfs_fop_write+0xdd/0x190
> #2: (kn->count#213){++++}, at: [<000000007a18ad18>] kernfs_fop_write+0xe5/0x190
sysfs write op calls kernfs_fop_write which takes:
of->mutex then kn->count#213 (no idea what that is)
then q->sysfs_lock (via queue_attr_store)
vs
blk_unregister_queue takes:
q->sysfs_lock then
kernfs_mutex (via kernfs_remove)
seems lockdep thinks "kernfs_mutex" is "kn->count#213"?
Feels like lockdep code in fs/kernfs/file.c and fs/kernfs/dir.c is
triggering false positives.. because these seem like different kernfs
locks yet they are reported as "kn->count#213".
Certainly feeling out of my depth with kernfs's locking though.
Mike
next prev parent reply other threads:[~2018-01-15 22:15 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-12 15:06 [for-4.16 PATCH v5 0/4] block/dm: allow DM to defer blk_register_queue() until ready Mike Snitzer
2018-01-12 15:06 ` [for-4.16 PATCH v5 1/4] block: only bdi_unregister() in del_gendisk() if !GENHD_FL_HIDDEN Mike Snitzer
2018-01-12 15:06 ` [for-4.16 PATCH v5 2/4] block: properly protect the 'queue' kobj in blk_unregister_queue Mike Snitzer
2018-01-12 15:17 ` Ming Lei
2018-01-12 15:29 ` Mike Snitzer
2018-01-12 16:03 ` [for-4.16 PATCH v6 " Mike Snitzer
2018-01-13 12:57 ` Ming Lei
2018-01-12 15:06 ` [for-4.16 PATCH v5 3/4] block: allow gendisk's request_queue registration to be deferred Mike Snitzer
2018-01-12 15:06 ` [for-4.16 PATCH v5 4/4] dm: fix incomplete request_queue initialization Mike Snitzer
2018-01-12 16:13 ` [for-4.16 PATCH v5 0/4] block/dm: allow DM to defer blk_register_queue() until ready Mike Snitzer
2018-01-15 17:16 ` Bart Van Assche
2018-01-15 17:29 ` Mike Snitzer
2018-01-15 17:36 ` Bart Van Assche
2018-01-15 17:48 ` Mike Snitzer
2018-01-15 17:51 ` Bart Van Assche
2018-01-15 17:57 ` Mike Snitzer
2018-01-15 22:15 ` Mike Snitzer [this message]
2018-01-15 22:51 ` Bart Van Assche
2018-01-15 23:10 ` Mike Snitzer
2018-01-15 23:13 ` Mike Snitzer
2018-01-16 2:21 ` Ming Lei
2018-01-16 18:19 ` Bart Van Assche
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180115221511.GA25858@redhat.com \
--to=snitzer@redhat.com \
--cc=Bart.VanAssche@wdc.com \
--cc=axboe@kernel.dk \
--cc=djeffery@redhat.com \
--cc=dm-devel@redhat.com \
--cc=hare@suse.de \
--cc=linux-block@vger.kernel.org \
--cc=tom.leiming@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).