| 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.
在 2024/10/26 00:05, Filipe Manana 写道: > 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. > Another question related to this is, if we switch ordered extent or extent map to XArray, how do we find such structure that covers a bytenr? For delayed ref and extent buffer (which uses radix tree) we are working with the exact bytenr, but for OE/EM we do a lot of search using a bytenr. Or do I miss some XArray feature that can do that? Thanks, Qu
On Fri, Oct 25, 2024 at 10:18 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote: > > > > 在 2024/10/26 00:05, Filipe Manana 写道: > > 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. > > > > Another question related to this is, if we switch ordered extent or > extent map to XArray, how do we find such structure that covers a bytenr? You don't, not without doing something like xa_for_each() and iterate the xarray until either: 1) we find a matching entry (within the ragne of an element in the xarray); 2) we find the first entry that starts beyond the offset/range we are looking for and stop, meaning there's no entry covering the range we want; 3) we reach the end of the array. The rbtree based search is more practical for that. > > For delayed ref and extent buffer (which uses radix tree) we are working > with the exact bytenr, but for OE/EM we do a lot of search using a bytenr. > > Or do I miss some XArray feature that can do that? There isn't any. There are ranges for xarray but they basically mean having a range of indexes point to the same value, which doesn't help in these cases. For extent maps and states for example, beside the range search, there's also the detail of merging adjacent entries, which is not doable in a practical and efficient way with xarrays, especially to find the previous entry. > > Thanks, > Qu
在 2024/10/25 02:54, fdmanana@kernel.org 写道: > 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. Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > > 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(-) >
On Tue, Oct 29, 2024 at 07:25:35AM +1030, Qu Wenruo wrote: > > > 在 2024/10/25 02:54, fdmanana@kernel.org 写道: > > 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. > > Reviewed-by: Qu Wenruo <wqu@suse.com> For the record, rev-by added to the whole series in for-next.
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(-)