On 2020/1/8 下午8:07, Johannes Thumshirn wrote:
> When running xfstests on the current btrfs I get the following splat from
> kmemleak:
> unreferenced object 0xffff88821b2404e0 (size 32):
> comm "kworker/u4:7", pid 26663, jiffies 4295283698 (age 8.776s)
> hex dump (first 32 bytes):
> 01 00 00 00 00 00 00 00 10 ff fd 26 82 88 ff ff ...........&....
> 10 ff fd 26 82 88 ff ff 20 ff fd 26 82 88 ff ff ...&.... ..&....
> backtrace:
> [<00000000f94fd43f>] ulist_alloc+0x25/0x60 [btrfs]
> [<00000000fd023d99>] btrfs_find_all_roots_safe+0x41/0x100 [btrfs]
> [<000000008f17bd32>] btrfs_find_all_roots+0x52/0x70 [btrfs]
> [<00000000b7660afb>] btrfs_qgroup_rescan_worker+0x343/0x680 [btrfs]
> [<0000000058e66778>] btrfs_work_helper+0xac/0x1e0 [btrfs]
> [<00000000f0188930>] process_one_work+0x1cf/0x350
> [<00000000af5f2f8e>] worker_thread+0x28/0x3c0
> [<00000000b55a1add>] kthread+0x109/0x120
> [<00000000f88cbd17>] ret_from_fork+0x35/0x40
>
> This corresponds to:
> (gdb) l *(btrfs_find_all_roots_safe+0x41)
> 0x8d7e1 is in btrfs_find_all_roots_safe (fs/btrfs/backref.c:1413).
> 1408
> 1409 tmp = ulist_alloc(GFP_NOFS);
> 1410 if (!tmp)
> 1411 return -ENOMEM;
> 1412 *roots = ulist_alloc(GFP_NOFS);
> 1413 if (!*roots) {
> 1414 ulist_free(tmp);
> 1415 return -ENOMEM;
> 1416 }
> 1417
>
> Following the lifetime of the allocated 'roots' ulist, it get's freed
> again in btrfs_qgroup_account_extent().
>
> But this does not happen if the function is called with the
> 'BTRFS_FS_QUOTA_ENABLED' flag cleared, then btrfs_qgroup_account_extent()
> does a short leave and directly returns.
>
> Instead of directly returning we should jump to the 'out_free' in order to
> free all resources as expected.
>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@xxxxxxx>
The patch itself is OK.
Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
This means the qgroup get disabled when rescan is still running.
So I'm a little curious, could we just cancel the running rescan and
wait for it before disabling qgroup?
Thanks,
Qu
> ---
> fs/btrfs/qgroup.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index b046b04d7cce..7ebcdd201eed 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -2416,7 +2416,7 @@ int btrfs_qgroup_account_extent(struct btrfs_trans_handle *trans, u64 bytenr,
> int ret = 0;
>
> if (!test_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags))
> - return 0;
> + goto out_free;
>
> if (new_roots) {
> if (!maybe_fs_roots(new_roots))
>
Attachment:
signature.asc
Description: OpenPGP digital signature
