linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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).