From 75dfc5d0cabb9dfb071583e1e30facee6a113227 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Thu, 19 Sep 2024 11:32:06 +0100 Subject: [PATCH 01/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.50.1 From 920e8ee2bfcaf886fd8c0ad9df097a7dddfeb2d8 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Tue, 4 Feb 2025 16:41:01 +0000 Subject: [PATCH 02/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.50.1 From 1f63d4b610181c1f6add492c9c23f27818aff361 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 11:14:09 +0000 Subject: [PATCH 03/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.50.1 From 147ff868609b99b48fb67cad54b41372cac23f3b Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 11:50:48 +0000 Subject: [PATCH 04/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.50.1 From c727371879488449a71d8ee7e199b30290ad0953 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 11:54:07 +0000 Subject: [PATCH 05/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.50.1 From a3d37502e7d9aca163459c6d715a5e79250aeb33 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 12:23:11 +0000 Subject: [PATCH 06/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.50.1 From 78843d7e4e8b2cd85754c121d3f636196c01045d Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 12:33:15 +0000 Subject: [PATCH 07/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.50.1 From a77749b3e21813566cea050bbb3414ae74562eba Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 5 Feb 2025 13:09:25 +0000 Subject: [PATCH 08/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.50.1 From dbee3fc55ac156006c47e71de3ed41a2307690c3 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Mon, 10 Feb 2025 11:38:47 +0000 Subject: [PATCH 09/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.50.1 From 6bb09d0c126334b950dc806180ad242fd6cc93ec Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Mon, 10 Feb 2025 11:45:06 +0000 Subject: [PATCH 10/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.50.1 From 91e9139e5b2531c53e165114227134d68b17c4f5 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Mon, 10 Feb 2025 11:46:26 +0000 Subject: [PATCH 11/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.50.1 From 17f6a74d0b89092e38e3328b66eda1ab29a195d4 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Tue, 11 Feb 2025 11:01:15 +0000 Subject: [PATCH 12/16] btrfs: send: factor out common logic when sending xattrs We always send xattrs for the current inode only and both callers of send_set_xattr() pass a path for the current inode. So move the path allocation and computation to send_set_xattr(), reducing duplicated code. This also facilitates an upcoming patch. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 41 +++++++++++++++-------------------------- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index e0e24ac94aac..3aa2877f8c80 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -4844,11 +4844,19 @@ out: } static int send_set_xattr(struct send_ctx *sctx, - struct fs_path *path, const char *name, int name_len, const char *data, int data_len) { - int ret = 0; + struct fs_path *path; + int ret; + + path = fs_path_alloc(); + if (!path) + return -ENOMEM; + + ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, path); + if (ret < 0) + goto out; ret = begin_cmd(sctx, BTRFS_SEND_C_SET_XATTR); if (ret < 0) @@ -4862,6 +4870,8 @@ static int send_set_xattr(struct send_ctx *sctx, tlv_put_failure: out: + fs_path_free(path); + return ret; } @@ -4889,19 +4899,13 @@ static int __process_new_xattr(int num, struct btrfs_key *di_key, const char *name, int name_len, const char *data, int data_len, void *ctx) { - int ret; struct send_ctx *sctx = ctx; - struct fs_path *p; struct posix_acl_xattr_header dummy_acl; /* Capabilities are emitted by finish_inode_if_needed */ if (!strncmp(name, XATTR_NAME_CAPS, name_len)) return 0; - p = fs_path_alloc(); - if (!p) - return -ENOMEM; - /* * This hack is needed because empty acls are stored as zero byte * data in xattrs. Problem with that is, that receiving these zero byte @@ -4918,15 +4922,7 @@ static int __process_new_xattr(int num, struct btrfs_key *di_key, } } - ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, p); - if (ret < 0) - goto out; - - ret = send_set_xattr(sctx, p, name, name_len, data, data_len); - -out: - fs_path_free(p); - return ret; + return send_set_xattr(sctx, name, name_len, data, data_len); } static int __process_deleted_xattr(int num, struct btrfs_key *di_key, @@ -5803,7 +5799,6 @@ static int send_extent_data(struct send_ctx *sctx, struct btrfs_path *path, */ static int send_capabilities(struct send_ctx *sctx) { - struct fs_path *fspath = NULL; struct btrfs_path *path; struct btrfs_dir_item *di; struct extent_buffer *leaf; @@ -5829,25 +5824,19 @@ static int send_capabilities(struct send_ctx *sctx) leaf = path->nodes[0]; buf_len = btrfs_dir_data_len(leaf, di); - fspath = fs_path_alloc(); buf = kmalloc(buf_len, GFP_KERNEL); - if (!fspath || !buf) { + if (!buf) { ret = -ENOMEM; goto out; } - ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, fspath); - if (ret < 0) - goto out; - data_ptr = (unsigned long)(di + 1) + btrfs_dir_name_len(leaf, di); read_extent_buffer(leaf, buf, data_ptr, buf_len); - ret = send_set_xattr(sctx, fspath, XATTR_NAME_CAPS, + ret = send_set_xattr(sctx, XATTR_NAME_CAPS, strlen(XATTR_NAME_CAPS), buf, buf_len); out: kfree(buf); - fs_path_free(fspath); btrfs_free_path(path); return ret; } -- 2.50.1 From 9453fe329789073d9a971de01da5902c32c1a01a Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 12 Feb 2025 11:31:59 +0000 Subject: [PATCH 13/16] btrfs: send: only use boolean variables at process_recorded_refs() We have several local variables at process_recorded_refs() that are used as booleans, with some of them having a 'bool' type while two of them having an 'int' type. Change this to make them all use the 'bool' type which is more clear and to make everything more consistent. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 3aa2877f8c80..6e27a7d77b25 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -4147,9 +4147,9 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) u64 ow_inode = 0; u64 ow_gen; u64 ow_mode; - int did_overwrite = 0; - int is_orphan = 0; u64 last_dir_ino_rm = 0; + bool did_overwrite = false; + bool is_orphan = false; bool can_rename = true; bool orphanized_dir = false; bool orphanized_ancestor = false; @@ -4191,14 +4191,14 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) if (ret < 0) goto out; if (ret) - did_overwrite = 1; + did_overwrite = true; } if (sctx->cur_inode_new || did_overwrite) { ret = gen_unique_name(sctx, sctx->cur_ino, sctx->cur_inode_gen, valid_path); if (ret < 0) goto out; - is_orphan = 1; + is_orphan = true; } else { ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, valid_path); @@ -4421,7 +4421,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) ret = send_rename(sctx, valid_path, cur->full_path); if (ret < 0) goto out; - is_orphan = 0; + is_orphan = false; ret = fs_path_copy(valid_path, cur->full_path); if (ret < 0) goto out; @@ -4482,7 +4482,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) sctx->cur_inode_gen, valid_path); if (ret < 0) goto out; - is_orphan = 1; + is_orphan = true; } list_for_each_entry(cur, &sctx->deleted_refs, list) { -- 2.50.1 From ec666c84deba56f714505b53556a97565f72db86 Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Wed, 12 Feb 2025 11:49:47 +0000 Subject: [PATCH 14/16] btrfs: send: add and use helper to rename current inode when processing refs Extract the logic to rename the current inode at process_recorded_refs() into a helper function and use it, therefore removing duplicated logic and making it easier for an upcoming patch by avoiding yet more duplicated logic. Signed-off-by: Filipe Manana Reviewed-by: David Sterba Signed-off-by: David Sterba --- fs/btrfs/send.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 6e27a7d77b25..653e0b9a94ca 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -4133,6 +4133,19 @@ out: return ret; } +static int rename_current_inode(struct send_ctx *sctx, + struct fs_path *current_path, + struct fs_path *new_path) +{ + int ret; + + ret = send_rename(sctx, current_path, new_path); + if (ret < 0) + return ret; + + return fs_path_copy(current_path, new_path); +} + /* * This does all the move/link/unlink/rmdir magic. */ @@ -4418,13 +4431,10 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) * it depending on the inode mode. */ if (is_orphan && can_rename) { - ret = send_rename(sctx, valid_path, cur->full_path); + ret = rename_current_inode(sctx, valid_path, cur->full_path); if (ret < 0) goto out; is_orphan = false; - ret = fs_path_copy(valid_path, cur->full_path); - if (ret < 0) - goto out; } else if (can_rename) { if (S_ISDIR(sctx->cur_inode_mode)) { /* @@ -4432,10 +4442,7 @@ static int process_recorded_refs(struct send_ctx *sctx, int *pending_move) * dirs, we always have one new and one deleted * ref. The deleted ref is ignored later. */ - ret = send_rename(sctx, valid_path, - cur->full_path); - if (!ret) - ret = fs_path_copy(valid_path, + ret = rename_current_inode(sctx, valid_path, cur->full_path); if (ret < 0) goto out; -- 2.50.1 From 9435159f28c4911ca33a69d8fe197baf368261df Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Tue, 18 Feb 2025 15:36:01 +0000 Subject: [PATCH 15/16] btrfs: send: simplify return logic from send_remove_xattr() There's no need for the 'out' label as there are no resources to cleanup in case of an error and we can directly return if begin_cmd() fails. 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 653e0b9a94ca..5fd3deaf14d6 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -4886,11 +4886,11 @@ static int send_remove_xattr(struct send_ctx *sctx, struct fs_path *path, const char *name, int name_len) { - int ret = 0; + int ret; ret = begin_cmd(sctx, BTRFS_SEND_C_REMOVE_XATTR); if (ret < 0) - goto out; + return ret; TLV_PUT_PATH(sctx, BTRFS_SEND_A_PATH, path); TLV_PUT_STRING(sctx, BTRFS_SEND_A_XATTR_NAME, name, name_len); @@ -4898,7 +4898,6 @@ static int send_remove_xattr(struct send_ctx *sctx, ret = send_cmd(sctx); tlv_put_failure: -out: return ret; } -- 2.50.1 From 25e5dee510a75e01a55f113e6411ff2b1c99e73c Mon Sep 17 00:00:00 2001 From: Filipe Manana Date: Tue, 18 Feb 2025 15:44:13 +0000 Subject: [PATCH 16/16] btrfs: send: simplify return logic from record_new_ref_if_needed() 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 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c index 5fd3deaf14d6..96aa519e791a 100644 --- a/fs/btrfs/send.c +++ b/fs/btrfs/send.c @@ -4683,7 +4683,7 @@ out: static int record_new_ref_if_needed(u64 dir, struct fs_path *name, void *ctx) { - int ret = 0; + int ret; struct send_ctx *sctx = ctx; struct rb_node *node = NULL; struct recorded_ref data; @@ -4692,7 +4692,7 @@ static int record_new_ref_if_needed(u64 dir, struct fs_path *name, void *ctx) ret = get_inode_gen(sctx->send_root, dir, &dir_gen); if (ret < 0) - goto out; + return ret; data.dir = dir; data.dir_gen = dir_gen; @@ -4706,7 +4706,7 @@ static int record_new_ref_if_needed(u64 dir, struct fs_path *name, void *ctx) &sctx->new_refs, name, dir, dir_gen, sctx); } -out: + return ret; } -- 2.50.1