Skip to content

Commit 136d210

Browse files
adam900710kdave
authored andcommitted
btrfs: delay btrfs_open_devices() until super block is created
Currently btrfs always call btrfs_open_devices() before creating the super block. It's fine for now because: - No blk_holder_ops is provided - btrfs_fs_type is used as a holder This means no matter who wins the device opening race, the holder will be the same thus not affecting the later sget_fc() race. And since no blk_holder_ops is provided, no bdev operation is depending on the holder. But this will no longer be true if we want to (we indeed want) implement a proper blk_holder_ops using fs_holder_ops. This means we will need a proper super block as the bdev holder. To prepare for such change: - Add btrfs_fs_devices::holding member This will prevent btrfs_free_stale_devices() and btrfS_close_device() from deleting the fs_devices when there is another process trying to mount the fs. Along with the new member, here comes two helpers, btrfs_fs_devices_inc_holding() and btrfs_fs_devices_dec_holding(). This will allow us to hold an fs_devices without opening it. We can not hold uuid_mutex while calling sget_fc(), this will reverse the lock sequence with s_umount, causing a lockdep warning. - Delay btrfs_open_devices() until a super block is returned This means we have to hold the initial fs_devices first, then unlock uuid_mutex, call sget_fc(), then re-lock uuid_mutex, and decrease the holding number. For new super block case, we continue to btrfs_open_devices() with uuid_mutex hold. For existing super block case, we can unlock uuid_mutex and continue. Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>
1 parent 56660b9 commit 136d210

File tree

3 files changed

+66
-17
lines changed

3 files changed

+66
-17
lines changed

fs/btrfs/super.c

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1841,7 +1841,6 @@ static int btrfs_get_tree_super(struct fs_context *fc)
18411841
struct btrfs_fs_info *fs_info = fc->s_fs_info;
18421842
struct btrfs_fs_context *ctx = fc->fs_private;
18431843
struct btrfs_fs_devices *fs_devices = NULL;
1844-
struct block_device *bdev;
18451844
struct btrfs_device *device;
18461845
struct super_block *sb;
18471846
blk_mode_t mode = btrfs_open_mode(fc);
@@ -1860,23 +1859,26 @@ static int btrfs_get_tree_super(struct fs_context *fc)
18601859
mutex_unlock(&uuid_mutex);
18611860
return PTR_ERR(device);
18621861
}
1863-
18641862
fs_devices = device->fs_devices;
1865-
fs_info->fs_devices = fs_devices;
1866-
1867-
ret = btrfs_open_devices(fs_devices, mode, &btrfs_fs_type);
1863+
/*
1864+
* We can not hold uuid_mutex calling sget_fc(), it will lead to a
1865+
* locking order reversal with s_umount.
1866+
*
1867+
* So here we increase the holding number of fs_devices, this will ensure
1868+
* the fs_devices itself won't be freed.
1869+
*/
1870+
btrfs_fs_devices_inc_holding(fs_devices);
18681871
mutex_unlock(&uuid_mutex);
1869-
if (ret)
1870-
return ret;
18711872

1872-
if (!(fc->sb_flags & SB_RDONLY) && fs_devices->rw_devices == 0)
1873-
return -EACCES;
1874-
1875-
bdev = fs_devices->latest_dev->bdev;
1873+
fs_info->fs_devices = fs_devices;
18761874

18771875
sb = sget_fc(fc, btrfs_fc_test_super, set_anon_super_fc);
1878-
if (IS_ERR(sb))
1876+
if (IS_ERR(sb)) {
1877+
mutex_lock(&uuid_mutex);
1878+
btrfs_fs_devices_dec_holding(fs_devices);
1879+
mutex_unlock(&uuid_mutex);
18791880
return PTR_ERR(sb);
1881+
}
18801882

18811883
set_device_specific_options(fs_info);
18821884

@@ -1888,26 +1890,47 @@ static int btrfs_get_tree_super(struct fs_context *fc)
18881890
*
18891891
* fc->s_fs_info is not touched and will be later freed by
18901892
* put_fs_context() through btrfs_free_fs_context().
1891-
*
1892-
* And the fs_info->fs_devices will also be closed by
1893-
* btrfs_free_fs_context().
18941893
*/
18951894
ASSERT(fc->s_fs_info == fs_info);
18961895

1896+
mutex_lock(&uuid_mutex);
1897+
btrfs_fs_devices_dec_holding(fs_devices);
1898+
mutex_unlock(&uuid_mutex);
1899+
/*
1900+
* But the fs_info->fs_devices is not opened, we should not let
1901+
* btrfs_free_fs_context() to close them.
1902+
*/
1903+
fs_info->fs_devices = NULL;
1904+
18971905
/*
18981906
* At this stage we may have RO flag mismatch between
18991907
* fc->sb_flags and sb->s_flags. Caller should detect such
19001908
* mismatch and reconfigure with sb->s_umount rwsem held if
19011909
* needed.
19021910
*/
19031911
} else {
1912+
struct block_device *bdev;
1913+
19041914
/*
19051915
* The first mount of the fs thus a new superblock, fc->s_fs_info
19061916
* should be NULL, and the owner ship of our fs_info and fs_devices is
19071917
* transferred to the super block.
19081918
*/
19091919
ASSERT(fc->s_fs_info == NULL);
19101920

1921+
mutex_lock(&uuid_mutex);
1922+
btrfs_fs_devices_dec_holding(fs_devices);
1923+
ret = btrfs_open_devices(fs_devices, mode, &btrfs_fs_type);
1924+
mutex_unlock(&uuid_mutex);
1925+
if (ret < 0) {
1926+
deactivate_locked_super(sb);
1927+
return ret;
1928+
}
1929+
if (!(fc->sb_flags & SB_RDONLY) && fs_devices->rw_devices == 0) {
1930+
deactivate_locked_super(sb);
1931+
return -EACCES;
1932+
}
1933+
bdev = fs_devices->latest_dev->bdev;
19111934
snprintf(sb->s_id, sizeof(sb->s_id), "%pg", bdev);
19121935
shrinker_debugfs_rename(sb->s_shrink, "sb-btrfs:%s", sb->s_id);
19131936
btrfs_sb(sb)->bdev_holder = &btrfs_fs_type;

fs/btrfs/volumes.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,7 @@ static void free_fs_devices(struct btrfs_fs_devices *fs_devices)
415415
struct btrfs_device *device;
416416

417417
WARN_ON(fs_devices->opened);
418+
WARN_ON(fs_devices->holding);
418419
while (!list_empty(&fs_devices->devices)) {
419420
device = list_first_entry(&fs_devices->devices,
420421
struct btrfs_device, dev_list);
@@ -542,7 +543,7 @@ static int btrfs_free_stale_devices(dev_t devt, struct btrfs_device *skip_device
542543
continue;
543544
if (devt && devt != device->devt)
544545
continue;
545-
if (fs_devices->opened) {
546+
if (fs_devices->opened || fs_devices->holding) {
546547
if (devt)
547548
ret = -EBUSY;
548549
break;
@@ -1198,7 +1199,7 @@ void btrfs_close_devices(struct btrfs_fs_devices *fs_devices)
11981199

11991200
mutex_lock(&uuid_mutex);
12001201
close_fs_devices(fs_devices);
1201-
if (!fs_devices->opened) {
1202+
if (!fs_devices->opened && !fs_devices->holding) {
12021203
list_splice_init(&fs_devices->seed_list, &list);
12031204

12041205
/*

fs/btrfs/volumes.h

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,17 @@ struct btrfs_fs_devices {
422422
/* Count fs-devices opened. */
423423
int opened;
424424

425+
/*
426+
* Counter of the processes that are holding this fs_devices but not
427+
* yet opened.
428+
* This is for mounting handling, as we can only open the fs_devices
429+
* after a super block is created.
430+
* But we can not hold uuid_mutex during sget_fc(), thus we have to
431+
* hold the fs_devices (meaning it can not be released) until a super
432+
* block is returned.
433+
*/
434+
int holding;
435+
425436
/* Set when we find or add a device that doesn't have the nonrot flag set. */
426437
bool rotating;
427438
/* Devices support TRIM/discard commands. */
@@ -854,6 +865,20 @@ static inline void btrfs_warn_unknown_chunk_allocation(enum btrfs_chunk_allocati
854865
WARN_ONCE(1, "unknown allocation policy %d, fallback to regular", pol);
855866
}
856867

868+
static inline void btrfs_fs_devices_inc_holding(struct btrfs_fs_devices *fs_devices)
869+
{
870+
lockdep_assert_held(&uuid_mutex);
871+
ASSERT(fs_devices->holding >= 0);
872+
fs_devices->holding++;
873+
}
874+
875+
static inline void btrfs_fs_devices_dec_holding(struct btrfs_fs_devices *fs_devices)
876+
{
877+
lockdep_assert_held(&uuid_mutex);
878+
ASSERT(fs_devices->holding > 0);
879+
fs_devices->holding--;
880+
}
881+
857882
void btrfs_commit_device_sizes(struct btrfs_transaction *trans);
858883

859884
struct list_head * __attribute_const__ btrfs_get_fs_uuids(void);

0 commit comments

Comments
 (0)