| Message ID | cover.1743549291.git.dsterba@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | More btrfs_path auto cleaning | expand |
On Wed, 2 Apr 2025 at 01:18, David Sterba <dsterba@suse.com> wrote: > > A few more conversions to automatic freeing of btrfs_path, same > separation as last time, first patch with the trivial ones and separated > when there are goto/return conversion. > > David Sterba (7): > btrfs: do more trivial BTRFS_PATH_AUTO_FREE conversions > btrfs: use BTRFS_PATH_AUTO_FREE in may_destroy_subvol() > btrfs: use BTRFS_PATH_AUTO_FREE in btrfs_set_inode_index_count() > btrfs: use BTRFS_PATH_AUTO_FREE in can_nocow_extent() > btrfs: use BTRFS_PATH_AUTO_FREE in btrfs_encoded_read_inline() > btrfs: use BTRFS_PATH_AUTO_FREE in btrfs_del_inode_extref() > btrfs: use BTRFS_PATH_AUTO_FREE in btrfs_insert_inode_extref() With what David mentioned for 1/7 the series looks good to me. Reviewed-by: Daniel Vacek <neelx@suse.com> --nX > > fs/btrfs/block-group.c | 3 +- > fs/btrfs/fiemap.c | 3 +- > fs/btrfs/file-item.c | 3 +- > fs/btrfs/file.c | 3 +- > fs/btrfs/free-space-cache.c | 3 +- > fs/btrfs/inode-item.c | 31 ++++----- > fs/btrfs/inode.c | 123 ++++++++++++++---------------------- > 7 files changed, 65 insertions(+), 104 deletions(-) > > -- > 2.48.1 > >
I'm new in lore, not having many experience in kernel development. I'm currently learning by read the code and patches. I'm just wondering why we cannot just make these `btrfs_path` become local variables so they can get freed automatically? Is it because the size of `btrfs_path` is too large to put them on stack? Sorry for bothering here. I'm glad to move to else where if it is not prorate to discuss here.
On Sat, Apr 05, 2025 at 12:13:58PM +0800, Sun YangKai wrote: > I'm new in lore, not having many experience in kernel development. I'm > currently learning by read the code and patches. I'm just wondering why we > cannot just make these `btrfs_path` become local variables so they can get > freed automatically? Is it because the size of `btrfs_path` is too large to > put them on stack? Yes, this is the reason. The size of the structure is 112 bytes, stack size is fixed to 16K and is considered a scarce resource. It's not that the filesystem would use the whole 16K of the stack but in connection with other internal subsystems like memory management, block layer, drivers and with other layers like device mapper or NFS, the overall stack consumption matters and each layer should not waste it. In some known cases like ioctl handlers that run from the process context it's known that there's more stack left, basically only the layers below, so there are on-stack structures that do not need to be allocated. > Sorry for bothering here. I'm glad to move to else where if it is not prorate > to discuss here. No problem, it's a valid question and keeping a structure on stack may simplify some things so it's something to look for, but as can be demonstrated on this example it's not always safe. The btrfs_path cannot be made smaller, the first member 'nodes' is already 64 bytes, the remaining members can be possibly squeezed when smaller int types would be used but this generates worse code and it's one of most frequently used structures.
Thank you for your kind reply. I've learned a lot here.