Messages in this thread | | | Date | Wed, 14 Feb 2024 10:58:09 -0800 | From | Boris Burkov <> | Subject | Re: [PATCH 4/5] btrfs: open block devices after superblock creation |
| |
On Wed, Feb 14, 2024 at 08:42:15AM -0800, Johannes Thumshirn wrote: > From: Christoph Hellwig <hch@lst.de> > > Currently btrfs_mount_root opens the block devices before committing to > allocating a super block. That creates problems for restricting the > number of writers to a device, and also leads to a unusual and not very > helpful holder (the fs_type). > > Reorganize the code to first check whether the superblock for a > particular fsid does already exist and open the block devices only if it > doesn't, mirroring the recent changes to the VFS mount helpers. To do > this the increment of the in_use counter moves out of btrfs_open_devices > and into the only caller in btrfs_mount_root so that it happens before > dropping uuid_mutex around the call to sget.
I believe this commit message is now out of date as of 'btrfs: remove old mount API code' which got rid of btrfs_mount_root.
As far as I can tell, the code itself is updated and fine.
> > Signed-off-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > --- > fs/btrfs/super.c | 41 +++++++++++++++++++++++++---------------- > fs/btrfs/volumes.c | 15 +++++---------- > 2 files changed, 30 insertions(+), 26 deletions(-) > > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > index 51b8fd272b15..1fa7d83d02c1 100644 > --- a/fs/btrfs/super.c > +++ b/fs/btrfs/super.c > @@ -1794,7 +1794,6 @@ static int btrfs_get_tree_super(struct fs_context *fc) > struct btrfs_fs_info *fs_info = fc->s_fs_info; > struct btrfs_fs_context *ctx = fc->fs_private; > struct btrfs_fs_devices *fs_devices = NULL; > - struct block_device *bdev; > struct btrfs_device *device; > struct super_block *sb; > blk_mode_t mode = btrfs_open_mode(fc); > @@ -1817,15 +1816,8 @@ static int btrfs_get_tree_super(struct fs_context *fc) > fs_devices = device->fs_devices; > fs_info->fs_devices = fs_devices; > > - ret = btrfs_open_devices(fs_devices, mode, &btrfs_fs_type); > + fs_devices->in_use++; > mutex_unlock(&uuid_mutex); > - if (ret) > - return ret; > - > - if (!(fc->sb_flags & SB_RDONLY) && fs_devices->rw_devices == 0) > - return -EACCES; > - > - bdev = fs_devices->latest_dev->bdev; > > /* > * From now on the error handling is not straightforward. > @@ -1843,24 +1835,41 @@ static int btrfs_get_tree_super(struct fs_context *fc) > set_device_specific_options(fs_info); > > if (sb->s_root) { > - if ((fc->sb_flags ^ sb->s_flags) & SB_RDONLY) > + if ((fc->sb_flags ^ sb->s_flags) & SB_RDONLY) { > ret = -EBUSY; > + goto error_deactivate; > + } > } else { > - snprintf(sb->s_id, sizeof(sb->s_id), "%pg", bdev); > + struct btrfs_fs_devices *fs_devices = fs_info->fs_devices; > + > + mutex_lock(&uuid_mutex); > + ret = btrfs_open_devices(fs_devices, mode, &btrfs_fs_type); > + mutex_unlock(&uuid_mutex); > + if (ret) > + goto error_deactivate; > + > + if (!(fc->sb_flags & SB_RDONLY) && !fs_devices->rw_devices) { > + ret = -EACCES; > + goto error_deactivate; > + } > + > + 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; > ret = btrfs_fill_super(sb, fs_devices, NULL); > - } > - > - if (ret) { > - deactivate_locked_super(sb); > - return ret; > + if (ret) > + goto error_deactivate; > } > > btrfs_clear_oneshot_options(fs_info); > > fc->root = dget(sb->s_root); > return 0; > + > +error_deactivate: > + deactivate_locked_super(sb); > + return ret; > } > > /* > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index f27af155abf0..6e82bd7ce501 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1220,8 +1220,6 @@ static int devid_cmp(void *priv, const struct list_head *a, > int btrfs_open_devices(struct btrfs_fs_devices *fs_devices, > blk_mode_t flags, void *holder) > { > - int ret; > - > lockdep_assert_held(&uuid_mutex); > /* > * The device_list_mutex cannot be taken here in case opening the > @@ -1230,14 +1228,11 @@ 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->is_open) { > - list_sort(NULL, &fs_devices->devices, devid_cmp); > - ret = open_fs_devices(fs_devices, flags, holder); > - if (ret) > - return ret; > - } > - fs_devices->in_use++; > - return 0; > + ASSERT(fs_devices->in_use); > + if (fs_devices->is_open) > + return 0; > + list_sort(NULL, &fs_devices->devices, devid_cmp); > + return open_fs_devices(fs_devices, flags, holder); > } > > void btrfs_release_disk_super(struct btrfs_super_block *super) > > -- > 2.43.0 >
| |