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

David Sterba
2025-04-15T15:56:40Z

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.