| Message ID | cover.1729784712.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | btrfs: convert delayed head refs to xarray and cleanups | expand |
On Thu, Oct 24, 2024 at 05:24:08PM +0100, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > This converts the rb-tree that tracks delayed ref heads into an xarray, > reducing memory used by delayed ref heads and making it more efficient > to add/remove/find delayed ref heads. The rest are mostly cleanups and > refactorings, removing some dead code, deduplicating code, move code > around, etc. More details in the changelogs. The cleanup patches all look perfectly reasonable to me, and the risks (new tracked variable, new allocation) with the xarray patches seem well considered. Thanks for making this improvement, I think xarray is a good fit for delayed refs. Reviewed-by: Boris Burkov <boris@bur.io> > > Filipe Manana (18): > btrfs: remove BUG_ON() at btrfs_destroy_delayed_refs() > btrfs: move btrfs_destroy_delayed_refs() to delayed-ref.c > btrfs: remove fs_info parameter from btrfs_destroy_delayed_refs() > btrfs: remove fs_info parameter from btrfs_cleanup_one_transaction() > btrfs: remove duplicated code to drop delayed ref during transaction abort > btrfs: use helper to find first ref head at btrfs_destroy_delayed_refs() > btrfs: remove num_entries atomic counter from delayed ref root > btrfs: change return type of btrfs_delayed_ref_lock() to boolean > btrfs: simplify obtaining a delayed ref head > btrfs: move delayed ref head unselection to delayed-ref.c > btrfs: pass fs_info to functions that search for delayed ref heads > btrfs: pass fs_info to btrfs_cleanup_ref_head_accounting > btrfs: assert delayed refs lock is held at find_ref_head() > btrfs: assert delayed refs lock is held at find_first_ref_head() > btrfs: assert delayed refs lock is held at add_delayed_ref_head() > btrfs: add comments regarding locking to struct btrfs_delayed_ref_root > btrfs: track delayed ref heads in an xarray > btrfs: remove no longer used delayed ref head search functionality > > fs/btrfs/backref.c | 3 +- > fs/btrfs/delayed-ref.c | 319 +++++++++++++++++++++++++---------------- > fs/btrfs/delayed-ref.h | 61 +++++--- > fs/btrfs/disk-io.c | 79 +--------- > fs/btrfs/disk-io.h | 3 +- > fs/btrfs/extent-tree.c | 69 ++------- > fs/btrfs/transaction.c | 8 +- > 7 files changed, 256 insertions(+), 286 deletions(-) > > -- > 2.43.0 >
On 24.10.24 18:24, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > This converts the rb-tree that tracks delayed ref heads into an xarray, > reducing memory used by delayed ref heads and making it more efficient > to add/remove/find delayed ref heads. The rest are mostly cleanups and > refactorings, removing some dead code, deduplicating code, move code > around, etc. More details in the changelogs. Stupid question (and by that I literally mean, it's a stupid question, I have no idea): looking at other places where we're heavily relying on rb-trees is ordered-extents. Would it make sense to move them over to xarrays as well?
On Fri, Oct 25, 2024 at 2:19 PM Johannes Thumshirn <Johannes.Thumshirn@wdc.com> wrote: > > On 24.10.24 18:24, fdmanana@kernel.org wrote: > > From: Filipe Manana <fdmanana@suse.com> > > > > This converts the rb-tree that tracks delayed ref heads into an xarray, > > reducing memory used by delayed ref heads and making it more efficient > > to add/remove/find delayed ref heads. The rest are mostly cleanups and > > refactorings, removing some dead code, deduplicating code, move code > > around, etc. More details in the changelogs. > > Stupid question (and by that I literally mean, it's a stupid question, I > have no idea): looking at other places where we're heavily relying on > rb-trees is ordered-extents. Would it make sense to move them over to > xarrays as well? For ordered extents I wouldn't consider it, because I don't think it's common to have thousands of them per inode. Same for delayed refs inside a delayed ref head for example. For delayed ref heads, for every cow operation we get one to delete the old extent and another one for the new extent, so these can easily be thousands even for small filesystems with moderate and even low workloads. It may still be worth just to reduce structure sizes and memory usage, though it would have to be analyzed on a case by case basis.
On 25.10.24 15:36, Filipe Manana wrote: > On Fri, Oct 25, 2024 at 2:19 PM Johannes Thumshirn > <Johannes.Thumshirn@wdc.com> wrote: >> >> On 24.10.24 18:24, fdmanana@kernel.org wrote: >>> From: Filipe Manana <fdmanana@suse.com> >>> >>> This converts the rb-tree that tracks delayed ref heads into an xarray, >>> reducing memory used by delayed ref heads and making it more efficient >>> to add/remove/find delayed ref heads. The rest are mostly cleanups and >>> refactorings, removing some dead code, deduplicating code, move code >>> around, etc. More details in the changelogs. >> >> Stupid question (and by that I literally mean, it's a stupid question, I >> have no idea): looking at other places where we're heavily relying on >> rb-trees is ordered-extents. Would it make sense to move them over to >> xarrays as well? > > For ordered extents I wouldn't consider it, because I don't think it's > common to have thousands of them per inode. > Same for delayed refs inside a delayed ref head for example. > For delayed ref heads, for every cow operation we get one to delete > the old extent and another one for the new extent, so these can easily > be thousands even for small filesystems with moderate and even low > workloads. > > It may still be worth just to reduce structure sizes and memory usage, > though it would have to be analyzed on a case by case basis. > Thanks for the info :)
On Thu, Oct 24, 2024 at 05:24:08PM +0100, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > This converts the rb-tree that tracks delayed ref heads into an xarray, > reducing memory used by delayed ref heads and making it more efficient > to add/remove/find delayed ref heads. The rest are mostly cleanups and > refactorings, removing some dead code, deduplicating code, move code > around, etc. More details in the changelogs. > > Filipe Manana (18): > btrfs: remove BUG_ON() at btrfs_destroy_delayed_refs() > btrfs: move btrfs_destroy_delayed_refs() to delayed-ref.c > btrfs: remove fs_info parameter from btrfs_destroy_delayed_refs() > btrfs: remove fs_info parameter from btrfs_cleanup_one_transaction() > btrfs: remove duplicated code to drop delayed ref during transaction abort > btrfs: use helper to find first ref head at btrfs_destroy_delayed_refs() > btrfs: remove num_entries atomic counter from delayed ref root > btrfs: change return type of btrfs_delayed_ref_lock() to boolean > btrfs: simplify obtaining a delayed ref head > btrfs: move delayed ref head unselection to delayed-ref.c > btrfs: pass fs_info to functions that search for delayed ref heads > btrfs: pass fs_info to btrfs_cleanup_ref_head_accounting > btrfs: assert delayed refs lock is held at find_ref_head() > btrfs: assert delayed refs lock is held at find_first_ref_head() > btrfs: assert delayed refs lock is held at add_delayed_ref_head() > btrfs: add comments regarding locking to struct btrfs_delayed_ref_root > btrfs: track delayed ref heads in an xarray > btrfs: remove no longer used delayed ref head search functionality Reviewed-by: David Sterba <dsterba@suse.com> Thank you, nice cleanups and I like another conversion to xarray.
From: Filipe Manana <fdmanana@suse.com> This converts the rb-tree that tracks delayed ref heads into an xarray, reducing memory used by delayed ref heads and making it more efficient to add/remove/find delayed ref heads. The rest are mostly cleanups and refactorings, removing some dead code, deduplicating code, move code around, etc. More details in the changelogs. Filipe Manana (18): btrfs: remove BUG_ON() at btrfs_destroy_delayed_refs() btrfs: move btrfs_destroy_delayed_refs() to delayed-ref.c btrfs: remove fs_info parameter from btrfs_destroy_delayed_refs() btrfs: remove fs_info parameter from btrfs_cleanup_one_transaction() btrfs: remove duplicated code to drop delayed ref during transaction abort btrfs: use helper to find first ref head at btrfs_destroy_delayed_refs() btrfs: remove num_entries atomic counter from delayed ref root btrfs: change return type of btrfs_delayed_ref_lock() to boolean btrfs: simplify obtaining a delayed ref head btrfs: move delayed ref head unselection to delayed-ref.c btrfs: pass fs_info to functions that search for delayed ref heads btrfs: pass fs_info to btrfs_cleanup_ref_head_accounting btrfs: assert delayed refs lock is held at find_ref_head() btrfs: assert delayed refs lock is held at find_first_ref_head() btrfs: assert delayed refs lock is held at add_delayed_ref_head() btrfs: add comments regarding locking to struct btrfs_delayed_ref_root btrfs: track delayed ref heads in an xarray btrfs: remove no longer used delayed ref head search functionality fs/btrfs/backref.c | 3 +- fs/btrfs/delayed-ref.c | 319 +++++++++++++++++++++++++---------------- fs/btrfs/delayed-ref.h | 61 +++++--- fs/btrfs/disk-io.c | 79 +--------- fs/btrfs/disk-io.h | 3 +- fs/btrfs/extent-tree.c | 69 ++------- fs/btrfs/transaction.c | 8 +- 7 files changed, 256 insertions(+), 286 deletions(-)