From patchwork Fri Apr 24 02:09:32 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: NeilBrown X-Patchwork-Id: 6266221 X-Patchwork-Delegate: snitzer@redhat.com Return-Path: X-Original-To: patchwork-dm-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 581C29F1C4 for ; Fri, 24 Apr 2015 02:15:04 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 67E12201B9 for ; Fri, 24 Apr 2015 02:15:03 +0000 (UTC) Received: from mx3-phx2.redhat.com (mx3-phx2.redhat.com [209.132.183.24]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A67CA2017E for ; Fri, 24 Apr 2015 02:15:01 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by mx3-phx2.redhat.com (8.13.8/8.13.8) with ESMTP id t3O29j8h030407; Thu, 23 Apr 2015 22:09:47 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id t3O29isV029215 for ; Thu, 23 Apr 2015 22:09:44 -0400 Received: from mx1.redhat.com (ext-mx11.extmail.prod.ext.phx2.redhat.com [10.5.110.16]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t3O29hSW000796 for ; Thu, 23 Apr 2015 22:09:43 -0400 Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t3O29f8a009969 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL) for ; Thu, 23 Apr 2015 22:09:42 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id E628AAB1C; Fri, 24 Apr 2015 02:09:40 +0000 (UTC) Date: Fri, 24 Apr 2015 12:09:32 +1000 From: NeilBrown To: Christoph Hellwig Message-ID: <20150424120932.3d554638@notabene.brown> In-Reply-To: <20150423161051.GA18971@lst.de> References: <20150414171537.GH25394@azat> <20150423160551.45345f96@notabene.brown> <20150423073724.GA8139@lst.de> <20150423180314.367c0876@notabene.brown> <20150423161051.GA18971@lst.de> MIME-Version: 1.0 X-RedHat-Spam-Score: -6.909 (BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, URIBL_BLOCKED) 195.135.220.15 cantor2.suse.de 195.135.220.15 cantor2.suse.de X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Scanned-By: MIMEDefang 2.68 on 10.5.110.16 X-loop: dm-devel@redhat.com Cc: Jens Axboe , Jan Kara , Azat Khuzhin , "Kernel.org-Linux-RAID" , dm-devel@redhat.com, Guoqing Jiang , Tejun Heo Subject: Re: [dm-devel] Bisected, with rfc/patch - was Re: BUG: unable to handle kernel NULL pointer dereference at sysfs_do_create_link_sd (after mdadm) X-BeenThere: dm-devel@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk Reply-To: device-mapper development List-Id: device-mapper development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dm-devel-bounces@redhat.com Errors-To: dm-devel-bounces@redhat.com X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, 23 Apr 2015 18:10:51 +0200 Christoph Hellwig wrote: > On Thu, Apr 23, 2015 at 06:03:14PM +1000, NeilBrown wrote: > > On Thu, 23 Apr 2015 09:37:24 +0200 Christoph Hellwig wrote: > > > > > Plase fix your device name lifetimes. > > > > Any chance you could be more explicit? > > > > The commit you identified doesn't seem to help much - md and dm are quite > > different in this area. > > > > It seems that it is no longer safe to call 'add_disk' between calling > > 'del_gendisk' and bdi_destroy being called. How can I find out if I am in > > that window, or wait for bdi_destroy to be called? > > The bdi is only around if the device is open, either through a device > node, or through a blkdev_get from a file system. If you get duplicate > names that means you're trying to allocate a new gendisk while the old > one is still around. > > In theory you're fine once the device gets ->release called. In practice .... the put_disk() shortly after the ->release call in __blkdev_put() is what ultimately releases the name of the bdi - at least in the cases where I get a crash. > > Except that we can hold sysfs reference to the qeue, eww. So for now > try to follow the dm model, but I'll need to add a callback to the > queue called once the request_queue actually is released for this. I'm pretty sure that the md code is already as close to the "dm model" as it meaningfully can be. If I move bdi_destroy out of blk_release_queue (which really think is too later) and place it in blk_cleanup_queue (which seems a credible place for it), and then move the blk_cleanup_queue call in md_free up before the del_gendisk() call (which is probably the right thing to do anyway, though dm has the same order that md currently has) then I don't get any crashes and I'm almost convince it is correct... Thoughts? Thanks, NeilBrown --- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel diff --git a/block/blk-core.c b/block/blk-core.c index 794c3e7f01cf..66406474f0c4 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -552,6 +552,8 @@ void blk_cleanup_queue(struct request_queue *q) q->queue_lock = &q->__queue_lock; spin_unlock_irq(lock); + bdi_destroy(&q->backing_dev_info); + /* @q is and will stay empty, shutdown and put */ blk_put_queue(q); } diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c index faaf36ade7eb..2b8fd302f677 100644 --- a/block/blk-sysfs.c +++ b/block/blk-sysfs.c @@ -522,8 +522,6 @@ static void blk_release_queue(struct kobject *kobj) blk_trace_shutdown(q); - bdi_destroy(&q->backing_dev_info); - ida_simple_remove(&blk_queue_ida, q->id); call_rcu(&q->rcu_head, blk_free_queue_rcu); } diff --git a/drivers/md/md.c b/drivers/md/md.c index d4f31e195e26..593a02476c78 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -4818,12 +4818,12 @@ static void md_free(struct kobject *ko) if (mddev->sysfs_state) sysfs_put(mddev->sysfs_state); + if (mddev->queue) + blk_cleanup_queue(mddev->queue); if (mddev->gendisk) { del_gendisk(mddev->gendisk); put_disk(mddev->gendisk); } - if (mddev->queue) - blk_cleanup_queue(mddev->queue); kfree(mddev); }