| Message ID | 648c181a4b3d2076ee838945232e3b87b5d575e7.1734582790.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] btrfs: handle free space tree rebuild in multiple transactions | expand |
On Thu, Dec 19, 2024 at 4:35 AM Qu Wenruo <wqu@suse.com> wrote: > > During free space tree rebuild, we're holding a transaction handler for handler -> handle > the whole rebuild process. > > This can lead to blocked task warning, e.g. btrfs-transaction kthread > (which is already created before btrfs_start_pre_rw_mount()) can be > waked up to join and commit the current transaction. > > But the free space tree rebuild process may need to go through thousands > block groups, this will block btrfs-transaction kthread for a long time. > > Fix the problem by calling btrfs_should_end_transaction() after each > block group, so that we won't hold the transaction handler too long. handler -> handle We could also start the transaction kthread only after building the free space tree, it would avoid some transaction commits, saving IO and rotation of backup roots in the super block. But with the free space tree being a default for quite a while now, and this being more likely only on very large filesystems, it's fine. Reviewed-by: Filipe Manana <fdmanana@suse.com> Thanks. > > And since the free-space-tree rebuild can be split into > multiple transactions, we need to consider the safety when the rebuild > process is interrupted. > > Thankfully since we only set the FREE_SPACE_TREE compat_ro flag without > FREE_SPACE_TREE_VALID flag, even if the rebuild is interrupted, on the > next RW mount, we will still go rebuild the free space tree, by deleting > any items we have and re-starting the rebuild from scratch. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > Changelog: > v2: > - Use btrfs_should_end_transaction() instead of batching > Which should be more reliable than batching. > --- > fs/btrfs/free-space-tree.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c > index 7ba50e133921..2400fa5a5be4 100644 > --- a/fs/btrfs/free-space-tree.c > +++ b/fs/btrfs/free-space-tree.c > @@ -1350,6 +1350,12 @@ int btrfs_rebuild_free_space_tree(struct btrfs_fs_info *fs_info) > btrfs_end_transaction(trans); > return ret; > } > + if (btrfs_should_end_transaction(trans)) { > + btrfs_end_transaction(trans); > + trans = btrfs_start_transaction(free_space_root, 1); > + if (IS_ERR(trans)) > + return PTR_ERR(trans); > + } > node = rb_next(node); > } > > -- > 2.47.1 > >
diff --git a/fs/btrfs/free-space-tree.c b/fs/btrfs/free-space-tree.c index 7ba50e133921..2400fa5a5be4 100644 --- a/fs/btrfs/free-space-tree.c +++ b/fs/btrfs/free-space-tree.c @@ -1350,6 +1350,12 @@ int btrfs_rebuild_free_space_tree(struct btrfs_fs_info *fs_info) btrfs_end_transaction(trans); return ret; } + if (btrfs_should_end_transaction(trans)) { + btrfs_end_transaction(trans); + trans = btrfs_start_transaction(free_space_root, 1); + if (IS_ERR(trans)) + return PTR_ERR(trans); + } node = rb_next(node); }
During free space tree rebuild, we're holding a transaction handler for the whole rebuild process. This can lead to blocked task warning, e.g. btrfs-transaction kthread (which is already created before btrfs_start_pre_rw_mount()) can be waked up to join and commit the current transaction. But the free space tree rebuild process may need to go through thousands block groups, this will block btrfs-transaction kthread for a long time. Fix the problem by calling btrfs_should_end_transaction() after each block group, so that we won't hold the transaction handler too long. And since the free-space-tree rebuild can be split into multiple transactions, we need to consider the safety when the rebuild process is interrupted. Thankfully since we only set the FREE_SPACE_TREE compat_ro flag without FREE_SPACE_TREE_VALID flag, even if the rebuild is interrupted, on the next RW mount, we will still go rebuild the free space tree, by deleting any items we have and re-starting the rebuild from scratch. Signed-off-by: Qu Wenruo <wqu@suse.com> --- Changelog: v2: - Use btrfs_should_end_transaction() instead of batching Which should be more reliable than batching. --- fs/btrfs/free-space-tree.c | 6 ++++++ 1 file changed, 6 insertions(+)