From patchwork Mon Jun 4 19:04:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kent Overstreet X-Patchwork-Id: 10447193 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 7353B603D7 for ; Mon, 4 Jun 2018 19:04:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 5DCE6286BF for ; Mon, 4 Jun 2018 19:04:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 51F3E28DD7; Mon, 4 Jun 2018 19:04:38 +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 1F38A286BF for ; Mon, 4 Jun 2018 19:04:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751687AbeFDTEf (ORCPT ); Mon, 4 Jun 2018 15:04:35 -0400 Received: from mail-qt0-f193.google.com ([209.85.216.193]:44742 "EHLO mail-qt0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751684AbeFDTEc (ORCPT ); Mon, 4 Jun 2018 15:04:32 -0400 Received: by mail-qt0-f193.google.com with SMTP id l33-v6so4962709qta.11 for ; Mon, 04 Jun 2018 12:04:32 -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=FPwE3+nNnE0uPC9/k/s9M2EJry+z9N0enKXNYBOQmL8=; b=HTTB7YSTBibAfcdLUdzBQahLHU/jvsnXhbjnq8AJfGEffJ7INwkOLDliaMv+Rf5LL8 HNO9D7u95bu9K/S/C8jsPDRBjSu+uX6jvKfpB++DbFwWRBrALS3k1tLhcRSvFWpfCYAW 386Mp74fuYOq5TfBMr5mnXykCRmveK2rdwHuVnKGCz8bwf8zDI7S9K9P4EWgr0Q2IXo3 0lO4+I5tthzelIWIf/Xnfa0mS1C5gdeW789SFkFiVJUwwFHZFHeusiyasTgBiDgBnMv+ KT9n8SSIixyBwwLO+S7ZzvLGQHFhEMGE9S+SwoE7qEZqGmTx2m+B3GKxKxYn21qFV5Gw VQxw== 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=FPwE3+nNnE0uPC9/k/s9M2EJry+z9N0enKXNYBOQmL8=; b=NmZAgN2UTJxmRIVUArJqHeZEKj50jWpK/Qs2ST6FAv5NP8wRd6N/s7tmBgb8fdqH+9 98PDOKWB23TcaRP0lbqEDOqEJcNoSbFy/JXH0+f9pmvj/K7cryc93+w/uFV7mAJhgyWr QzEQ236FZ78U1pprDeu8GMpVlb0p3mhkIsASNDupbk+Y4zCY2ZwsLgNkHmbuuWkKYznV X1KP53O8Iy2ypugOWbJD7/ljk+UHWE76xt3/5uwXFk8YAbxlXoEn0xZpVKlHqP9xP+EW Ir7YE3D9voqrZi2Tlef9cKFsIlJRwF4pcK8+JRCgqzu50VmX4jy9EEIvn7KUR7VRGf6y 3Ulw== X-Gm-Message-State: APt69E0QfkTE2c1IXQA/Oxq6tXexL4RJ+Cp5CTwJyN3ugPNFIZt1ELae IaTdzxc6B7Lab81UOvCCnQ== X-Google-Smtp-Source: ADUXVKInmyoiZEdXTK+2TO/7OP0i5WFIu723Gtu7QmH7X9PKX89/c9NUiHFN/qncPLBrP3HrKqZySQ== X-Received: by 2002:ac8:2fec:: with SMTP id m41-v6mr22846558qta.55.1528139071579; Mon, 04 Jun 2018 12:04:31 -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 34-v6sm11290027qtt.76.2018.06.04.12.04.29 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 04 Jun 2018 12:04:30 -0700 (PDT) Date: Mon, 4 Jun 2018 15:04:28 -0400 From: Kent Overstreet To: Jens Axboe Cc: Linus Torvalds , Tejun Heo , linux-block , NeilBrown , Arnd Bergmann Subject: Re: [GIT PULL] Block changes for 4.18-rc Message-ID: <20180604190428.GA30325@kmo-pixel> References: <11b01169-8e11-34ed-8137-aa5cd50a39c2@kernel.dk> <52aaf207-ef5d-a886-66fe-566de9d9bbee@kernel.dk> <20180604182017.GA1351649@devbig577.frc2.facebook.com> 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 12:25:54PM -0600, Jens Axboe wrote: > On 6/4/18 12:22 PM, Linus Torvalds wrote: > > On Mon, Jun 4, 2018 at 11:20 AM Tejun Heo wrote: > >> > >> > >> Looking at the code, the fundamental problem seems to be that it's > >> weaving different parts of sync and async paths. I don't understand > >> why it'd punt the destructin of mddev but destroy biosets > >> synchronously. Can't it do sth like the following? > > > > Yeah, that looks like the right thing to do. > > > > Jens/Kent? > > I agree with Tejun, we already discussed this offline. I don't see any good reason for the exit path to have two or three variations, depending on how you count. My preference would be for a patch that does this: 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. However, I have looked at where mddev_put() is called from and I think the stack usage is fine for now - unless I've missed something it's not called from e.g. under generic_make_request() anywhere, it's just called from sysfs code and ioctls and things like that, where the stack is going to be pretty much empty. diff --git a/drivers/md/md.c b/drivers/md/md.c index fc692b7128..f18d783785 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -510,36 +510,24 @@ 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) && mddev->ctime == 0 && !mddev->hold_active) { /* 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); + + /* 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); } spin_unlock(&all_mddevs_lock); - bioset_exit(&bs); - bioset_exit(&sync_bs); } static void md_safemode_timeout(struct timer_list *t);