| Message ID | 20230802154131.2221419-8-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9c09a7cf6220a11dac0f4845b7e8925706cdc458 |
| Headers | show |
| Series | [f2fs-dev,01/12] fs: export setup_bdev_super | expand |
On Wed 02-08-23 17:41:26, Christoph Hellwig wrote: > fs_mark_dead currently uses get_super to find the superblock for the > block device that is going away. This means it is limited to the > main device stored in sb->s_dev, leading to a lot of code duplication > for file systems that can use multiple block devices. > > Now that the holder for all block devices used by file systems is set > to the super_block, we can instead look at that holder and then check > if the file system is born and active, so do that instead. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > fs/super.c | 30 ++++++++++++++++++++++++++---- > 1 file changed, 26 insertions(+), 4 deletions(-) > > diff --git a/fs/super.c b/fs/super.c > index 09b65ee1a8b737..0cda4af0a7e16c 100644 > --- a/fs/super.c > +++ b/fs/super.c > @@ -1209,17 +1209,39 @@ int get_tree_keyed(struct fs_context *fc, > EXPORT_SYMBOL(get_tree_keyed); > > #ifdef CONFIG_BLOCK > +/* > + * Lock a super block that the callers holds a reference to. > + * > + * The caller needs to ensure that the super_block isn't being freed while > + * calling this function, e.g. by holding a lock over the call to this function > + * and the place that clears the pointer to the superblock used by this function > + * before freeing the superblock. > + */ > +static bool lock_active_super(struct super_block *sb) > +{ > + down_read(&sb->s_umount); > + if (!sb->s_root || > + (sb->s_flags & (SB_ACTIVE | SB_BORN)) != (SB_ACTIVE | SB_BORN)) { > + up_read(&sb->s_umount); > + return false; > + } > + return true; > +} > + > static void fs_mark_dead(struct block_device *bdev) > { > - struct super_block *sb; > + struct super_block *sb = bdev->bd_holder; > > - sb = get_super(bdev); > - if (!sb) > + /* bd_holder_lock ensures that the sb isn't freed */ > + lockdep_assert_held(&bdev->bd_holder_lock); > + > + if (!lock_active_super(sb)) > return; > > if (sb->s_op->shutdown) > sb->s_op->shutdown(sb); > - drop_super(sb); > + > + up_read(&sb->s_umount); > } > > static const struct blk_holder_ops fs_holder_ops = { > -- > 2.39.2 >
On Wed, Aug 02, 2023 at 05:41:26PM +0200, Christoph Hellwig wrote: > fs_mark_dead currently uses get_super to find the superblock for the > block device that is going away. This means it is limited to the > main device stored in sb->s_dev, leading to a lot of code duplication > for file systems that can use multiple block devices. > > Now that the holder for all block devices used by file systems is set > to the super_block, we can instead look at that holder and then check > if the file system is born and active, so do that instead. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- Looks good to me, Reviewed-by: Christian Brauner <brauner@kernel.org>
diff --git a/fs/super.c b/fs/super.c index 09b65ee1a8b737..0cda4af0a7e16c 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1209,17 +1209,39 @@ int get_tree_keyed(struct fs_context *fc, EXPORT_SYMBOL(get_tree_keyed); #ifdef CONFIG_BLOCK +/* + * Lock a super block that the callers holds a reference to. + * + * The caller needs to ensure that the super_block isn't being freed while + * calling this function, e.g. by holding a lock over the call to this function + * and the place that clears the pointer to the superblock used by this function + * before freeing the superblock. + */ +static bool lock_active_super(struct super_block *sb) +{ + down_read(&sb->s_umount); + if (!sb->s_root || + (sb->s_flags & (SB_ACTIVE | SB_BORN)) != (SB_ACTIVE | SB_BORN)) { + up_read(&sb->s_umount); + return false; + } + return true; +} + static void fs_mark_dead(struct block_device *bdev) { - struct super_block *sb; + struct super_block *sb = bdev->bd_holder; - sb = get_super(bdev); - if (!sb) + /* bd_holder_lock ensures that the sb isn't freed */ + lockdep_assert_held(&bdev->bd_holder_lock); + + if (!lock_active_super(sb)) return; if (sb->s_op->shutdown) sb->s_op->shutdown(sb); - drop_super(sb); + + up_read(&sb->s_umount); } static const struct blk_holder_ops fs_holder_ops = {
fs_mark_dead currently uses get_super to find the superblock for the block device that is going away. This means it is limited to the main device stored in sb->s_dev, leading to a lot of code duplication for file systems that can use multiple block devices. Now that the holder for all block devices used by file systems is set to the super_block, we can instead look at that holder and then check if the file system is born and active, so do that instead. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/super.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-)