| Message ID | 20230927-vfs-super-freeze-v1-5-ecc36d9ab4d9@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement freeze and thaw as holder operations | expand |
On Wed, Sep 27, 2023 at 03:21:18PM +0200, Christian Brauner wrote: > Both bd_fsfreeze_mutex and bd_fsfreeze_sb are now unused and can be > removed. Also move bd_fsfreeze_count down to not have it weirdly placed > in the middle of the holder fields. > > Suggested-by: Jan Kara <jack@suse.cz> > Suggested-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Christian Brauner <brauner@kernel.org> > --- > block/bdev.c | 1 - > include/linux/blk_types.h | 7 ++----- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/block/bdev.c b/block/bdev.c > index 3deccd0ffcf2..084855b669f7 100644 > --- a/block/bdev.c > +++ b/block/bdev.c > @@ -392,7 +392,6 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno) > mapping_set_gfp_mask(&inode->i_data, GFP_USER); > > bdev = I_BDEV(inode); > - mutex_init(&bdev->bd_fsfreeze_mutex); > spin_lock_init(&bdev->bd_size_lock); > mutex_init(&bdev->bd_holder_lock); > bdev->bd_partno = partno; > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index 88e1848b0869..0238236852b7 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -56,14 +56,11 @@ struct block_device { > void * bd_holder; Hmmm. get_bdev_super from patch 3 now requires that bd_holder is a pointer to a struct super_block. AFAICT it's only called in conjunction with fs_holder_ops, so I suggest that the declaration for that should grow a comment to that effect: /* * For filesystems, the @holder argument passed to blkdev_get_* and * bd_prepare_to_claim must point to a super_block object. */ extern const struct blk_holder_ops fs_holder_ops; This patch itself looks ok so: Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > const struct blk_holder_ops *bd_holder_ops; > struct mutex bd_holder_lock; > - /* The counter of freeze processes */ > - atomic_t bd_fsfreeze_count; > int bd_holders; > struct kobject *bd_holder_dir; > > - /* Mutex for freeze */ > - struct mutex bd_fsfreeze_mutex; > - struct super_block *bd_fsfreeze_sb; > + /* The counter of freeze processes */ > + atomic_t bd_fsfreeze_count; > > struct partition_meta_info *bd_meta_info; > #ifdef CONFIG_FAIL_MAKE_REQUEST > > -- > 2.34.1 >
On Wed, Sep 27, 2023 at 08:11:11AM -0700, Darrick J. Wong wrote: > On Wed, Sep 27, 2023 at 03:21:18PM +0200, Christian Brauner wrote: > > Both bd_fsfreeze_mutex and bd_fsfreeze_sb are now unused and can be > > removed. Also move bd_fsfreeze_count down to not have it weirdly placed > > in the middle of the holder fields. > > > > Suggested-by: Jan Kara <jack@suse.cz> > > Suggested-by: Christoph Hellwig <hch@lst.de> > > Signed-off-by: Christian Brauner <brauner@kernel.org> > > --- > > block/bdev.c | 1 - > > include/linux/blk_types.h | 7 ++----- > > 2 files changed, 2 insertions(+), 6 deletions(-) > > > > diff --git a/block/bdev.c b/block/bdev.c > > index 3deccd0ffcf2..084855b669f7 100644 > > --- a/block/bdev.c > > +++ b/block/bdev.c > > @@ -392,7 +392,6 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno) > > mapping_set_gfp_mask(&inode->i_data, GFP_USER); > > > > bdev = I_BDEV(inode); > > - mutex_init(&bdev->bd_fsfreeze_mutex); > > spin_lock_init(&bdev->bd_size_lock); > > mutex_init(&bdev->bd_holder_lock); > > bdev->bd_partno = partno; > > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > > index 88e1848b0869..0238236852b7 100644 > > --- a/include/linux/blk_types.h > > +++ b/include/linux/blk_types.h > > @@ -56,14 +56,11 @@ struct block_device { > > void * bd_holder; > > Hmmm. get_bdev_super from patch 3 now requires that bd_holder is a > pointer to a struct super_block. AFAICT it's only called in conjunction Yeah, it's documented in Documentations/filesystems/porting.rst as of 060e6c7d179e ("porting: document superblock as block device holder") which tries to explain differences between the old and new world in detail.
On Wed, Sep 27, 2023 at 08:11:11AM -0700, Darrick J. Wong wrote: > > @@ -56,14 +56,11 @@ struct block_device { > > void * bd_holder; > > Hmmm. get_bdev_super from patch 3 now requires that bd_holder is a > pointer to a struct super_block. AFAICT it's only called in conjunction > with fs_holder_ops, so I suggest that the declaration for that should > grow a comment to that effect: > > /* > * For filesystems, the @holder argument passed to blkdev_get_* and > * bd_prepare_to_claim must point to a super_block object. > */ > extern const struct blk_holder_ops fs_holder_ops; Note that this is not strictly speaking true, it is only true for file systems actually using fs_holder_ops, not file systems in general. That being said a comment to that extent is indeed useful.
On Wed 27-09-23 15:21:18, Christian Brauner wrote: > Both bd_fsfreeze_mutex and bd_fsfreeze_sb are now unused and can be > removed. Also move bd_fsfreeze_count down to not have it weirdly placed > in the middle of the holder fields. > > Suggested-by: Jan Kara <jack@suse.cz> > Suggested-by: Christoph Hellwig <hch@lst.de> > Signed-off-by: Christian Brauner <brauner@kernel.org> The patch looks good to me. Feel free to add: Reviewed-by: Jan Kara <jack@suse.cz> Honza > --- > block/bdev.c | 1 - > include/linux/blk_types.h | 7 ++----- > 2 files changed, 2 insertions(+), 6 deletions(-) > > diff --git a/block/bdev.c b/block/bdev.c > index 3deccd0ffcf2..084855b669f7 100644 > --- a/block/bdev.c > +++ b/block/bdev.c > @@ -392,7 +392,6 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno) > mapping_set_gfp_mask(&inode->i_data, GFP_USER); > > bdev = I_BDEV(inode); > - mutex_init(&bdev->bd_fsfreeze_mutex); > spin_lock_init(&bdev->bd_size_lock); > mutex_init(&bdev->bd_holder_lock); > bdev->bd_partno = partno; > diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h > index 88e1848b0869..0238236852b7 100644 > --- a/include/linux/blk_types.h > +++ b/include/linux/blk_types.h > @@ -56,14 +56,11 @@ struct block_device { > void * bd_holder; > const struct blk_holder_ops *bd_holder_ops; > struct mutex bd_holder_lock; > - /* The counter of freeze processes */ > - atomic_t bd_fsfreeze_count; > int bd_holders; > struct kobject *bd_holder_dir; > > - /* Mutex for freeze */ > - struct mutex bd_fsfreeze_mutex; > - struct super_block *bd_fsfreeze_sb; > + /* The counter of freeze processes */ > + atomic_t bd_fsfreeze_count; > > struct partition_meta_info *bd_meta_info; > #ifdef CONFIG_FAIL_MAKE_REQUEST > > -- > 2.34.1 >
diff --git a/block/bdev.c b/block/bdev.c index 3deccd0ffcf2..084855b669f7 100644 --- a/block/bdev.c +++ b/block/bdev.c @@ -392,7 +392,6 @@ struct block_device *bdev_alloc(struct gendisk *disk, u8 partno) mapping_set_gfp_mask(&inode->i_data, GFP_USER); bdev = I_BDEV(inode); - mutex_init(&bdev->bd_fsfreeze_mutex); spin_lock_init(&bdev->bd_size_lock); mutex_init(&bdev->bd_holder_lock); bdev->bd_partno = partno; diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h index 88e1848b0869..0238236852b7 100644 --- a/include/linux/blk_types.h +++ b/include/linux/blk_types.h @@ -56,14 +56,11 @@ struct block_device { void * bd_holder; const struct blk_holder_ops *bd_holder_ops; struct mutex bd_holder_lock; - /* The counter of freeze processes */ - atomic_t bd_fsfreeze_count; int bd_holders; struct kobject *bd_holder_dir; - /* Mutex for freeze */ - struct mutex bd_fsfreeze_mutex; - struct super_block *bd_fsfreeze_sb; + /* The counter of freeze processes */ + atomic_t bd_fsfreeze_count; struct partition_meta_info *bd_meta_info; #ifdef CONFIG_FAIL_MAKE_REQUEST
Both bd_fsfreeze_mutex and bd_fsfreeze_sb are now unused and can be removed. Also move bd_fsfreeze_count down to not have it weirdly placed in the middle of the holder fields. Suggested-by: Jan Kara <jack@suse.cz> Suggested-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Christian Brauner <brauner@kernel.org> --- block/bdev.c | 1 - include/linux/blk_types.h | 7 ++----- 2 files changed, 2 insertions(+), 6 deletions(-)