| Message ID | 1697680236677896913e26948a76a2dd01dad235.1711488980.git.boris@bur.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: various qg meta rsv leak fixes | expand |
在 2024/3/27 08:09, Boris Burkov 写道: > Some of the operations after the free might convert more pertrans > metadata. Do the freeing as late as possible to eliminate a source of > leaked pertrans metadata. > > Helps with the pass rate of generic/269 and generic/475. > > Signed-off-by: Boris Burkov <boris@bur.io> Well, you can also move other fs level cleanup out of the btrfs_cleanup_one_transaction() call. (e.g. destory_delayed_inodes()). For qgroup part, it looks fine to me as a precautious behavior. Thanks, Qu > --- > fs/btrfs/disk-io.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 3df5477d48a8..4d7893cc0d4e 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -4850,8 +4850,6 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans, > EXTENT_DIRTY); > btrfs_destroy_pinned_extent(fs_info, &cur_trans->pinned_extents); > > - btrfs_free_all_qgroup_pertrans(fs_info); > - > cur_trans->state =TRANS_STATE_COMPLETED; > wake_up(&cur_trans->commit_wait); > } > @@ -4904,6 +4902,7 @@ static int btrfs_cleanup_transaction(struct btrfs_fs_info *fs_info) > btrfs_assert_delayed_root_empty(fs_info); > btrfs_destroy_all_delalloc_inodes(fs_info); > btrfs_drop_all_logs(fs_info); > + btrfs_free_all_qgroup_pertrans(fs_info); > mutex_unlock(&fs_info->transaction_kthread_mutex); > > return 0;
On Wed, Mar 27, 2024 at 08:46:39AM +1030, Qu Wenruo wrote: > > > 在 2024/3/27 08:09, Boris Burkov 写道: > > Some of the operations after the free might convert more pertrans > > metadata. Do the freeing as late as possible to eliminate a source of > > leaked pertrans metadata. > > > > Helps with the pass rate of generic/269 and generic/475. > > > > Signed-off-by: Boris Burkov <boris@bur.io> > > Well, you can also move other fs level cleanup out of the > btrfs_cleanup_one_transaction() call. > (e.g. destory_delayed_inodes()). > > For qgroup part, it looks fine to me as a precautious behavior. Since the call isn't per transaction, do you think it just makes more sense to call it once per cleanup not once per trans per cleanup? Or would you rather I refactored it some other way? > > Thanks, > Qu > > > --- > > fs/btrfs/disk-io.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > > index 3df5477d48a8..4d7893cc0d4e 100644 > > --- a/fs/btrfs/disk-io.c > > +++ b/fs/btrfs/disk-io.c > > @@ -4850,8 +4850,6 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans, > > EXTENT_DIRTY); > > btrfs_destroy_pinned_extent(fs_info, &cur_trans->pinned_extents); > > - btrfs_free_all_qgroup_pertrans(fs_info); > > - > > cur_trans->state =TRANS_STATE_COMPLETED; > > wake_up(&cur_trans->commit_wait); > > } > > @@ -4904,6 +4902,7 @@ static int btrfs_cleanup_transaction(struct btrfs_fs_info *fs_info) > > btrfs_assert_delayed_root_empty(fs_info); > > btrfs_destroy_all_delalloc_inodes(fs_info); > > btrfs_drop_all_logs(fs_info); > > + btrfs_free_all_qgroup_pertrans(fs_info); > > mutex_unlock(&fs_info->transaction_kthread_mutex); > > return 0;
在 2024/3/28 03:52, Boris Burkov 写道: > On Wed, Mar 27, 2024 at 08:46:39AM +1030, Qu Wenruo wrote: >> >> >> 在 2024/3/27 08:09, Boris Burkov 写道: >>> Some of the operations after the free might convert more pertrans >>> metadata. Do the freeing as late as possible to eliminate a source of >>> leaked pertrans metadata. >>> >>> Helps with the pass rate of generic/269 and generic/475. >>> >>> Signed-off-by: Boris Burkov <boris@bur.io> >> >> Well, you can also move other fs level cleanup out of the >> btrfs_cleanup_one_transaction() call. >> (e.g. destory_delayed_inodes()). >> >> For qgroup part, it looks fine to me as a precautious behavior. > > Since the call isn't per transaction, do you think it just makes more > sense to call it once per cleanup not once per trans per cleanup? Yes, just like what you did for btrfs_free_all_qgroup_pertrans(). > > Or would you rather I refactored it some other way? Just an idea for future cleanups (moving all global cleanups out of btrfs_cleanup_one_transaction()). Thanks, Qu > >> >> Thanks, >> Qu >> >>> --- >>> fs/btrfs/disk-io.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c >>> index 3df5477d48a8..4d7893cc0d4e 100644 >>> --- a/fs/btrfs/disk-io.c >>> +++ b/fs/btrfs/disk-io.c >>> @@ -4850,8 +4850,6 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans, >>> EXTENT_DIRTY); >>> btrfs_destroy_pinned_extent(fs_info, &cur_trans->pinned_extents); >>> - btrfs_free_all_qgroup_pertrans(fs_info); >>> - >>> cur_trans->state =TRANS_STATE_COMPLETED; >>> wake_up(&cur_trans->commit_wait); >>> } >>> @@ -4904,6 +4902,7 @@ static int btrfs_cleanup_transaction(struct btrfs_fs_info *fs_info) >>> btrfs_assert_delayed_root_empty(fs_info); >>> btrfs_destroy_all_delalloc_inodes(fs_info); >>> btrfs_drop_all_logs(fs_info); >>> + btrfs_free_all_qgroup_pertrans(fs_info); >>> mutex_unlock(&fs_info->transaction_kthread_mutex); >>> return 0;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 3df5477d48a8..4d7893cc0d4e 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -4850,8 +4850,6 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans, EXTENT_DIRTY); btrfs_destroy_pinned_extent(fs_info, &cur_trans->pinned_extents); - btrfs_free_all_qgroup_pertrans(fs_info); - cur_trans->state =TRANS_STATE_COMPLETED; wake_up(&cur_trans->commit_wait); } @@ -4904,6 +4902,7 @@ static int btrfs_cleanup_transaction(struct btrfs_fs_info *fs_info) btrfs_assert_delayed_root_empty(fs_info); btrfs_destroy_all_delalloc_inodes(fs_info); btrfs_drop_all_logs(fs_info); + btrfs_free_all_qgroup_pertrans(fs_info); mutex_unlock(&fs_info->transaction_kthread_mutex); return 0;
Some of the operations after the free might convert more pertrans metadata. Do the freeing as late as possible to eliminate a source of leaked pertrans metadata. Helps with the pass rate of generic/269 and generic/475. Signed-off-by: Boris Burkov <boris@bur.io> --- fs/btrfs/disk-io.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)