From: Christoph Hellwig <hch@lst.de>
To: Al Viro <viro@zeniv.linux.org.uk>,
Christian Brauner <brauner@kernel.org>
Cc: Jens Axboe <axboe@kernel.dk>, Denis Efremov <efremov@linux.com>,
Josef Bacik <josef@toxicpanda.com>,
Stefan Haberland <sth@linux.ibm.com>,
Jan Hoeppner <hoeppner@linux.ibm.com>,
Heiko Carstens <hca@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Alexander Gordeev <agordeev@linux.ibm.com>,
"Darrick J . Wong" <djwong@kernel.org>, Chris Mason <clm@fb.com>,
David Sterba <dsterba@suse.com>,
linux-block@vger.kernel.org, nbd@other.debian.org,
linux-s390@vger.kernel.org, linux-btrfs@vger.kernel.org,
linux-fsdevel@vger.kernel.org
Subject: [PATCH 04/17] btrfs: split btrfs_fs_devices.opened
Date: Fri, 11 Aug 2023 12:08:15 +0200 [thread overview]
Message-ID: <20230811100828.1897174-5-hch@lst.de> (raw)
In-Reply-To: <20230811100828.1897174-1-hch@lst.de>
The btrfs_fs_devices.opened member mixes an in use counter for the
fs_devices structure that prevents it from being garbage collected with
a flag if the underlying devices were actually opened. This not only
makes the code hard to follow, but also prevents btrfs from switching
to opening the block device only after super block creation. Split it
into an in_use counter and an is_open boolean flag instead.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/btrfs/volumes.c | 49 ++++++++++++++++++++++++++--------------------
fs/btrfs/volumes.h | 6 ++++--
2 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8246578c70f55b..3ac1a3aa8939bc 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -405,7 +405,8 @@ static void free_fs_devices(struct btrfs_fs_devices *fs_devices)
{
struct btrfs_device *device;
- WARN_ON(fs_devices->opened);
+ WARN_ON_ONCE(fs_devices->in_use);
+ WARN_ON_ONCE(fs_devices->is_open);
while (!list_empty(&fs_devices->devices)) {
device = list_entry(fs_devices->devices.next,
struct btrfs_device, dev_list);
@@ -578,7 +579,7 @@ static int btrfs_free_stale_devices(dev_t devt, struct btrfs_device *skip_device
continue;
if (devt && devt != device->devt)
continue;
- if (fs_devices->opened) {
+ if (fs_devices->in_use) {
/* for an already deleted device return 0 */
if (devt && ret != 0)
ret = -EBUSY;
@@ -849,7 +850,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
if (!device) {
unsigned int nofs_flag;
- if (fs_devices->opened) {
+ if (fs_devices->in_use) {
btrfs_err(NULL,
"device %s belongs to fsid %pU, and the fs is already mounted",
path, fs_devices->fsid);
@@ -913,7 +914,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
* tracking a problem where systems fail mount by subvolume id
* when we reject replacement on a mounted FS.
*/
- if (!fs_devices->opened && found_transid < device->generation) {
+ if (!fs_devices->in_use && found_transid < device->generation) {
/*
* That is if the FS is _not_ mounted and if you
* are here, that means there is more than one
@@ -974,7 +975,7 @@ static noinline struct btrfs_device *device_list_add(const char *path,
* it back. We need it to pick the disk with largest generation
* (as above).
*/
- if (!fs_devices->opened) {
+ if (!fs_devices->in_use) {
device->generation = found_transid;
fs_devices->latest_generation = max_t(u64, found_transid,
fs_devices->latest_generation);
@@ -1172,15 +1173,19 @@ static void close_fs_devices(struct btrfs_fs_devices *fs_devices)
lockdep_assert_held(&uuid_mutex);
- if (--fs_devices->opened > 0)
+ if (--fs_devices->in_use > 0)
return;
+ if (!fs_devices->is_open)
+ goto done;
+
list_for_each_entry_safe(device, tmp, &fs_devices->devices, dev_list)
btrfs_close_one_device(device);
WARN_ON(fs_devices->open_devices);
WARN_ON(fs_devices->rw_devices);
- fs_devices->opened = 0;
+ fs_devices->is_open = false;
+done:
fs_devices->seeding = false;
fs_devices->fs_info = NULL;
}
@@ -1192,7 +1197,7 @@ void btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
mutex_lock(&uuid_mutex);
close_fs_devices(fs_devices);
- if (!fs_devices->opened) {
+ if (!fs_devices->in_use) {
list_splice_init(&fs_devices->seed_list, &list);
/*
@@ -1240,7 +1245,7 @@ static int open_fs_devices(struct btrfs_fs_devices *fs_devices,
if (fs_devices->open_devices == 0)
return -EINVAL;
- fs_devices->opened = 1;
+ fs_devices->is_open = true;
fs_devices->latest_dev = latest_dev;
fs_devices->total_rw_bytes = 0;
fs_devices->chunk_alloc_policy = BTRFS_CHUNK_ALLOC_REGULAR;
@@ -1277,16 +1282,14 @@ int btrfs_open_devices(struct btrfs_fs_devices *fs_devices,
* We also don't need the lock here as this is called during mount and
* exclusion is provided by uuid_mutex
*/
-
- if (fs_devices->opened) {
- fs_devices->opened++;
- ret = 0;
- } else {
+ if (!fs_devices->is_open) {
list_sort(NULL, &fs_devices->devices, devid_cmp);
ret = open_fs_devices(fs_devices, flags, holder);
+ if (ret)
+ return ret;
}
-
- return ret;
+ fs_devices->in_use++;
+ return 0;
}
void btrfs_release_disk_super(struct btrfs_super_block *super)
@@ -2242,13 +2245,14 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info,
* This can happen if cur_devices is the private seed devices list. We
* cannot call close_fs_devices() here because it expects the uuid_mutex
* to be held, but in fact we don't need that for the private
- * seed_devices, we can simply decrement cur_devices->opened and then
+ * seed_devices, we can simply decrement cur_devices->in_use and then
* remove it from our list and free the fs_devices.
*/
if (cur_devices->num_devices == 0) {
list_del_init(&cur_devices->seed_list);
- ASSERT(cur_devices->opened == 1);
- cur_devices->opened--;
+ ASSERT(cur_devices->in_use == 1);
+ cur_devices->in_use--;
+ cur_devices->is_open = false;
free_fs_devices(cur_devices);
}
@@ -2478,7 +2482,8 @@ static struct btrfs_fs_devices *btrfs_init_sprout(struct btrfs_fs_info *fs_info)
list_add(&old_devices->fs_list, &fs_uuids);
memcpy(seed_devices, fs_devices, sizeof(*seed_devices));
- seed_devices->opened = 1;
+ seed_devices->in_use = 1;
+ seed_devices->is_open = true;
INIT_LIST_HEAD(&seed_devices->devices);
INIT_LIST_HEAD(&seed_devices->alloc_list);
mutex_init(&seed_devices->device_list_mutex);
@@ -6883,7 +6888,8 @@ static struct btrfs_fs_devices *open_seed_devices(struct btrfs_fs_info *fs_info,
return fs_devices;
fs_devices->seeding = true;
- fs_devices->opened = 1;
+ fs_devices->in_use = 1;
+ fs_devices->is_open = true;
return fs_devices;
}
@@ -6900,6 +6906,7 @@ static struct btrfs_fs_devices *open_seed_devices(struct btrfs_fs_info *fs_info,
free_fs_devices(fs_devices);
return ERR_PTR(ret);
}
+ fs_devices->in_use = 1;
if (!fs_devices->seeding) {
close_fs_devices(fs_devices);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 824161c6dd063e..f472d646715e72 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -346,8 +346,10 @@ struct btrfs_fs_devices {
struct list_head seed_list;
- /* Count fs-devices opened. */
- int opened;
+ /* Count if fs_device is in used. */
+ unsigned int in_use;
+ /* True if the devices were opened. */
+ bool is_open;
/* Set when we find or add a device that doesn't have the nonrot flag set. */
bool rotating;
--
2.39.2
next prev parent reply other threads:[~2023-08-11 10:09 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-11 10:08 remove get_super Christoph Hellwig
2023-08-11 10:08 ` [PATCH 01/17] FOLD: reverts part of "fs: use the super_block as holder when mounting file systems" Christoph Hellwig
2023-08-11 10:44 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 02/17] btrfs: always open the device read-only in btrfs_scan_one_device Christoph Hellwig
2023-08-11 12:00 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 03/17] btrfs: call btrfs_close_devices from ->kill_sb Christoph Hellwig
2023-08-11 12:03 ` Christian Brauner
2023-08-11 10:08 ` Christoph Hellwig [this message]
2023-08-11 12:40 ` [PATCH 04/17] btrfs: split btrfs_fs_devices.opened Christian Brauner
2023-08-11 10:08 ` [PATCH 05/17] btrfs: open block devices after superblock creation Christoph Hellwig
2023-08-11 12:44 ` Christian Brauner
2023-08-11 13:11 ` David Sterba
2023-08-17 13:24 ` David Sterba
2023-08-11 10:08 ` [PATCH 06/17] btrfs: use the super_block as holder when mounting file systems Christoph Hellwig
2023-08-11 12:45 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 07/17] nbd: call blk_mark_disk_dead in nbd_clear_sock_ioctl Christoph Hellwig
2023-09-20 20:41 ` Samuel Holland
2023-09-25 7:48 ` Christoph Hellwig
2023-10-01 17:10 ` Wouter Verhelst
2023-10-02 6:21 ` Christoph Hellwig
2023-10-02 19:15 ` Samuel Holland
2023-08-11 10:08 ` [PATCH 08/17] block: simplify the disk_force_media_change interface Christoph Hellwig
2023-08-11 10:08 ` [PATCH 09/17] floppy: call disk_force_media_change when changing the format Christoph Hellwig
2023-08-11 10:08 ` [PATCH 10/17] amiflop: don't call fsync_bdev in FDFMTBEG Christoph Hellwig
2023-08-11 10:08 ` [PATCH 11/17] dasd: also call __invalidate_device when setting the device offline Christoph Hellwig
2023-08-11 10:08 ` [PATCH 12/17] block: drop the "busy inodes on changed media" log message Christoph Hellwig
2023-08-11 10:08 ` [PATCH 13/17] block: consolidate __invalidate_device and fsync_bdev Christoph Hellwig
2023-08-12 10:51 ` Christoph Hellwig
2023-08-12 17:04 ` Heiko Carstens
2023-08-12 17:28 ` Heiko Carstens
2023-08-12 20:43 ` Matthew Wilcox
2023-08-11 10:08 ` [PATCH 14/17] block: call into the file system for bdev_mark_dead Christoph Hellwig
2023-08-11 10:08 ` [PATCH 15/17] block: call into the file system for ioctl BLKFLSBUF Christoph Hellwig
2023-08-11 14:06 ` Josef Bacik
2023-08-11 10:08 ` [PATCH 16/17] fs: remove get_super Christoph Hellwig
2023-08-11 12:46 ` Christian Brauner
2023-08-11 10:08 ` [PATCH 17/17] fs: simplify invalidate_inodes Christoph Hellwig
2023-08-11 12:48 ` Christian Brauner
2023-08-11 13:58 ` remove get_super Josef Bacik
2023-08-11 19:05 ` Josef Bacik
2023-08-14 19:19 ` David Sterba
2023-09-12 17:42 ` David Sterba
2023-09-14 8:48 ` Jan Kara
2023-09-14 12:03 ` David Sterba
2023-09-14 12:54 ` Jan Kara
2023-09-15 17:28 ` Jan Kara
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230811100828.1897174-5-hch@lst.de \
--to=hch@lst.de \
--cc=agordeev@linux.ibm.com \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=clm@fb.com \
--cc=djwong@kernel.org \
--cc=dsterba@suse.com \
--cc=efremov@linux.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=hoeppner@linux.ibm.com \
--cc=josef@toxicpanda.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=nbd@other.debian.org \
--cc=sth@linux.ibm.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).