| Message ID | 20200211214042.4645-3-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Error condition failure fixes | expand |
On 2020/2/12 上午5:40, Josef Bacik wrote: > btrfs_assert_delayed_root_empty() will check if the delayed root is > completely empty, but this is a fs wide check. On cleanup we may have > allowed other transactions to begin, for whatever reason, and thus the > delayed root is not empty. So remove this check from > cleanup_one_transation(). This however can stay in > btrfs_cleanup_transaction(), because it checks only after all of the > transactions have been properly cleaned up, and thus is valid. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Just a nitpick, to allow other user to verify the fix, would you mind to provide a specific reproducer? Like the error injection (I guess it's still memory allocation failure), the call chain. Thanks, Qu > --- > fs/btrfs/disk-io.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c > index 5b6140482cef..601ed3335cf6 100644 > --- a/fs/btrfs/disk-io.c > +++ b/fs/btrfs/disk-io.c > @@ -4543,7 +4543,6 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans, > wake_up(&fs_info->transaction_wait); > > btrfs_destroy_delayed_inodes(fs_info); > - btrfs_assert_delayed_root_empty(fs_info); > > btrfs_destroy_marked_extents(fs_info, &cur_trans->dirty_pages, > EXTENT_DIRTY); >
On 11.02.20 г. 23:40 ч., Josef Bacik wrote: > btrfs_assert_delayed_root_empty() will check if the delayed root is > completely empty, but this is a fs wide check. On cleanup we may have > allowed other transactions to begin, for whatever reason, and thus the > delayed root is not empty. So remove this check from > cleanup_one_transation(). This however can stay in > btrfs_cleanup_transaction(), because it checks only after all of the > transactions have been properly cleaned up, and thus is valid. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 5b6140482cef..601ed3335cf6 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -4543,7 +4543,6 @@ void btrfs_cleanup_one_transaction(struct btrfs_transaction *cur_trans, wake_up(&fs_info->transaction_wait); btrfs_destroy_delayed_inodes(fs_info); - btrfs_assert_delayed_root_empty(fs_info); btrfs_destroy_marked_extents(fs_info, &cur_trans->dirty_pages, EXTENT_DIRTY);
btrfs_assert_delayed_root_empty() will check if the delayed root is completely empty, but this is a fs wide check. On cleanup we may have allowed other transactions to begin, for whatever reason, and thus the delayed root is not empty. So remove this check from cleanup_one_transation(). This however can stay in btrfs_cleanup_transaction(), because it checks only after all of the transactions have been properly cleaned up, and thus is valid. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/disk-io.c | 1 - 1 file changed, 1 deletion(-)