Re: [PATCH 1/3] btrfs: get rid of path allocation in btrfs_del_inode_extref()
From: David Sterba
Date: Tue Apr 15 2025 - 11:56:52 EST
On Tue, Apr 15, 2025 at 10:45:06PM +0800, Sun YangKai wrote:
> It seems a nice try to reduce path allocation and improve performance.
>
> But it also seems make the code less maintainable. I would prefer to have a
> comment saying something like the @path argument is just for reuse the
> btrfs_path allocation and only a released or empty btrfs_path should be used
> here.
Yes, this should be there, though we use the pattern of passing existing
path to functions so this within what'd consider OK.
> Also, although the path passed is released, it seems the bit flags are still
> passed, which makes the behavior of the functions a little different. But it
> seems fine since those bit flags are never set in this code path.
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.