From patchwork Mon Jan 28 22:09:31 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alex Elder X-Patchwork-Id: 2058611 Return-Path: X-Original-To: patchwork-ceph-devel@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 2E923DF2A1 for ; Mon, 28 Jan 2013 22:09:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754895Ab3A1WJb (ORCPT ); Mon, 28 Jan 2013 17:09:31 -0500 Received: from mail-ia0-f181.google.com ([209.85.210.181]:55442 "EHLO mail-ia0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754748Ab3A1WJa (ORCPT ); Mon, 28 Jan 2013 17:09:30 -0500 Received: by mail-ia0-f181.google.com with SMTP id k25so4892679iah.40 for ; Mon, 28 Jan 2013 14:09:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:content-type:content-transfer-encoding :x-gm-message-state; bh=eQNMMEyoa9+dZ098TN06dPvRB/RklkMtPCQKISqt/Jg=; b=Tb9x9tdIVYju1agnNs+T/wtN8QGSb3hqqxm1638RTVps4DtJLHDozXQwsCOKlqi46H yH0y5wqBCk05NJNoBhTnznVTTn/qFltlLfLlD3jBL1bVwKoLRco9MZG0pGjqGrq/2bfN twZNm0q2N51rYZEp8gQDJ7qAzmv97Pvra9ijh0+KjbK5n161XsSUJ/bOQd/+xDKXerTq g0SQ69OyhAvUiQQu2tVp8zv6LzSXISi3xxHtN0v7rnvDcroJ8K4ccXs3fAiajWt2CKmu 49WNwGN5eiSQb+XKRmtJ5LcwKI8AiCLQW3sZS6n2llt2o8qAAfp8wHqtxx1jR6/4AHG9 BXeQ== X-Received: by 10.50.12.137 with SMTP id y9mr1558272igb.57.1359410970354; Mon, 28 Jan 2013 14:09:30 -0800 (PST) Received: from [172.22.22.4] (c-71-195-31-37.hsd1.mn.comcast.net. [71.195.31.37]) by mx.google.com with ESMTPS id eo7sm112200igc.12.2013.01.28.14.09.28 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 28 Jan 2013 14:09:29 -0800 (PST) Message-ID: <5106F71B.2030002@inktank.com> Date: Mon, 28 Jan 2013 16:09:31 -0600 From: Alex Elder User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130106 Thunderbird/17.0.2 MIME-Version: 1.0 To: ceph-devel@vger.kernel.org Subject: [PATCH 2/2] rbd: prevent open for image being removed References: <5106F6D2.7040805@inktank.com> In-Reply-To: <5106F6D2.7040805@inktank.com> X-Gm-Message-State: ALoCoQlJsZ8V9ipwbiom6UauJeqezOt5Fy/fGd3aHLORn3qAa78IpLG4HuXFaaLxVZTT8XRBVKq3 Sender: ceph-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: ceph-devel@vger.kernel.org An open request for a mapped rbd image can arrive while removal of that mapping is underway. We need to prevent such an open request from succeeding. (It appears that Maciej Galkiewicz ran into this problem.) Define and use a "removing" flag to indicate a mapping is getting removed. Set it in the remove path after verifying nothing holds the device open. And check it in the open path before allowing the open to proceed. Acquire the rbd device's lock around each of these spots to avoid any races accessing the flags and open_count fields. This addresses: http://tracker.newdream.net/issues/3427 Reported-by: Maciej Galkiewicz Signed-off-by: Alex Elder Reviewed-by: Josh Durgin --- drivers/block/rbd.c | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) return 0; @@ -399,10 +415,14 @@ static int rbd_open(struct block_device *bdev, fmode_t mode) static int rbd_release(struct gendisk *disk, fmode_t mode) { struct rbd_device *rbd_dev = disk->private_data; + unsigned long open_count_before; + + spin_lock(&rbd_dev->lock); + open_count_before = rbd_dev->open_count--; + spin_unlock(&rbd_dev->lock); + rbd_assert(open_count_before > 0); mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); - rbd_assert(rbd_dev->open_count > 0); - rbd_dev->open_count--; put_device(&rbd_dev->dev); mutex_unlock(&ctl_mutex); @@ -4135,10 +4155,14 @@ static ssize_t rbd_remove(struct bus_type *bus, goto done; } - if (rbd_dev->open_count) { + spin_lock(&rbd_dev->lock); + if (rbd_dev->open_count) ret = -EBUSY; + else + set_bit(rbd_dev_flag_removing, &rbd_dev->flags); + spin_unlock(&rbd_dev->lock); + if (ret < 0) goto done; - } while (rbd_dev->parent_spec) { struct rbd_device *first = rbd_dev; diff --git a/drivers/block/rbd.c b/drivers/block/rbd.c index 107df40..03b15b8 100644 --- a/drivers/block/rbd.c +++ b/drivers/block/rbd.c @@ -259,10 +259,10 @@ struct rbd_device { char name[DEV_NAME_LEN]; /* blkdev name, e.g. rbd3 */ - spinlock_t lock; /* queue lock */ + spinlock_t lock; /* queue, flags, open_count */ struct rbd_image_header header; - unsigned long flags; + unsigned long flags; /* possibly lock protected */ struct rbd_spec *spec; char *header_name; @@ -288,13 +288,20 @@ struct rbd_device { /* sysfs related */ struct device dev; - unsigned long open_count; + unsigned long open_count; /* protected by lock */ }; -/* Flag bits for rbd_dev->flags */ +/* + * Flag bits for rbd_dev->flags. If atomicity is required, + * rbd_dev->lock is used to protect access. + * + * Currently, only the "removing" flag (which is coupled with the + * "open_count" field) requires atomic access. + */ enum rbd_dev_flags { rbd_dev_flag_exists, /* mapped snapshot has not been deleted */ + rbd_dev_flag_removing, /* this mapping is being removed */ }; static DEFINE_MUTEX(ctl_mutex); /* Serialize open/close/setup/teardown */ @@ -383,14 +390,23 @@ static int rbd_dev_v2_refresh(struct rbd_device *rbd_dev, u64 *hver); static int rbd_open(struct block_device *bdev, fmode_t mode) { struct rbd_device *rbd_dev = bdev->bd_disk->private_data; + bool removing = false; if ((mode & FMODE_WRITE) && rbd_dev->mapping.read_only) return -EROFS; + spin_lock(&rbd_dev->lock); + if (test_bit(rbd_dev_flag_removing, &rbd_dev->flags)) + removing = true; + else + rbd_dev->open_count++; + spin_unlock(&rbd_dev->lock); + if (removing) + return -ENOENT; + mutex_lock_nested(&ctl_mutex, SINGLE_DEPTH_NESTING); (void) get_device(&rbd_dev->dev); set_device_ro(bdev, rbd_dev->mapping.read_only); - rbd_dev->open_count++; mutex_unlock(&ctl_mutex);