From 61dbdeb870bbdab28cabe91f6fcbd320fcda4ff2 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Tue, 18 Feb 2025 01:36:53 +0100 Subject: [PATCH 01/16] btrfs: pass struct btrfs_inode to btrfs_extent_same_range() Pass a struct btrfs_inode to btrfs_extent_same_range() as it's an internal interface, allowing to remove some use of BTRFS_I. Reviewed-by: Johannes Thumshirn Signed-off-by: David Sterba --- fs/btrfs/reflink.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c index f3aa3f4e9684..63d4a4d22801 100644 --- a/fs/btrfs/reflink.c +++ b/fs/btrfs/reflink.c @@ -631,12 +631,12 @@ static void btrfs_double_mmap_unlock(struct btrfs_inode *inode1, struct btrfs_in up_write(&inode2->i_mmap_lock); } -static int btrfs_extent_same_range(struct inode *src, u64 loff, u64 len, - struct inode *dst, u64 dst_loff) +static int btrfs_extent_same_range(struct btrfs_inode *src, u64 loff, u64 len, + struct btrfs_inode *dst, u64 dst_loff) { const u64 end = dst_loff + len - 1; struct extent_state *cached_state = NULL; - struct btrfs_fs_info *fs_info = BTRFS_I(src)->root->fs_info; + struct btrfs_fs_info *fs_info = src->root->fs_info; const u64 bs = fs_info->sectorsize; int ret; @@ -646,9 +646,10 @@ static int btrfs_extent_same_range(struct inode *src, u64 loff, u64 len, * because we have already locked the inode's i_mmap_lock in exclusive * mode. */ - lock_extent(&BTRFS_I(dst)->io_tree, dst_loff, end, &cached_state); - ret = btrfs_clone(src, dst, loff, len, ALIGN(len, bs), dst_loff, 1); - unlock_extent(&BTRFS_I(dst)->io_tree, dst_loff, end, &cached_state); + lock_extent(&dst->io_tree, dst_loff, end, &cached_state); + ret = btrfs_clone(&src->vfs_inode, &dst->vfs_inode, loff, len, + ALIGN(len, bs), dst_loff, 1); + unlock_extent(&dst->io_tree, dst_loff, end, &cached_state); btrfs_btree_balance_dirty(fs_info); @@ -678,8 +679,8 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen, chunk_count = div_u64(olen, BTRFS_MAX_DEDUPE_LEN); for (i = 0; i < chunk_count; i++) { - ret = btrfs_extent_same_range(src, loff, BTRFS_MAX_DEDUPE_LEN, - dst, dst_loff); + ret = btrfs_extent_same_range(BTRFS_I(src), loff, BTRFS_MAX_DEDUPE_LEN, + BTRFS_I(dst), dst_loff); if (ret) goto out; @@ -688,7 +689,8 @@ static int btrfs_extent_same(struct inode *src, u64 loff, u64 olen, } if (tail_len > 0) - ret = btrfs_extent_same_range(src, loff, tail_len, dst, dst_loff); + ret = btrfs_extent_same_range(BTRFS_I(src), loff, tail_len, + BTRFS_I(dst), dst_loff); out: spin_lock(&root_dst->root_item_lock); root_dst->dedupe_in_progress--; -- 2.51.0 From 8b044e17e55aef2d64739a99f6f5d912a68f7f73 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Tue, 18 Feb 2025 01:39:43 +0100 Subject: [PATCH 02/16] btrfs: use struct btrfs_inode inside btrfs_remap_file_range() Use a struct btrfs_inode to btrfs_remap_file_range() as it's an internal helper, allowing to remove some use of BTRFS_I. Reviewed-by: Johannes Thumshirn Signed-off-by: David Sterba --- fs/btrfs/reflink.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c index 63d4a4d22801..f63f92e176c8 100644 --- a/fs/btrfs/reflink.c +++ b/fs/btrfs/reflink.c @@ -865,8 +865,8 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off, struct file *dst_file, loff_t destoff, loff_t len, unsigned int remap_flags) { - struct inode *src_inode = file_inode(src_file); - struct inode *dst_inode = file_inode(dst_file); + struct btrfs_inode *src_inode = BTRFS_I(file_inode(src_file)); + struct btrfs_inode *dst_inode = BTRFS_I(file_inode(dst_file)); bool same_inode = dst_inode == src_inode; int ret; @@ -874,10 +874,10 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off, return -EINVAL; if (same_inode) { - btrfs_inode_lock(BTRFS_I(src_inode), BTRFS_ILOCK_MMAP); + btrfs_inode_lock(src_inode, BTRFS_ILOCK_MMAP); } else { - lock_two_nondirectories(src_inode, dst_inode); - btrfs_double_mmap_lock(BTRFS_I(src_inode), BTRFS_I(dst_inode)); + lock_two_nondirectories(&src_inode->vfs_inode, &dst_inode->vfs_inode); + btrfs_double_mmap_lock(src_inode, dst_inode); } ret = btrfs_remap_file_range_prep(src_file, off, dst_file, destoff, @@ -886,16 +886,18 @@ loff_t btrfs_remap_file_range(struct file *src_file, loff_t off, goto out_unlock; if (remap_flags & REMAP_FILE_DEDUP) - ret = btrfs_extent_same(src_inode, off, len, dst_inode, destoff); + ret = btrfs_extent_same(&src_inode->vfs_inode, off, len, + &dst_inode->vfs_inode, destoff); else ret = btrfs_clone_files(dst_file, src_file, off, len, destoff); out_unlock: if (same_inode) { - btrfs_inode_unlock(BTRFS_I(src_inode), BTRFS_ILOCK_MMAP); + btrfs_inode_unlock(src_inode, BTRFS_ILOCK_MMAP); } else { - btrfs_double_mmap_unlock(BTRFS_I(src_inode), BTRFS_I(dst_inode)); - unlock_two_nondirectories(src_inode, dst_inode); + btrfs_double_mmap_unlock(src_inode, dst_inode); + unlock_two_nondirectories(&src_inode->vfs_inode, + &dst_inode->vfs_inode); } /* -- 2.51.0 From 2d4323ced58f949626a93c179f26dcee418ceb53 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Tue, 18 Feb 2025 01:43:49 +0100 Subject: [PATCH 03/16] btrfs: use struct btrfs_inode inside btrfs_remap_file_range_prep() Use a struct btrfs_inode in btrfs_remap_file_range_prep() as it's an internal helper, allowing to remove some use of BTRFS_I. Reviewed-by: Johannes Thumshirn Signed-off-by: David Sterba --- fs/btrfs/reflink.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c index f63f92e176c8..15c296cb4dac 100644 --- a/fs/btrfs/reflink.c +++ b/fs/btrfs/reflink.c @@ -777,24 +777,24 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in, struct file *file_out, loff_t pos_out, loff_t *len, unsigned int remap_flags) { - struct inode *inode_in = file_inode(file_in); - struct inode *inode_out = file_inode(file_out); - u64 bs = BTRFS_I(inode_out)->root->fs_info->sectorsize; + struct btrfs_inode *inode_in = BTRFS_I(file_inode(file_in)); + struct btrfs_inode *inode_out = BTRFS_I(file_inode(file_out)); + u64 bs = inode_out->root->fs_info->sectorsize; u64 wb_len; int ret; if (!(remap_flags & REMAP_FILE_DEDUP)) { - struct btrfs_root *root_out = BTRFS_I(inode_out)->root; + struct btrfs_root *root_out = inode_out->root; if (btrfs_root_readonly(root_out)) return -EROFS; - ASSERT(inode_in->i_sb == inode_out->i_sb); + ASSERT(inode_in->vfs_inode.i_sb == inode_out->vfs_inode.i_sb); } /* Don't make the dst file partly checksummed */ - if ((BTRFS_I(inode_in)->flags & BTRFS_INODE_NODATASUM) != - (BTRFS_I(inode_out)->flags & BTRFS_INODE_NODATASUM)) { + if ((inode_in->flags & BTRFS_INODE_NODATASUM) != + (inode_out->flags & BTRFS_INODE_NODATASUM)) { return -EINVAL; } @@ -813,7 +813,7 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in, * to complete so that new file extent items are in the fs tree. */ if (*len == 0 && !(remap_flags & REMAP_FILE_DEDUP)) - wb_len = ALIGN(inode_in->i_size, bs) - ALIGN_DOWN(pos_in, bs); + wb_len = ALIGN(inode_in->vfs_inode.i_size, bs) - ALIGN_DOWN(pos_in, bs); else wb_len = ALIGN(*len, bs); @@ -834,16 +834,14 @@ static int btrfs_remap_file_range_prep(struct file *file_in, loff_t pos_in, * Also we don't need to check ASYNC_EXTENT, as async extent will be * CoWed anyway, not affecting nocow part. */ - ret = filemap_flush(inode_in->i_mapping); + ret = filemap_flush(inode_in->vfs_inode.i_mapping); if (ret < 0) return ret; - ret = btrfs_wait_ordered_range(BTRFS_I(inode_in), ALIGN_DOWN(pos_in, bs), - wb_len); + ret = btrfs_wait_ordered_range(inode_in, ALIGN_DOWN(pos_in, bs), wb_len); if (ret < 0) return ret; - ret = btrfs_wait_ordered_range(BTRFS_I(inode_out), ALIGN_DOWN(pos_out, bs), - wb_len); + ret = btrfs_wait_ordered_range(inode_out, ALIGN_DOWN(pos_out, bs), wb_len); if (ret < 0) return ret; -- 2.51.0 From 8dddeb53ab1e11ea8a5b634a2d16586e5f93cd18 Mon Sep 17 00:00:00 2001 From: David Sterba Date: Tue, 18 Feb 2025 01:45:57 +0100 Subject: [PATCH 04/16] btrfs: use struct btrfs_inode inside btrfs_get_parent() Use a struct btrfs_inode to btrfs_get_parent() as it's an internal helper, allowing to remove some use of BTRFS_I. Reviewed-by: Johannes Thumshirn Signed-off-by: David Sterba --- fs/btrfs/export.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c index c087424ac067..ab4d8625ad55 100644 --- a/fs/btrfs/export.c +++ b/fs/btrfs/export.c @@ -145,9 +145,9 @@ static struct dentry *btrfs_fh_to_dentry(struct super_block *sb, struct fid *fh, struct dentry *btrfs_get_parent(struct dentry *child) { - struct inode *dir = d_inode(child); - struct btrfs_fs_info *fs_info = btrfs_sb(dir->i_sb); - struct btrfs_root *root = BTRFS_I(dir)->root; + struct btrfs_inode *dir = BTRFS_I(d_inode(child)); + struct btrfs_root *root = dir->root; + struct btrfs_fs_info *fs_info = root->fs_info; struct btrfs_path *path; struct extent_buffer *leaf; struct btrfs_root_ref *ref; @@ -159,13 +159,13 @@ struct dentry *btrfs_get_parent(struct dentry *child) if (!path) return ERR_PTR(-ENOMEM); - if (btrfs_ino(BTRFS_I(dir)) == BTRFS_FIRST_FREE_OBJECTID) { + if (btrfs_ino(dir) == BTRFS_FIRST_FREE_OBJECTID) { key.objectid = btrfs_root_id(root); key.type = BTRFS_ROOT_BACKREF_KEY; key.offset = (u64)-1; root = fs_info->tree_root; } else { - key.objectid = btrfs_ino(BTRFS_I(dir)); + key.objectid = btrfs_ino(dir); key.type = BTRFS_INODE_REF_KEY; key.offset = (u64)-1; } -- 2.51.0 From a4bb776cbe4ce8ad5f71667a75d1a5bef53f399f Mon Sep 17 00:00:00 2001 From: David Sterba Date: Tue, 18 Feb 2025 01:48:21 +0100 Subject: [PATCH 05/16] btrfs: use struct btrfs_inode inside btrfs_get_name() Use a struct btrfs_inode in btrfs_get_name() as it's an internal helper, allowing to remove some use of BTRFS_I. Reviewed-by: Johannes Thumshirn Signed-off-by: David Sterba --- fs/btrfs/export.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/export.c b/fs/btrfs/export.c index ab4d8625ad55..0c0b8db82df6 100644 --- a/fs/btrfs/export.c +++ b/fs/btrfs/export.c @@ -219,11 +219,11 @@ fail: static int btrfs_get_name(struct dentry *parent, char *name, struct dentry *child) { - struct inode *inode = d_inode(child); - struct inode *dir = d_inode(parent); - struct btrfs_fs_info *fs_info = inode_to_fs_info(inode); + struct btrfs_inode *inode = BTRFS_I(d_inode(child)); + struct btrfs_inode *dir = BTRFS_I(d_inode(parent)); + struct btrfs_root *root = dir->root; + struct btrfs_fs_info *fs_info = root->fs_info; struct btrfs_path *path; - struct btrfs_root *root = BTRFS_I(dir)->root; struct btrfs_inode_ref *iref; struct btrfs_root_ref *rref; struct extent_buffer *leaf; @@ -233,24 +233,24 @@ static int btrfs_get_name(struct dentry *parent, char *name, int ret; u64 ino; - if (!S_ISDIR(dir->i_mode)) + if (!S_ISDIR(dir->vfs_inode.i_mode)) return -EINVAL; - ino = btrfs_ino(BTRFS_I(inode)); + ino = btrfs_ino(inode); path = btrfs_alloc_path(); if (!path) return -ENOMEM; if (ino == BTRFS_FIRST_FREE_OBJECTID) { - key.objectid = btrfs_root_id(BTRFS_I(inode)->root); + key.objectid = btrfs_root_id(inode->root); key.type = BTRFS_ROOT_BACKREF_KEY; key.offset = (u64)-1; root = fs_info->tree_root; } else { key.objectid = ino; key.type = BTRFS_INODE_REF_KEY; - key.offset = btrfs_ino(BTRFS_I(dir)); + key.offset = btrfs_ino(dir); } ret = btrfs_search_slot(NULL, root, &key, path, 0, 0); -- 2.51.0 From 75dfc5d0cabb9dfb071583e1e30facee6a113227 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Thu, 19 Sep 2024 11:32:06 +0100 Subject: [PATCH 06/16] btrfs: send: remove duplicated logic from fs_path_reset() There's duplicated logic in both branches of the if statement, so move it outside the branches. This also reduces the object code size. Before this change: $ size fs/btrfs/btrfs.ko text data bss dec hex filename 1746279 163600 16920 1926799 1d668f fs/btrfs/btrfs.ko After this change: $ size fs/btrfs/btrfs.ko text data bss dec hex filename 1746047 163592 16920 1926559 1d659f fs/btrfs/btrfs.ko Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index d513f7fd5fe8..8de561fb1390 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -424,15 +424,13 @@ static int need_send_hole(struct send_ctx *sctx) static void fs_path_reset(struct fs_path *p) { - if (p->reversed) { + if (p->reversed) p->start = p->buf + p->buf_len - 1; - p->end = p->start; - *p->start = 0; - } else { + else p->start = p->buf; - p->end = p->start; - *p->start = 0; - } + + p->end = p->start; + *p->start = 0; } static struct fs_path *fs_path_alloc(void) -- 2.51.0 From 920e8ee2bfcaf886fd8c0ad9df097a7dddfeb2d8 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Tue, 4 Feb 2025 16:41:01 +0000 Subject: [PATCH 07/16] btrfs: send: make fs_path_len() inline and constify its argument The helper function fs_path_len() is trivial and doesn't need to change its path argument, so make it inline and constify the argument. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 8de561fb1390..4e998bf8d379 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -468,7 +468,7 @@ static void fs_path_free(struct fs_path *p) kfree(p); } -static int fs_path_len(struct fs_path *p) +static inline int fs_path_len(const struct fs_path *p) { return p->end - p->start; } -- 2.51.0 From 1f63d4b610181c1f6add492c9c23f27818aff361 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 11:14:09 +0000 Subject: [PATCH 08/16] btrfs: send: always use fs_path_len() to determine a path's length Several places are hardcoding the path length calculation instead of using the helper fs_path_len() for that. Update all those places to instead use fs_path_len(). Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 4e998bf8d379..9f9885dc1e10 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -489,7 +489,7 @@ static int fs_path_ensure_buf(struct fs_path *p, int len) return -ENOMEM; } - path_len = p->end - p->start; + path_len = fs_path_len(p); old_buf_len = p->buf_len; /* @@ -530,7 +530,7 @@ static int fs_path_prepare_for_add(struct fs_path *p, int name_len, int ret; int new_len; - new_len = p->end - p->start + name_len; + new_len = fs_path_len(p) + name_len; if (p->start != p->end) new_len++; ret = fs_path_ensure_buf(p, new_len); @@ -571,12 +571,13 @@ out: static int fs_path_add_path(struct fs_path *p, struct fs_path *p2) { int ret; + const int p2_len = fs_path_len(p2); char *prepared; - ret = fs_path_prepare_for_add(p, p2->end - p2->start, &prepared); + ret = fs_path_prepare_for_add(p, p2_len, &prepared); if (ret < 0) goto out; - memcpy(prepared, p2->start, p2->end - p2->start); + memcpy(prepared, p2->start, p2_len); out: return ret; @@ -616,7 +617,7 @@ static void fs_path_unreverse(struct fs_path *p) return; tmp = p->start; - len = p->end - p->start; + len = fs_path_len(p); p->start = p->buf; p->end = p->start + len; memmove(p->start, tmp, len + 1); @@ -737,7 +738,7 @@ static int tlv_put_btrfs_timespec(struct send_ctx *sctx, u16 attr, #define TLV_PUT_PATH(sctx, attrtype, p) \ do { \ ret = tlv_put_string(sctx, attrtype, p->start, \ - p->end - p->start); \ + fs_path_len((p))); \ if (ret < 0) \ goto tlv_put_failure; \ } while(0) @@ -2364,7 +2365,7 @@ static int __get_cur_name_and_parent(struct send_ctx *sctx, * earlier. If yes, treat as orphan and return 1. */ ret = did_overwrite_ref(sctx, *parent_ino, *parent_gen, ino, gen, - dest->start, dest->end - dest->start); + dest->start, fs_path_len(dest)); if (ret < 0) goto out; if (ret) { -- 2.51.0 From 147ff868609b99b48fb67cad54b41372cac23f3b Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 11:50:48 +0000 Subject: [PATCH 09/16] btrfs: send: simplify return logic from fs_path_prepare_for_add() There is no need to have an 'out' label and jump into it since there are no resource cleanups to perform (release locks, free memory, etc), so make this simpler by removing the label and goto and instead return directly. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 9f9885dc1e10..535384028cb8 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -535,7 +535,7 @@ static int fs_path_prepare_for_add(struct fs_path *p, int name_len, new_len++; ret = fs_path_ensure_buf(p, new_len); if (ret < 0) - goto out; + return ret; if (p->reversed) { if (p->start != p->end) @@ -550,8 +550,7 @@ static int fs_path_prepare_for_add(struct fs_path *p, int name_len, *p->end = 0; } -out: - return ret; + return 0; } static int fs_path_add(struct fs_path *p, const char *name, int name_len) -- 2.51.0 From c727371879488449a71d8ee7e199b30290ad0953 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 11:54:07 +0000 Subject: [PATCH 10/16] btrfs: send: simplify return logic from fs_path_add() There is no need to have an 'out' label and jump into it since there are no resource cleanups to perform (release locks, free memory, etc), so make this simpler by removing the label and goto and instead return directly. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 535384028cb8..2203745569e0 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -560,11 +560,10 @@ static int fs_path_add(struct fs_path *p, const char *name, int name_len) ret = fs_path_prepare_for_add(p, name_len, &prepared); if (ret < 0) - goto out; + return ret; memcpy(prepared, name, name_len); -out: - return ret; + return 0; } static int fs_path_add_path(struct fs_path *p, struct fs_path *p2) -- 2.51.0 From a3d37502e7d9aca163459c6d715a5e79250aeb33 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 12:23:11 +0000 Subject: [PATCH 11/16] btrfs: send: implement fs_path_add_path() using fs_path_add() The helper fs_path_add_path() is basically a copy of fs_path_add() and it can be made a wrapper around fs_path_add(). So do that and also make it inline and constify its second argument. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 2203745569e0..7a75f1d963f9 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -566,19 +566,9 @@ static int fs_path_add(struct fs_path *p, const char *name, int name_len) return 0; } -static int fs_path_add_path(struct fs_path *p, struct fs_path *p2) +static inline int fs_path_add_path(struct fs_path *p, const struct fs_path *p2) { - int ret; - const int p2_len = fs_path_len(p2); - char *prepared; - - ret = fs_path_prepare_for_add(p, p2_len, &prepared); - if (ret < 0) - goto out; - memcpy(prepared, p2->start, p2_len); - -out: - return ret; + return fs_path_add(p, p2->start, fs_path_len(p2)); } static int fs_path_add_from_extent_buffer(struct fs_path *p, -- 2.51.0 From 78843d7e4e8b2cd85754c121d3f636196c01045d Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 12:33:15 +0000 Subject: [PATCH 12/16] btrfs: send: simplify return logic from fs_path_add_from_extent_buffer() There is no need to have an 'out' label and jump into it since there are no resource cleanups to perform (release locks, free memory, etc), so make this simpler by removing the label and goto and instead return directly. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 7a75f1d963f9..b9de1ab94367 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -580,12 +580,11 @@ static int fs_path_add_from_extent_buffer(struct fs_path *p, ret = fs_path_prepare_for_add(p, len, &prepared); if (ret < 0) - goto out; + return ret; read_extent_buffer(eb, prepared, off, len); -out: - return ret; + return 0; } static int fs_path_copy(struct fs_path *p, struct fs_path *from) -- 2.51.0 From a77749b3e21813566cea050bbb3414ae74562eba Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 13:09:25 +0000 Subject: [PATCH 13/16] btrfs: send: return -ENAMETOOLONG when attempting a path that is too long When attempting to build a too long path we are currently returning -ENOMEM, which is very odd and misleading. So update fs_path_ensure_buf() to return -ENAMETOOLONG instead. Also, while at it, move the WARN_ON() into the if statement's expression, as it makes it clear what is being tested and also has the effect of adding 'unlikely' to the statement, which allows the compiler to generate better code as this condition is never expected to happen. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index b9de1ab94367..dcc1cf7d1dbd 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -484,10 +484,8 @@ static int fs_path_ensure_buf(struct fs_path *p, int len) if (p->buf_len >= len) return 0; - if (len > PATH_MAX) { - WARN_ON(1); - return -ENOMEM; - } + if (WARN_ON(len > PATH_MAX)) + return -ENAMETOOLONG; path_len = fs_path_len(p); old_buf_len = p->buf_len; -- 2.51.0 From dbee3fc55ac156006c47e71de3ed41a2307690c3 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Mon, 10 Feb 2025 11:38:47 +0000 Subject: [PATCH 14/16] btrfs: send: simplify return logic from __get_cur_name_and_parent() There is no need to have an 'out' label and jump into it since there are no resource cleanups to perform (release locks, free memory, etc), so make this simpler by removing the label and goto and instead return directly. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index dcc1cf7d1dbd..393c9ca5de90 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -2309,9 +2309,8 @@ static int __get_cur_name_and_parent(struct send_ctx *sctx, *parent_gen = nce->parent_gen; ret = fs_path_add(dest, nce->name, nce->name_len); if (ret < 0) - goto out; - ret = nce->ret; - goto out; + return ret; + return nce->ret; } } @@ -2322,12 +2321,12 @@ static int __get_cur_name_and_parent(struct send_ctx *sctx, */ ret = is_inode_existent(sctx, ino, gen, NULL, NULL); if (ret < 0) - goto out; + return ret; if (!ret) { ret = gen_unique_name(sctx, ino, gen, dest); if (ret < 0) - goto out; + return ret; ret = 1; goto out_cache; } @@ -2343,7 +2342,7 @@ static int __get_cur_name_and_parent(struct send_ctx *sctx, ret = get_first_ref(sctx->parent_root, ino, parent_ino, parent_gen, dest); if (ret < 0) - goto out; + return ret; /* * Check if the ref was overwritten by an inode's ref that was processed @@ -2352,12 +2351,12 @@ static int __get_cur_name_and_parent(struct send_ctx *sctx, ret = did_overwrite_ref(sctx, *parent_ino, *parent_gen, ino, gen, dest->start, fs_path_len(dest)); if (ret < 0) - goto out; + return ret; if (ret) { fs_path_reset(dest); ret = gen_unique_name(sctx, ino, gen, dest); if (ret < 0) - goto out; + return ret; ret = 1; } @@ -2366,10 +2365,8 @@ out_cache: * Store the result of the lookup in the name cache. */ nce = kmalloc(sizeof(*nce) + fs_path_len(dest), GFP_KERNEL); - if (!nce) { - ret = -ENOMEM; - goto out; - } + if (!nce) + return -ENOMEM; nce->entry.key = ino; nce->entry.gen = gen; @@ -2387,10 +2384,9 @@ out_cache: nce_ret = btrfs_lru_cache_store(&sctx->name_cache, &nce->entry, GFP_KERNEL); if (nce_ret < 0) { kfree(nce); - ret = nce_ret; + return nce_ret; } -out: return ret; } -- 2.51.0 From 6bb09d0c126334b950dc806180ad242fd6cc93ec Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Mon, 10 Feb 2025 11:45:06 +0000 Subject: [PATCH 15/16] btrfs: send: simplify return logic from is_inode_existent() There is no need to have an 'out' label and jump into it since there are no resource cleanups to perform (release locks, free memory, etc), so make this simpler by removing the label and goto and instead return directly. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 393c9ca5de90..0a908e1066a6 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -1950,17 +1950,14 @@ static int is_inode_existent(struct send_ctx *sctx, u64 ino, u64 gen, ret = get_cur_inode_state(sctx, ino, gen, send_gen, parent_gen); if (ret < 0) - goto out; + return ret; if (ret == inode_state_no_change || ret == inode_state_did_create || ret == inode_state_will_delete) - ret = 1; - else - ret = 0; + return 1; -out: - return ret; + return 0; } /* -- 2.51.0 From 91e9139e5b2531c53e165114227134d68b17c4f5 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Mon, 10 Feb 2025 11:46:26 +0000 Subject: [PATCH 16/16] btrfs: send: simplify return logic from get_cur_inode_state() There is no need to have an 'out' label and jump into it since there are no resource cleanups to perform (release locks, free memory, etc), so make this simpler by removing the label and goto and instead return directly. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 0a908e1066a6..e0e24ac94aac 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -1880,7 +1880,7 @@ static int get_cur_inode_state(struct send_ctx *sctx, u64 ino, u64 gen, ret = get_inode_info(sctx->send_root, ino, &info); if (ret < 0 && ret != -ENOENT) - goto out; + return ret; left_ret = (info.nlink == 0) ? -ENOENT : ret; left_gen = info.gen; if (send_gen) @@ -1891,7 +1891,7 @@ static int get_cur_inode_state(struct send_ctx *sctx, u64 ino, u64 gen, } else { ret = get_inode_info(sctx->parent_root, ino, &info); if (ret < 0 && ret != -ENOENT) - goto out; + return ret; right_ret = (info.nlink == 0) ? -ENOENT : ret; right_gen = info.gen; if (parent_gen) @@ -1936,7 +1936,6 @@ static int get_cur_inode_state(struct send_ctx *sctx, u64 ino, u64 gen, ret = -ENOENT; } -out: return ret; } -- 2.51.0