Re: 回复: [PATCH 1/3] btrfs: get rid of path allocation in btrfs_del_inode_extref()

[David Sterba <dsterba@suse.cz>]

On Wed, Apr 16, 2025 at 01:24:30PM +0000, 李扬韬 wrote:
> 
> 
> > Also a good point, the path should be in a pristine state, as if it were just allocated. Releasing paths in other functions may want to keep the bits but in this case we're crossing a function boundary and the same assumptions may not be the same.
> 
> > Release resets the ->nodes, so what's left is from ->slots until the the end of the structure. And a helper for that would be desirable rather than opencoding that.
> 
> IIUC, use btrfs_reset_path instead of btrfs_release_path?
> 
> noinline void btrfs_reset_path(struct btrfs_path *p)
> {
>         int i;
> 
>         for (i = 0; i < BTRFS_MAX_LEVEL; i++) {
>                 if (!p->nodes[i])
>                         continue;
>                 if (p->locks[i])
>                         btrfs_tree_unlock_rw(p->nodes[i], p->locks[i]);
>                 free_extent_buffer(p->nodes[i]);
>         }
>         memset(p, 0, sizeof(struct btrfs_path));
> }
> 
> BTW, I have seen released paths being passed across functions in some other paths.
> 
> Should these also be changed to reset paths, or should these flags be cleared in the release path?

No, a path may be passed among several functions but the path bits may
be set up in a particular way and must be preserved. E.g. if the first
caller sets up path to search commit root the next call expect this bit
to be set as well so clearing it would be a bug.

Example is resolve_indirect_ref(), resolve_indirect_refs() and
find_parent_nodes() that set skip_locks and search_commit_root.