From patchwork Tue Jun 5 00:56:14 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kent Overstreet X-Patchwork-Id: 10447521 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 00AC960284 for ; Tue, 5 Jun 2018 00:56:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E2D0C28970 for ; Tue, 5 Jun 2018 00:56:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D5E972901B; Tue, 5 Jun 2018 00:56:20 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4F81C28970 for ; Tue, 5 Jun 2018 00:56:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751170AbeFEA4T (ORCPT ); Mon, 4 Jun 2018 20:56:19 -0400 Received: from mail-qk0-f194.google.com ([209.85.220.194]:41256 "EHLO mail-qk0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751099AbeFEA4S (ORCPT ); Mon, 4 Jun 2018 20:56:18 -0400 Received: by mail-qk0-f194.google.com with SMTP id w23-v6so438894qkb.8 for ; Mon, 04 Jun 2018 17:56:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=gHB8cq46rJjxajk7KGimLigeBAw++Twl+zktcUF7SiE=; b=efimTVSlIV0K2m3/QH52r23le3HJWTkoX2EvC5EJuc7ihAGE9zkKRRTbbFfblKtHdN oatn4CEszZ0w6xwlVAfm2QvfpfogygAGQOHSlZwtNMS9llXXUTAfCZCmHmNyIq9M9Dsv OcL9wjHWHGTyRFpx89klhqLbiHW67TcLcqBAYeG6rI2uKE42K1F47noX3EjG9Oqdc4KF KfDEKUadu4qYmXCYh3VIN/HCftyeCKivVZ1gkXfrNEl8/CiGNOJ79gNtmUPqaJFl7/rv a/WZfsKKYTfcjlTB6QUdZT6vhvwVWzMYtdk80gFlqJL+wFUDkUOmkzdtfrdFz7VlQ14M NWwA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=gHB8cq46rJjxajk7KGimLigeBAw++Twl+zktcUF7SiE=; b=koeIrSL4jt/9NdDs9sh99iCPmVJoO48g53UASn16RsVjhlyrlCBdmfgPzO545y4q57 8v9RYb6+Gyco0eppHd0JpGb/mh7GsH7gO2lKJEALlM2iCMkm/pekMMxQxOfFJWNzM+Uc AXkSBsxKVcVMFDen6YoKoG8stx/AhzA4XSwGnGln3LR9DeUrWvF3GeZBo+/3XErPYobz ff8vdD1cZCtlJSZs7YCDEnUTrgsK55iw2Gc3IdVWXkEwD3xj2Kt4ObLSheMhIDHdEB/p i24GekEt+5EUQNi0K0NHqZMhiu1x7z77b0PEeX6U4rwJ3KM181dDpqmVe+MVMwoql+1E JE3Q== X-Gm-Message-State: APt69E1mGcC0KyxWsD59lB9gsEgUNkYE81Z4Nl0GxUof1kFGB0TzC5r0 7aOlPdDHTiQ75ZqN/9JtNA== X-Google-Smtp-Source: ADUXVKLbCbs8d/O9Z6hmTSTwIaOqMl9ZuyDtX0fcoh+hnKbanvXA/XS96PfPNLmCZK/xLtj560u0BQ== X-Received: by 2002:a37:8b07:: with SMTP id n7-v6mr16649739qkd.353.1528160178013; Mon, 04 Jun 2018 17:56:18 -0700 (PDT) Received: from kmo-pixel (c-71-234-172-214.hsd1.vt.comcast.net. [71.234.172.214]) by smtp.gmail.com with ESMTPSA id y128-v6sm5372004qkc.1.2018.06.04.17.56.16 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 04 Jun 2018 17:56:16 -0700 (PDT) Date: Mon, 4 Jun 2018 20:56:14 -0400 From: Kent Overstreet To: Linus Torvalds Cc: Jens Axboe , Tejun Heo , linux-block , NeilBrown , Arnd Bergmann Subject: Re: [GIT PULL] Block changes for 4.18-rc Message-ID: <20180605005614.GE30325@kmo-pixel> References: <11b01169-8e11-34ed-8137-aa5cd50a39c2@kernel.dk> <52aaf207-ef5d-a886-66fe-566de9d9bbee@kernel.dk> <20180604182017.GA1351649@devbig577.frc2.facebook.com> <20180604190428.GA30325@kmo-pixel> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.0 (2018-05-17) Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Jun 04, 2018 at 05:42:04PM -0700, Linus Torvalds wrote: > On Mon, Jun 4, 2018 at 12:04 PM Kent Overstreet > wrote: > > > > However, that's not correct as is because mddev_delayed_put() calls > > kobject_put(), and the kobject isn't initialized when the mddev is first > > allocated, it's initialized when the gendisk is allocated... that isn't hard to > > fix but that's getting into real refactoring that I'll need to put actual work > > into testing. > > Well, it also removes the bioset_exit() calls entirely. Yeah, I realized that when I went back to finish that patch > > How about just the attached? > > It simply does it as two different cases, and adds the bioset_exit() > calls to mddev_delayed_delete(). Oh right, just taking advantage of the fact that just the queue_work() needs to be under the spinlock, not the actual free in the other case. I like your patch for a less invasive version, but I did finish and test my version, which deletes more code :) I've already gone to the trouble of coming up with a VM smoketest, so I can test yours too... I don't really have a strong opinion on which patch should go in. drivers/md/md.c | 53 ++++++++++++++++++----------------------------------- 1 file changed, 18 insertions(+), 35 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index fc692b7128..22203eba1e 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -84,6 +84,8 @@ static void autostart_arrays(int part); static LIST_HEAD(pers_list); static DEFINE_SPINLOCK(pers_lock); +static struct kobj_type md_ktype; + struct md_cluster_operations *md_cluster_ops; EXPORT_SYMBOL(md_cluster_ops); struct module *md_cluster_mod; @@ -510,11 +512,6 @@ static void mddev_delayed_delete(struct work_struct *ws); static void mddev_put(struct mddev *mddev) { - struct bio_set bs, sync_bs; - - memset(&bs, 0, sizeof(bs)); - memset(&sync_bs, 0, sizeof(sync_bs)); - if (!atomic_dec_and_lock(&mddev->active, &all_mddevs_lock)) return; if (!mddev->raid_disks && list_empty(&mddev->disks) && @@ -522,30 +519,23 @@ static void mddev_put(struct mddev *mddev) /* Array is not configured at all, and not held active, * so destroy it */ list_del_init(&mddev->all_mddevs); - bs = mddev->bio_set; - sync_bs = mddev->sync_set; - memset(&mddev->bio_set, 0, sizeof(mddev->bio_set)); - memset(&mddev->sync_set, 0, sizeof(mddev->sync_set)); - if (mddev->gendisk) { - /* We did a probe so need to clean up. Call - * queue_work inside the spinlock so that - * flush_workqueue() after mddev_find will - * succeed in waiting for the work to be done. - */ - INIT_WORK(&mddev->del_work, mddev_delayed_delete); - queue_work(md_misc_wq, &mddev->del_work); - } else - kfree(mddev); + + /* + * Call queue_work inside the spinlock so that + * flush_workqueue() after mddev_find will succeed in waiting + * for the work to be done. + */ + INIT_WORK(&mddev->del_work, mddev_delayed_delete); + queue_work(md_misc_wq, &mddev->del_work); } spin_unlock(&all_mddevs_lock); - bioset_exit(&bs); - bioset_exit(&sync_bs); } static void md_safemode_timeout(struct timer_list *t); void mddev_init(struct mddev *mddev) { + kobject_init(&mddev->kobj, &md_ktype); mutex_init(&mddev->open_mutex); mutex_init(&mddev->reconfig_mutex); mutex_init(&mddev->bitmap_info.mutex); @@ -5215,6 +5205,8 @@ static void md_free(struct kobject *ko) put_disk(mddev->gendisk); percpu_ref_exit(&mddev->writes_pending); + bioset_exit(&mddev->bio_set); + bioset_exit(&mddev->sync_set); kfree(mddev); } @@ -5348,8 +5340,7 @@ static int md_alloc(dev_t dev, char *name) mutex_lock(&mddev->open_mutex); add_disk(disk); - error = kobject_init_and_add(&mddev->kobj, &md_ktype, - &disk_to_dev(disk)->kobj, "%s", "md"); + error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md"); if (error) { /* This isn't possible, but as kobject_init_and_add is marked * __must_check, we must do something with the result @@ -5506,7 +5497,7 @@ int md_run(struct mddev *mddev) if (!bioset_initialized(&mddev->sync_set)) { err = bioset_init(&mddev->sync_set, BIO_POOL_SIZE, 0, BIOSET_NEED_BVECS); if (err) - goto abort; + return err; } spin_lock(&pers_lock); @@ -5519,8 +5510,7 @@ int md_run(struct mddev *mddev) else pr_warn("md: personality for level %s is not loaded!\n", mddev->clevel); - err = -EINVAL; - goto abort; + return -EINVAL; } spin_unlock(&pers_lock); if (mddev->level != pers->level) { @@ -5533,8 +5523,7 @@ int md_run(struct mddev *mddev) pers->start_reshape == NULL) { /* This personality cannot handle reshaping... */ module_put(pers->owner); - err = -EINVAL; - goto abort; + return -EINVAL; } if (pers->sync_request) { @@ -5603,7 +5592,7 @@ int md_run(struct mddev *mddev) mddev->private = NULL; module_put(pers->owner); bitmap_destroy(mddev); - goto abort; + return err; } if (mddev->queue) { bool nonrot = true; @@ -5665,12 +5654,6 @@ int md_run(struct mddev *mddev) sysfs_notify_dirent_safe(mddev->sysfs_action); sysfs_notify(&mddev->kobj, NULL, "degraded"); return 0; - -abort: - bioset_exit(&mddev->bio_set); - bioset_exit(&mddev->sync_set); - - return err; } EXPORT_SYMBOL_GPL(md_run);