| Message ID | 20200918133439.23187-8-nborisov@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Remove struct extent_io_ops | expand |
On Fri, Sep 18, 2020 at 04:34:39PM +0300, Nikolay Borisov wrote:
You should really write changelogs for patches that not obviously
trivial.
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
On 21.09.20 г. 23:38 ч., David Sterba wrote: > On Fri, Sep 18, 2020 at 04:34:39PM +0300, Nikolay Borisov wrote: > > You should really write changelogs for patches that not obviously > trivial. > >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> > I thought this patch was rather self-explanatory - just removing no longer used struct and related functions but I guess that's just me given I have written the previous 6 patches, so will add a changelog in the next iteration. Thanks.
On Wed, Sep 23, 2020 at 09:25:50AM +0300, Nikolay Borisov wrote: > > > On 21.09.20 г. 23:38 ч., David Sterba wrote: > > On Fri, Sep 18, 2020 at 04:34:39PM +0300, Nikolay Borisov wrote: > > > > You should really write changelogs for patches that not obviously > > trivial. > > > >> Signed-off-by: Nikolay Borisov <nborisov@suse.com> > > > > > I thought this patch was rather self-explanatory - just removing no > longer used struct and related functions but I guess that's just me > given I have written the previous 6 patches, so will add a changelog in > the next iteration. Thanks. The subject says "remove some struct", but when somebody reads it it's missing the "..., because ..." part. It becomes clear after reading the patch that is' not used anymore, but there should be some clue in the changelog from the beginning.
On 23.09.20 г. 17:19 ч., David Sterba wrote: > On Wed, Sep 23, 2020 at 09:25:50AM +0300, Nikolay Borisov wrote: >> >> >> On 21.09.20 г. 23:38 ч., David Sterba wrote: >>> On Fri, Sep 18, 2020 at 04:34:39PM +0300, Nikolay Borisov wrote: >>> >>> You should really write changelogs for patches that not obviously >>> trivial. >>> >>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com> >>> >> >> >> I thought this patch was rather self-explanatory - just removing no >> longer used struct and related functions but I guess that's just me >> given I have written the previous 6 patches, so will add a changelog in >> the next iteration. Thanks. > > The subject says "remove some struct", but when somebody reads it it's > missing the "..., because ..." part. It becomes clear after reading the > patch that is' not used anymore, but there should be some clue in the > changelog from the beginning. > Since you are going to update the patches inline can you add the following: " Since it's no longer used just remove the function and any related code which was initialising it for inodes. No functional changes. "
On Wed, Sep 23, 2020 at 05:23:05PM +0300, Nikolay Borisov wrote: > > > On 23.09.20 г. 17:19 ч., David Sterba wrote: > > On Wed, Sep 23, 2020 at 09:25:50AM +0300, Nikolay Borisov wrote: > >> > >> > >> On 21.09.20 г. 23:38 ч., David Sterba wrote: > >>> On Fri, Sep 18, 2020 at 04:34:39PM +0300, Nikolay Borisov wrote: > >>> > >>> You should really write changelogs for patches that not obviously > >>> trivial. > >>> > >>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com> > >>> > >> > >> > >> I thought this patch was rather self-explanatory - just removing no > >> longer used struct and related functions but I guess that's just me > >> given I have written the previous 6 patches, so will add a changelog in > >> the next iteration. Thanks. > > > > The subject says "remove some struct", but when somebody reads it it's > > missing the "..., because ..." part. It becomes clear after reading the > > patch that is' not used anymore, but there should be some clue in the > > changelog from the beginning. > > > > Since you are going to update the patches inline can you add the following: > > " > Since it's no longer used just remove the function and any related code > which was initialising it for inodes. No functional changes. > " Will update. There are no direct functional changes, but the extent_io_tree is shrunk by 8 bytes and there are eg. 3 instances in the btrfs_inode, so this removes 24 bytes in total.
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h index 5fc18b7ab771..8e811debae57 100644 --- a/fs/btrfs/ctree.h +++ b/fs/btrfs/ctree.h @@ -3576,9 +3576,7 @@ static inline int btrfs_defrag_cancelled(struct btrfs_fs_info *fs_info) /* Sanity test specific functions */ #ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS -void btrfs_test_inode_set_ops(struct inode *inode); void btrfs_test_destroy_inode(struct inode *inode); - static inline int btrfs_is_testing(struct btrfs_fs_info *fs_info) { return test_bit(BTRFS_FS_STATE_DUMMY_FS_INFO, &fs_info->fs_state); diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 54f2b95cc305..85b59797a4a4 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -50,7 +50,6 @@ BTRFS_SUPER_FLAG_METADUMP |\ BTRFS_SUPER_FLAG_METADUMP_V2) -static const struct extent_io_ops btree_extent_io_ops; static void end_workqueue_fn(struct btrfs_work *work); static void btrfs_destroy_ordered_extents(struct btrfs_root *root); static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans, @@ -2066,8 +2065,6 @@ static void btrfs_init_btree_inode(struct btrfs_fs_info *fs_info) BTRFS_I(inode)->io_tree.track_uptodate = false; extent_map_tree_init(&BTRFS_I(inode)->extent_tree); - BTRFS_I(inode)->io_tree.ops = &btree_extent_io_ops; - BTRFS_I(inode)->root = btrfs_grab_root(fs_info->tree_root); memset(&BTRFS_I(inode)->location, 0, sizeof(struct btrfs_key)); set_bit(BTRFS_INODE_DUMMY, &BTRFS_I(inode)->runtime_flags); @@ -4634,7 +4631,3 @@ static int btrfs_cleanup_transaction(struct btrfs_fs_info *fs_info) return 0; } - -static const struct extent_io_ops btree_extent_io_ops = { - .submit_bio_hook = NULL -}; diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h index 250b8cbaaf97..b1b737e7ef5b 100644 --- a/fs/btrfs/extent-io-tree.h +++ b/fs/btrfs/extent-io-tree.h @@ -62,7 +62,6 @@ struct extent_io_tree { u8 owner; spinlock_t lock; - const struct extent_io_ops *ops; }; struct extent_state { diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 4a00cfd4082f..b5e00b62bcb1 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -281,7 +281,6 @@ void extent_io_tree_init(struct btrfs_fs_info *fs_info, { tree->fs_info = fs_info; tree->state = RB_ROOT; - tree->ops = NULL; tree->dirty_bytes = 0; spin_lock_init(&tree->lock); tree->private_data = private_data; @@ -3056,7 +3055,6 @@ static int submit_extent_page(unsigned int opf, else contig = bio_end_sector(bio) == sector; - ASSERT(tree->ops); if (btrfs_bio_fits_in_stripe(page, page_size, bio, bio_flags)) can_merge = false; diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 955a66207fec..befbe19996b8 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -71,7 +71,6 @@ static const struct inode_operations btrfs_special_inode_operations; static const struct inode_operations btrfs_file_inode_operations; static const struct address_space_operations btrfs_aops; static const struct file_operations btrfs_dir_file_operations; -static const struct extent_io_ops btrfs_extent_io_ops; static struct kmem_cache *btrfs_inode_cachep; struct kmem_cache *btrfs_trans_handle_cachep; @@ -141,13 +140,6 @@ static inline void btrfs_cleanup_ordered_extents(struct btrfs_inode *inode, static int btrfs_dirty_inode(struct inode *inode); -#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS -void btrfs_test_inode_set_ops(struct inode *inode) -{ - BTRFS_I(inode)->io_tree.ops = &btrfs_extent_io_ops; -} -#endif - static int btrfs_init_inode_security(struct btrfs_trans_handle *trans, struct inode *inode, struct inode *dir, const struct qstr *qstr) @@ -3376,7 +3368,6 @@ static int btrfs_read_locked_inode(struct inode *inode, switch (inode->i_mode & S_IFMT) { case S_IFREG: inode->i_mapping->a_ops = &btrfs_aops; - BTRFS_I(inode)->io_tree.ops = &btrfs_extent_io_ops; inode->i_fop = &btrfs_file_operations; inode->i_op = &btrfs_file_inode_operations; break; @@ -6292,7 +6283,6 @@ static int btrfs_create(struct inode *dir, struct dentry *dentry, if (err) goto out_unlock; - BTRFS_I(inode)->io_tree.ops = &btrfs_extent_io_ops; d_instantiate_new(dentry, inode); out_unlock: @@ -9504,7 +9494,6 @@ static int btrfs_symlink(struct inode *dir, struct dentry *dentry, inode->i_fop = &btrfs_file_operations; inode->i_op = &btrfs_file_inode_operations; inode->i_mapping->a_ops = &btrfs_aops; - BTRFS_I(inode)->io_tree.ops = &btrfs_extent_io_ops; err = btrfs_init_inode_security(trans, inode, dir, &dentry->d_name); if (err) @@ -9826,7 +9815,6 @@ static int btrfs_tmpfile(struct inode *dir, struct dentry *dentry, umode_t mode) inode->i_op = &btrfs_file_inode_operations; inode->i_mapping->a_ops = &btrfs_aops; - BTRFS_I(inode)->io_tree.ops = &btrfs_extent_io_ops; ret = btrfs_init_inode_security(trans, inode, dir, NULL); if (ret) @@ -10244,10 +10232,6 @@ static const struct file_operations btrfs_dir_file_operations = { .fsync = btrfs_sync_file, }; -static const struct extent_io_ops btrfs_extent_io_ops = { - .submit_bio_hook = NULL -}; - /* * btrfs doesn't support the bmap operation because swapfiles * use bmap to make a mapping of extents in the file. They assume diff --git a/fs/btrfs/tests/inode-tests.c b/fs/btrfs/tests/inode-tests.c index cc54d4973a74..e6719f7db386 100644 --- a/fs/btrfs/tests/inode-tests.c +++ b/fs/btrfs/tests/inode-tests.c @@ -949,7 +949,6 @@ static int test_extent_accounting(u32 sectorsize, u32 nodesize) } BTRFS_I(inode)->root = root; - btrfs_test_inode_set_ops(inode); /* [BTRFS_MAX_EXTENT_SIZE] */ ret = btrfs_set_extent_delalloc(BTRFS_I(inode), 0,
Signed-off-by: Nikolay Borisov <nborisov@suse.com> --- fs/btrfs/ctree.h | 2 -- fs/btrfs/disk-io.c | 7 ------- fs/btrfs/extent-io-tree.h | 1 - fs/btrfs/extent_io.c | 2 -- fs/btrfs/inode.c | 16 ---------------- fs/btrfs/tests/inode-tests.c | 1 - 6 files changed, 29 deletions(-)