From patchwork Thu Apr 6 21:02:06 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Josef Bacik X-Patchwork-Id: 9668431 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 D496360364 for ; Thu, 6 Apr 2017 21:02:37 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DDAE5285DD for ; Thu, 6 Apr 2017 21:02:37 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D29BF285E1; Thu, 6 Apr 2017 21:02:37 +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=-6.4 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM 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 4741C285DE for ; Thu, 6 Apr 2017 21:02:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753160AbdDFVCf (ORCPT ); Thu, 6 Apr 2017 17:02:35 -0400 Received: from mail-qt0-f193.google.com ([209.85.216.193]:33248 "EHLO mail-qt0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932310AbdDFVC0 (ORCPT ); Thu, 6 Apr 2017 17:02:26 -0400 Received: by mail-qt0-f193.google.com with SMTP id r45so7384174qte.0 for ; Thu, 06 Apr 2017 14:02:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=toxicpanda-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:in-reply-to:references; bh=uwFTRNM0jdeITZVWQABUnh00qflSFt46N6kEFt5BDbU=; b=fiR52y31KA8NbzouyIizuiMiQbZVDeVvcFvTizGMB3rk9MHWkfGp3IbD/t7kHsNUCp bs3FscX67D+ZckLhIZFfD2vmg76nWssnKB7rdDCpOU/N2krXOWBfLl6lH19vV2EeFpy3 0WqYCTNorbXL3ClOZk3TGohVpdeOJpETxnWQC3V7GjaFV1lB8A+jNaVrzzxIXn0fSR4Q QFkdqDzDIPyEtljqLGxkP3bTjJkmblknVyK9c+ENQ19rRuMc4+61Nrbm6BV3pflbsdeO Ckew4LdFeMe+kaQZVVARMavX2Zb7hyB1A927HiCgsUyL2D//dz3dWsnxTAg9+5KRS2aF AfdA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=uwFTRNM0jdeITZVWQABUnh00qflSFt46N6kEFt5BDbU=; b=V04TyFGGWTfQIVVapkLagCSXs5wHu10yhPBfhOS/V8Od8/OFDIxCuj5y56ZxfrY1vz nLPkX34bu4kxWLuhvNzVCHK1OiVL56UIQlWBFCEDt4YettGaHRVBPofXxYiMLjimgklr A8f18RhEkJbGlVs9/8IuLfVKfYzGqynjPx2LbkNL/2l9pNRmjx202rb6dV0D1Jst0W03 AS1kI9sndLvYM7HIAANnVPH8hAFtOJhBj+cWd25LrpoqLyVxJXtwJrtqJ5GRHMAhOenz 5yE3AOf+DnpO13HwsQI7699aQWimKWJAzrUTl3uQGIx//OWmuGJVDLblNuI/b/yJK8bC ZouQ== X-Gm-Message-State: AFeK/H2wJOYdGg9jx1cdj5wy8zCUJZ2jut6Yx/33yMk0lW7RlwTtuY75Eb573/ZPSF13cA== X-Received: by 10.237.62.25 with SMTP id l25mr36604005qtf.225.1491512544421; Thu, 06 Apr 2017 14:02:24 -0700 (PDT) Received: from localhost (cpe-2606-A000-4381-1201-225-22FF-FEB3-E51A.dyn6.twc.com. [2606:a000:4381:1201:225:22ff:feb3:e51a]) by smtp.gmail.com with ESMTPSA id f28sm1717102qtf.44.2017.04.06.14.02.23 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 06 Apr 2017 14:02:23 -0700 (PDT) From: Josef Bacik X-Google-Original-From: Josef Bacik To: axboe@kernel.dk, nbd-general@lists.sourceforge.net, linux-block@vger.kernel.org, kernel-team@fb.com Subject: [PATCH 11/12] nbd: add device refcounting Date: Thu, 6 Apr 2017 17:02:06 -0400 Message-Id: <1491512527-4286-12-git-send-email-jbacik@fb.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1491512527-4286-1-git-send-email-jbacik@fb.com> References: <1491512527-4286-1-git-send-email-jbacik@fb.com> 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 In order to support deleting the device on disconnect we need to refcount the actual nbd_device struct. So add the refcounting framework and change how we free the normal devices at rmmod time so we can catch reference leaks. Signed-off-by: Josef Bacik --- drivers/block/nbd.c | 104 +++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 86 insertions(+), 18 deletions(-) diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index e1289d0..b33014a 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -99,10 +99,12 @@ struct nbd_device { int index; refcount_t config_refs; + refcount_t refs; struct nbd_config *config; struct mutex config_lock; struct gendisk *disk; + struct list_head list; struct task_struct *task_recv; struct task_struct *task_setup; }; @@ -165,6 +167,28 @@ static struct device_attribute pid_attr = { .show = pid_show, }; +static void nbd_dev_remove(struct nbd_device *nbd) +{ + struct gendisk *disk = nbd->disk; + if (disk) { + del_gendisk(disk); + blk_cleanup_queue(disk->queue); + blk_mq_free_tag_set(&nbd->tag_set); + put_disk(disk); + } + kfree(nbd); +} + +static void nbd_put(struct nbd_device *nbd) +{ + if (refcount_dec_and_mutex_lock(&nbd->refs, + &nbd_index_mutex)) { + idr_remove(&nbd_index_idr, nbd->index); + mutex_unlock(&nbd_index_mutex); + nbd_dev_remove(nbd); + } +} + static int nbd_disconnected(struct nbd_config *config) { return test_bit(NBD_DISCONNECTED, &config->runtime_flags) || @@ -1005,6 +1029,7 @@ static void nbd_config_put(struct nbd_device *nbd) } nbd_reset(nbd); mutex_unlock(&nbd->config_lock); + nbd_put(nbd); module_put(THIS_MODULE); } } @@ -1199,6 +1224,10 @@ static int nbd_open(struct block_device *bdev, fmode_t mode) ret = -ENXIO; goto out; } + if (!refcount_inc_not_zero(&nbd->refs)) { + ret = -ENXIO; + goto out; + } if (!refcount_inc_not_zero(&nbd->config_refs)) { struct nbd_config *config; @@ -1214,6 +1243,7 @@ static int nbd_open(struct block_device *bdev, fmode_t mode) goto out; } refcount_set(&nbd->config_refs, 1); + refcount_inc(&nbd->refs); mutex_unlock(&nbd->config_lock); } out: @@ -1225,6 +1255,7 @@ static void nbd_release(struct gendisk *disk, fmode_t mode) { struct nbd_device *nbd = disk->private_data; nbd_config_put(nbd); + nbd_put(nbd); } static const struct block_device_operations nbd_fops = @@ -1378,18 +1409,6 @@ static const struct blk_mq_ops nbd_mq_ops = { .timeout = nbd_xmit_timeout, }; -static void nbd_dev_remove(struct nbd_device *nbd) -{ - struct gendisk *disk = nbd->disk; - if (disk) { - del_gendisk(disk); - blk_cleanup_queue(disk->queue); - blk_mq_free_tag_set(&nbd->tag_set); - put_disk(disk); - } - kfree(nbd); -} - static int nbd_dev_add(int index) { struct nbd_device *nbd; @@ -1453,6 +1472,8 @@ static int nbd_dev_add(int index) mutex_init(&nbd->config_lock); refcount_set(&nbd->config_refs, 0); + refcount_set(&nbd->refs, 1); + INIT_LIST_HEAD(&nbd->list); disk->major = NBD_MAJOR; disk->first_minor = index << part_shift; disk->fops = &nbd_fops; @@ -1550,16 +1571,26 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) } else { nbd = idr_find(&nbd_index_idr, index); } - mutex_unlock(&nbd_index_mutex); if (!nbd) { printk(KERN_ERR "nbd: couldn't find device at index %d\n", index); + mutex_unlock(&nbd_index_mutex); + return -EINVAL; + } + if (!refcount_inc_not_zero(&nbd->refs)) { + mutex_unlock(&nbd_index_mutex); + if (index == -1) + goto again; + printk(KERN_ERR "nbd: device at index %d is going down\n", + index); return -EINVAL; } + mutex_unlock(&nbd_index_mutex); mutex_lock(&nbd->config_lock); if (refcount_read(&nbd->config_refs)) { mutex_unlock(&nbd->config_lock); + nbd_put(nbd); if (index == -1) goto again; printk(KERN_ERR "nbd: nbd%d already in use\n", index); @@ -1567,11 +1598,13 @@ static int nbd_genl_connect(struct sk_buff *skb, struct genl_info *info) } if (WARN_ON(nbd->config)) { mutex_unlock(&nbd->config_lock); + nbd_put(nbd); return -EINVAL; } config = nbd->config = nbd_alloc_config(); if (!nbd->config) { mutex_unlock(&nbd->config_lock); + nbd_put(nbd); printk(KERN_ERR "nbd: couldn't allocate config\n"); return -ENOMEM; } @@ -1656,14 +1689,23 @@ static int nbd_genl_disconnect(struct sk_buff *skb, struct genl_info *info) index = nla_get_u32(info->attrs[NBD_ATTR_INDEX]); mutex_lock(&nbd_index_mutex); nbd = idr_find(&nbd_index_idr, index); - mutex_unlock(&nbd_index_mutex); if (!nbd) { + mutex_unlock(&nbd_index_mutex); printk(KERN_ERR "nbd: couldn't find device at index %d\n", index); return -EINVAL; } - if (!refcount_inc_not_zero(&nbd->config_refs)) + if (!refcount_inc_not_zero(&nbd->refs)) { + mutex_unlock(&nbd_index_mutex); + printk(KERN_ERR "nbd: device at index %d is going down\n", + index); + return -EINVAL; + } + mutex_unlock(&nbd_index_mutex); + if (!refcount_inc_not_zero(&nbd->config_refs)) { + nbd_put(nbd); return 0; + } mutex_lock(&nbd->config_lock); nbd_disconnect(nbd); mutex_unlock(&nbd->config_lock); @@ -1671,6 +1713,7 @@ static int nbd_genl_disconnect(struct sk_buff *skb, struct genl_info *info) &nbd->config->runtime_flags)) nbd_config_put(nbd); nbd_config_put(nbd); + nbd_put(nbd); return 0; } @@ -1691,16 +1734,24 @@ static int nbd_genl_reconfigure(struct sk_buff *skb, struct genl_info *info) index = nla_get_u32(info->attrs[NBD_ATTR_INDEX]); mutex_lock(&nbd_index_mutex); nbd = idr_find(&nbd_index_idr, index); - mutex_unlock(&nbd_index_mutex); if (!nbd) { + mutex_unlock(&nbd_index_mutex); printk(KERN_ERR "nbd: couldn't find a device at index %d\n", index); return -EINVAL; } + if (!refcount_inc_not_zero(&nbd->refs)) { + mutex_unlock(&nbd_index_mutex); + printk(KERN_ERR "nbd: device at index %d is going down\n", + index); + return -EINVAL; + } + mutex_unlock(&nbd_index_mutex); if (!refcount_inc_not_zero(&nbd->config_refs)) { dev_err(nbd_to_dev(nbd), "not configured, cannot reconfigure\n"); + nbd_put(nbd); return -EINVAL; } @@ -1759,6 +1810,7 @@ static int nbd_genl_reconfigure(struct sk_buff *skb, struct genl_info *info) out: mutex_unlock(&nbd->config_lock); nbd_config_put(nbd); + nbd_put(nbd); return ret; } @@ -2004,16 +2056,32 @@ static int __init nbd_init(void) static int nbd_exit_cb(int id, void *ptr, void *data) { + struct list_head *list = (struct list_head *)data; struct nbd_device *nbd = ptr; - nbd_dev_remove(nbd); + + refcount_inc(&nbd->refs); + list_add_tail(&nbd->list, list); return 0; } static void __exit nbd_cleanup(void) { + struct nbd_device *nbd; + LIST_HEAD(del_list); + nbd_dbg_close(); - idr_for_each(&nbd_index_idr, &nbd_exit_cb, NULL); + mutex_lock(&nbd_index_mutex); + idr_for_each(&nbd_index_idr, &nbd_exit_cb, &del_list); + mutex_unlock(&nbd_index_mutex); + + list_for_each_entry(nbd, &del_list, list) { + if (refcount_read(&nbd->refs) != 2) + printk(KERN_ERR "nbd: possibly leaking a device\n"); + nbd_put(nbd); + nbd_put(nbd); + } + idr_destroy(&nbd_index_idr); genl_unregister_family(&nbd_genl_family); destroy_workqueue(recv_workqueue);