This is a series I've picked up from Christoph, it changes the
block_device's bdev holder from fs_type to the super block.
Here's the original cover letter:
Hi all,
this series contains the btrfs parts of the "remove get_super" from June
that managed to get lost.
I've dropped all the reviews from back then as the rebase against the new
mount API conversion led to a lot of non-trivial conflicts.
Josef kindly ran it through the CI farm and provided a fixup based on that.
---
Christoph Hellwig (5):
btrfs: always open the device read-only in btrfs_scan_one_device
btrfs: call btrfs_close_devices from ->kill_sb
btrfs: split btrfs_fs_devices.opened
btrfs: open block devices after superblock creation
btrfs: use the super_block as holder when mounting file systems
fs/btrfs/disk-io.c | 4 +--
fs/btrfs/super.c | 71 ++++++++++++++++++++++++++++++------------------------
fs/btrfs/volumes.c | 60 +++++++++++++++++++++++----------------------
fs/btrfs/volumes.h | 8 +++---
4 files changed, 78 insertions(+), 65 deletions(-)
---
base-commit: a50d41606b333e4364844987deb1060e7ea6c038
change-id: 20240214-hch-device-open-309ef9c98c62
Best regards,
--
Johannes Thumshirn <[email protected]>
From: Christoph Hellwig <[email protected]>
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 <[email protected]>
Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/volumes.c | 51 +++++++++++++++++++++++++++++----------------------
fs/btrfs/volumes.h | 6 ++++--
2 files changed, 33 insertions(+), 24 deletions(-)
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 44caf1a48d33..f27af155abf0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -412,7 +412,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);
@@ -535,7 +536,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) {
if (devt)
ret = -EBUSY;
break;
@@ -607,7 +608,7 @@ static struct btrfs_fs_devices *find_fsid_by_device(
if (found_by_devt) {
/* Existing device. */
if (fsid_fs_devices == NULL) {
- if (devt_fs_devices->opened == 0) {
+ if (devt_fs_devices->in_use == 0) {
/* Stale device. */
return NULL;
} else {
@@ -795,7 +796,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, scanned by %s (%d)",
path, fs_devices->fsid, current->comm,
@@ -860,7 +861,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
@@ -921,7 +922,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);
@@ -1120,15 +1121,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;
}
@@ -1140,7 +1145,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);
/*
@@ -1188,7 +1193,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;
@@ -1225,16 +1230,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)
@@ -2201,13 +2204,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);
}
@@ -2437,7 +2441,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);
@@ -7115,7 +7120,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;
}
@@ -7132,6 +7138,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 97c7284e7565..d6dc41c62998 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -372,8 +372,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.43.0
From: Christoph Hellwig <[email protected]>
The file system type is not a very useful holder as it doesn't allow us
to go back to the actual file system instance. Pass the super_block
instead which is useful when passed back to the file system driver.
This matches what is done for all other block device based file systems.
Signed-off-by: Christoph Hellwig <[email protected]>
Signed-off-by: Johannes Thumshirn <[email protected]>
---
fs/btrfs/super.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 1fa7d83d02c1..0c7956e8f21e 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -1843,7 +1843,7 @@ static int btrfs_get_tree_super(struct fs_context *fc)
struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
mutex_lock(&uuid_mutex);
- ret = btrfs_open_devices(fs_devices, mode, &btrfs_fs_type);
+ ret = btrfs_open_devices(fs_devices, mode, sb);
mutex_unlock(&uuid_mutex);
if (ret)
goto error_deactivate;
@@ -1856,7 +1856,7 @@ static int btrfs_get_tree_super(struct fs_context *fc)
snprintf(sb->s_id, sizeof(sb->s_id), "%pg",
fs_devices->latest_dev->bdev);
shrinker_debugfs_rename(sb->s_shrink, "sb-btrfs:%s", sb->s_id);
- btrfs_sb(sb)->bdev_holder = &btrfs_fs_type;
+ btrfs_sb(sb)->bdev_holder = sb;
ret = btrfs_fill_super(sb, fs_devices, NULL);
if (ret)
goto error_deactivate;
--
2.43.0
On Wed, Feb 14, 2024 at 08:42:11AM -0800, Johannes Thumshirn wrote:
> This is a series I've picked up from Christoph, it changes the
> block_device's bdev holder from fs_type to the super block.
Applies and builds on my for-next, and LGTM. A few non-urgent inline
comments in the patches, but assuming this has gone through CI again,
Reviewed-by: Boris Burkov <[email protected]>
>
> Here's the original cover letter:
> Hi all,
>
> this series contains the btrfs parts of the "remove get_super" from June
> that managed to get lost.
>
> I've dropped all the reviews from back then as the rebase against the new
> mount API conversion led to a lot of non-trivial conflicts.
>
> Josef kindly ran it through the CI farm and provided a fixup based on that.
>
> ---
> Christoph Hellwig (5):
> btrfs: always open the device read-only in btrfs_scan_one_device
> btrfs: call btrfs_close_devices from ->kill_sb
> btrfs: split btrfs_fs_devices.opened
> btrfs: open block devices after superblock creation
> btrfs: use the super_block as holder when mounting file systems
>
> fs/btrfs/disk-io.c | 4 +--
> fs/btrfs/super.c | 71 ++++++++++++++++++++++++++++++------------------------
> fs/btrfs/volumes.c | 60 +++++++++++++++++++++++----------------------
> fs/btrfs/volumes.h | 8 +++---
> 4 files changed, 78 insertions(+), 65 deletions(-)
> ---
> base-commit: a50d41606b333e4364844987deb1060e7ea6c038
> change-id: 20240214-hch-device-open-309ef9c98c62
>
> Best regards,
> --
> Johannes Thumshirn <[email protected]>
>
On Wed, Feb 14, 2024 at 08:42:11AM -0800, Johannes Thumshirn wrote:
> This is a series I've picked up from Christoph, it changes the
> block_device's bdev holder from fs_type to the super block.
Thanks Johannes,
from a quick look the rebase looks good to me.