| Message ID | SEZPR06MB526928EBDA13B6463D7EE00BE8B32@SEZPR06MB5269.apcprd06.prod.outlook.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | 回复: [PATCH] btrfs: use BTRFS_PATH_AUTO_FREE in btrfs_truncate_inode_items() | expand |
On Mon, 14 Apr 2025 at 15:34, 李扬韬 <frank.li@vivo.com> wrote: > > Hi Daniel, > > > And what about the other functions in that file? We could even get rid of two allocations passing the path from ..._inode_ref() to ..._inode_extref(). > > I made the following changes, is this what you meant? > I will do the rest if that's ok. Yeah, quite almost. > Thx, > Yangtao > > diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c > index 3530de0618c8..e082d7e27c29 100644 > --- a/fs/btrfs/inode-item.c > +++ b/fs/btrfs/inode-item.c > @@ -105,11 +105,11 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans, > > static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > struct btrfs_root *root, > + struct btrfs_path *path, > const struct fscrypt_str *name, > u64 inode_objectid, u64 ref_objectid, > u64 *index) > { > - struct btrfs_path *path; > struct btrfs_key key; > struct btrfs_inode_extref *extref; > struct extent_buffer *leaf; I think you missed this: @@ -123,10 +123,6 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, key.type = BTRFS_INODE_EXTREF_KEY; key.offset = btrfs_extref_hash(ref_objectid, name->name, name->len); - path = btrfs_alloc_path(); - if (!path) - return -ENOMEM; - ret = btrfs_search_slot(trans, root, &key, path, -1, 1); if (ret > 0) ret = -ENOENT; Which was ironically the whole point ;-) > @@ -131,7 +131,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > if (ret > 0) > ret = -ENOENT; You can also directly return -ENOENT; here. > if (ret < 0) > - goto out; > + return ret; > > /* > * Sanity check - did we find the right item for this name? > @@ -142,8 +142,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > ref_objectid, name); > if (!extref) { > btrfs_abort_transaction(trans, -ENOENT); > - ret = -ENOENT; > - goto out; > + return -ENOENT; > } > > leaf = path->nodes[0]; > @@ -156,8 +155,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > * Common case only one ref in the item, remove the > * whole item. > */ > - ret = btrfs_del_item(trans, root, path); > - goto out; > + return btrfs_del_item(trans, root, path); > } > > ptr = (unsigned long)extref; > @@ -168,9 +166,6 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > > btrfs_truncate_item(trans, path, item_size - del_len, 1); > > -out: > - btrfs_free_path(path); > - > return ret; > } > > @@ -178,7 +173,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans, > struct btrfs_root *root, const struct fscrypt_str *name, > u64 inode_objectid, u64 ref_objectid, u64 *index) > { > - struct btrfs_path *path; > + BTRFS_PATH_AUTO_FREE(path); > struct btrfs_key key; > struct btrfs_inode_ref *ref; > struct extent_buffer *leaf; > @@ -230,7 +225,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans, > item_size - (ptr + sub_item_len - item_start)); > btrfs_truncate_item(trans, path, item_size - sub_item_len, 1); > out: > - btrfs_free_path(path); > + btrfs_release_path(path); > > if (search_ext_refs) { > /* > @@ -238,7 +233,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans, > * name in our ref array. Find and remove the extended > * inode ref then. > */ > - return btrfs_del_inode_extref(trans, root, name, > + return btrfs_del_inode_extref(trans, root, path, name, > inode_objectid, ref_objectid, index); > }
On Mon, 14 Apr 2025 at 15:34, 李扬韬 <frank.li@vivo.com> wrote: > > Hi Daniel, > > > And what about the other functions in that file? We could even get rid of two allocations passing the path from ..._inode_ref() to ..._inode_extref(). > > I made the following changes, is this what you meant? > I will do the rest if that's ok. > > Thx, > Yangtao > > diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c > index 3530de0618c8..e082d7e27c29 100644 > --- a/fs/btrfs/inode-item.c > +++ b/fs/btrfs/inode-item.c > @@ -105,11 +105,11 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans, > > static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > struct btrfs_root *root, > + struct btrfs_path *path, > const struct fscrypt_str *name, > u64 inode_objectid, u64 ref_objectid, > u64 *index) > { > - struct btrfs_path *path; > struct btrfs_key key; > struct btrfs_inode_extref *extref; > struct extent_buffer *leaf; > @@ -131,7 +131,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > if (ret > 0) > ret = -ENOENT; > if (ret < 0) > - goto out; > + return ret; > > /* > * Sanity check - did we find the right item for this name? > @@ -142,8 +142,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > ref_objectid, name); > if (!extref) { > btrfs_abort_transaction(trans, -ENOENT); > - ret = -ENOENT; > - goto out; > + return -ENOENT; > } > > leaf = path->nodes[0]; > @@ -156,8 +155,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > * Common case only one ref in the item, remove the > * whole item. > */ > - ret = btrfs_del_item(trans, root, path); > - goto out; > + return btrfs_del_item(trans, root, path); > } > > ptr = (unsigned long)extref; > @@ -168,9 +166,6 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, > > btrfs_truncate_item(trans, path, item_size - del_len, 1); > > -out: > - btrfs_free_path(path); > - > return ret; And if I look correctly you can also directly return 0 here. > } > > @@ -178,7 +173,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans, > struct btrfs_root *root, const struct fscrypt_str *name, > u64 inode_objectid, u64 ref_objectid, u64 *index) > { > - struct btrfs_path *path; > + BTRFS_PATH_AUTO_FREE(path); > struct btrfs_key key; > struct btrfs_inode_ref *ref; > struct extent_buffer *leaf; > @@ -230,7 +225,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans, > item_size - (ptr + sub_item_len - item_start)); > btrfs_truncate_item(trans, path, item_size - sub_item_len, 1); > out: > - btrfs_free_path(path); > + btrfs_release_path(path); > > if (search_ext_refs) { > /* > @@ -238,7 +233,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans, > * name in our ref array. Find and remove the extended > * inode ref then. > */ > - return btrfs_del_inode_extref(trans, root, name, > + return btrfs_del_inode_extref(trans, root, path, name, > inode_objectid, ref_objectid, index); > }
diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c index 3530de0618c8..e082d7e27c29 100644 --- a/fs/btrfs/inode-item.c +++ b/fs/btrfs/inode-item.c @@ -105,11 +105,11 @@ btrfs_lookup_inode_extref(struct btrfs_trans_handle *trans, static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, struct btrfs_root *root, + struct btrfs_path *path, const struct fscrypt_str *name, u64 inode_objectid, u64 ref_objectid, u64 *index) { - struct btrfs_path *path; struct btrfs_key key; struct btrfs_inode_extref *extref; struct extent_buffer *leaf; @@ -131,7 +131,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, if (ret > 0) ret = -ENOENT; if (ret < 0) - goto out; + return ret; /* * Sanity check - did we find the right item for this name? @@ -142,8 +142,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, ref_objectid, name); if (!extref) { btrfs_abort_transaction(trans, -ENOENT); - ret = -ENOENT; - goto out; + return -ENOENT; } leaf = path->nodes[0]; @@ -156,8 +155,7 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, * Common case only one ref in the item, remove the * whole item. */ - ret = btrfs_del_item(trans, root, path); - goto out; + return btrfs_del_item(trans, root, path); } ptr = (unsigned long)extref; @@ -168,9 +166,6 @@ static int btrfs_del_inode_extref(struct btrfs_trans_handle *trans, btrfs_truncate_item(trans, path, item_size - del_len, 1); -out: - btrfs_free_path(path); - return ret; } @@ -178,7 +173,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans, struct btrfs_root *root, const struct fscrypt_str *name, u64 inode_objectid, u64 ref_objectid, u64 *index) { - struct btrfs_path *path; + BTRFS_PATH_AUTO_FREE(path); struct btrfs_key key; struct btrfs_inode_ref *ref; struct extent_buffer *leaf; @@ -230,7 +225,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans, item_size - (ptr + sub_item_len - item_start)); btrfs_truncate_item(trans, path, item_size - sub_item_len, 1); out: - btrfs_free_path(path); + btrfs_release_path(path); if (search_ext_refs) { /* @@ -238,7 +233,7 @@ int btrfs_del_inode_ref(struct btrfs_trans_handle *trans, * name in our ref array. Find and remove the extended * inode ref then. */ - return btrfs_del_inode_extref(trans, root, name, + return btrfs_del_inode_extref(trans, root, path, name, inode_objectid, ref_objectid, index); }
Hi Daniel, > And what about the other functions in that file? We could even get rid of two allocations passing the path from ..._inode_ref() to ..._inode_extref(). I made the following changes, is this what you meant? I will do the rest if that's ok. Thx, Yangtao