* [PATCH] btrfs: free exchange changeset on failures
@ 2021-12-03 10:55 Johannes Thumshirn
2021-12-03 11:25 ` Filipe Manana
2021-12-06 17:48 ` David Sterba
0 siblings, 2 replies; 3+ messages in thread
From: Johannes Thumshirn @ 2021-12-03 10:55 UTC (permalink / raw)
To: David Sterba; +Cc: Johannes Thumshirn, linux-btrfs, Filipe Manana
Fstests runs on my VMs have show several kmemleak reports like the following.
unreferenced object 0xffff88811ae59080 (size 64):
comm "xfs_io", pid 12124, jiffies 4294987392 (age 6.368s)
hex dump (first 32 bytes):
00 c0 1c 00 00 00 00 00 ff cf 1c 00 00 00 00 00 ................
90 97 e5 1a 81 88 ff ff 90 97 e5 1a 81 88 ff ff ................
backtrace:
[<00000000ac0176d2>] ulist_add_merge+0x60/0x150 [btrfs]
[<0000000076e9f312>] set_state_bits+0x86/0xc0 [btrfs]
[<0000000014fe73d6>] set_extent_bit+0x270/0x690 [btrfs]
[<000000004f675208>] set_record_extent_bits+0x19/0x20 [btrfs]
[<00000000b96137b1>] qgroup_reserve_data+0x274/0x310 [btrfs]
[<0000000057e9dcbb>] btrfs_check_data_free_space+0x5c/0xa0 [btrfs]
[<0000000019c4511d>] btrfs_delalloc_reserve_space+0x1b/0xa0 [btrfs]
[<000000006d37e007>] btrfs_dio_iomap_begin+0x415/0x970 [btrfs]
[<00000000fb8a74b8>] iomap_iter+0x161/0x1e0
[<0000000071dff6ff>] __iomap_dio_rw+0x1df/0x700
[<000000002567ba53>] iomap_dio_rw+0x5/0x20
[<0000000072e555f8>] btrfs_file_write_iter+0x290/0x530 [btrfs]
[<000000005eb3d845>] new_sync_write+0x106/0x180
[<000000003fb505bf>] vfs_write+0x24d/0x2f0
[<000000009bb57d37>] __x64_sys_pwrite64+0x69/0xa0
[<000000003eba3fdf>] do_syscall_64+0x43/0x90
In case brtfs_qgroup_reserve_data() or btrfs_delalloc_reserve_metadata()
fail the allocated extent_changeset will not be freed.
So in btrfs_check_data_free_space() and btrfs_delalloc_reserve_space()
free the allocated extent_changeset to get rid of the allocated memory.
Cc: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
fs/btrfs/delalloc-space.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c
index bca438c7c972..fb46a28f5065 100644
--- a/fs/btrfs/delalloc-space.c
+++ b/fs/btrfs/delalloc-space.c
@@ -143,10 +143,13 @@ int btrfs_check_data_free_space(struct btrfs_inode *inode,
/* Use new btrfs_qgroup_reserve_data to reserve precious data space. */
ret = btrfs_qgroup_reserve_data(inode, reserved, start, len);
- if (ret < 0)
+ if (ret < 0) {
btrfs_free_reserved_data_space_noquota(fs_info, len);
- else
+ extent_changeset_free(*reserved);
+ *reserved = NULL;
+ } else {
ret = 0;
+ }
return ret;
}
@@ -452,8 +455,11 @@ int btrfs_delalloc_reserve_space(struct btrfs_inode *inode,
if (ret < 0)
return ret;
ret = btrfs_delalloc_reserve_metadata(inode, len);
- if (ret < 0)
+ if (ret < 0) {
btrfs_free_reserved_data_space(inode, *reserved, start, len);
+ extent_changeset_free(*reserved);
+ *reserved = NULL;
+ }
return ret;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: free exchange changeset on failures
2021-12-03 10:55 [PATCH] btrfs: free exchange changeset on failures Johannes Thumshirn
@ 2021-12-03 11:25 ` Filipe Manana
2021-12-06 17:48 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: Filipe Manana @ 2021-12-03 11:25 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: David Sterba, linux-btrfs, Filipe Manana
On Fri, Dec 03, 2021 at 02:55:33AM -0800, Johannes Thumshirn wrote:
> Fstests runs on my VMs have show several kmemleak reports like the following.
>
> unreferenced object 0xffff88811ae59080 (size 64):
> comm "xfs_io", pid 12124, jiffies 4294987392 (age 6.368s)
> hex dump (first 32 bytes):
> 00 c0 1c 00 00 00 00 00 ff cf 1c 00 00 00 00 00 ................
> 90 97 e5 1a 81 88 ff ff 90 97 e5 1a 81 88 ff ff ................
> backtrace:
> [<00000000ac0176d2>] ulist_add_merge+0x60/0x150 [btrfs]
> [<0000000076e9f312>] set_state_bits+0x86/0xc0 [btrfs]
> [<0000000014fe73d6>] set_extent_bit+0x270/0x690 [btrfs]
> [<000000004f675208>] set_record_extent_bits+0x19/0x20 [btrfs]
> [<00000000b96137b1>] qgroup_reserve_data+0x274/0x310 [btrfs]
> [<0000000057e9dcbb>] btrfs_check_data_free_space+0x5c/0xa0 [btrfs]
> [<0000000019c4511d>] btrfs_delalloc_reserve_space+0x1b/0xa0 [btrfs]
> [<000000006d37e007>] btrfs_dio_iomap_begin+0x415/0x970 [btrfs]
> [<00000000fb8a74b8>] iomap_iter+0x161/0x1e0
> [<0000000071dff6ff>] __iomap_dio_rw+0x1df/0x700
> [<000000002567ba53>] iomap_dio_rw+0x5/0x20
> [<0000000072e555f8>] btrfs_file_write_iter+0x290/0x530 [btrfs]
> [<000000005eb3d845>] new_sync_write+0x106/0x180
> [<000000003fb505bf>] vfs_write+0x24d/0x2f0
> [<000000009bb57d37>] __x64_sys_pwrite64+0x69/0xa0
> [<000000003eba3fdf>] do_syscall_64+0x43/0x90
>
> In case brtfs_qgroup_reserve_data() or btrfs_delalloc_reserve_metadata()
> fail the allocated extent_changeset will not be freed.
>
> So in btrfs_check_data_free_space() and btrfs_delalloc_reserve_space()
> free the allocated extent_changeset to get rid of the allocated memory.
>
> Cc: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Looks good, and it ran successfully on my fstests run with kmemleak as well.
Just a note worth adding, is that the issue currently only happens in the
direct IO write path, but only after my change "btrfs: fix ENOSPC failure
when attempting direct IO write into NOCOW range", and also at
defrag_one_locked_target() (haven't checked since when). Every other place
is always calling extent_changeset_free() even if its call to
btrfs_delalloc_reserve_space() or btrfs_check_data_free_space() has failed.
With that, which probably David can add, it looks good:
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Thanks.
> ---
> fs/btrfs/delalloc-space.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c
> index bca438c7c972..fb46a28f5065 100644
> --- a/fs/btrfs/delalloc-space.c
> +++ b/fs/btrfs/delalloc-space.c
> @@ -143,10 +143,13 @@ int btrfs_check_data_free_space(struct btrfs_inode *inode,
>
> /* Use new btrfs_qgroup_reserve_data to reserve precious data space. */
> ret = btrfs_qgroup_reserve_data(inode, reserved, start, len);
> - if (ret < 0)
> + if (ret < 0) {
> btrfs_free_reserved_data_space_noquota(fs_info, len);
> - else
> + extent_changeset_free(*reserved);
> + *reserved = NULL;
> + } else {
> ret = 0;
> + }
> return ret;
> }
>
> @@ -452,8 +455,11 @@ int btrfs_delalloc_reserve_space(struct btrfs_inode *inode,
> if (ret < 0)
> return ret;
> ret = btrfs_delalloc_reserve_metadata(inode, len);
> - if (ret < 0)
> + if (ret < 0) {
> btrfs_free_reserved_data_space(inode, *reserved, start, len);
> + extent_changeset_free(*reserved);
> + *reserved = NULL;
> + }
> return ret;
> }
>
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: free exchange changeset on failures
2021-12-03 10:55 [PATCH] btrfs: free exchange changeset on failures Johannes Thumshirn
2021-12-03 11:25 ` Filipe Manana
@ 2021-12-06 17:48 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: David Sterba @ 2021-12-06 17:48 UTC (permalink / raw)
To: Johannes Thumshirn; +Cc: David Sterba, linux-btrfs, Filipe Manana
On Fri, Dec 03, 2021 at 02:55:33AM -0800, Johannes Thumshirn wrote:
> Fstests runs on my VMs have show several kmemleak reports like the following.
>
> unreferenced object 0xffff88811ae59080 (size 64):
> comm "xfs_io", pid 12124, jiffies 4294987392 (age 6.368s)
> hex dump (first 32 bytes):
> 00 c0 1c 00 00 00 00 00 ff cf 1c 00 00 00 00 00 ................
> 90 97 e5 1a 81 88 ff ff 90 97 e5 1a 81 88 ff ff ................
> backtrace:
> [<00000000ac0176d2>] ulist_add_merge+0x60/0x150 [btrfs]
> [<0000000076e9f312>] set_state_bits+0x86/0xc0 [btrfs]
> [<0000000014fe73d6>] set_extent_bit+0x270/0x690 [btrfs]
> [<000000004f675208>] set_record_extent_bits+0x19/0x20 [btrfs]
> [<00000000b96137b1>] qgroup_reserve_data+0x274/0x310 [btrfs]
> [<0000000057e9dcbb>] btrfs_check_data_free_space+0x5c/0xa0 [btrfs]
> [<0000000019c4511d>] btrfs_delalloc_reserve_space+0x1b/0xa0 [btrfs]
> [<000000006d37e007>] btrfs_dio_iomap_begin+0x415/0x970 [btrfs]
> [<00000000fb8a74b8>] iomap_iter+0x161/0x1e0
> [<0000000071dff6ff>] __iomap_dio_rw+0x1df/0x700
> [<000000002567ba53>] iomap_dio_rw+0x5/0x20
> [<0000000072e555f8>] btrfs_file_write_iter+0x290/0x530 [btrfs]
> [<000000005eb3d845>] new_sync_write+0x106/0x180
> [<000000003fb505bf>] vfs_write+0x24d/0x2f0
> [<000000009bb57d37>] __x64_sys_pwrite64+0x69/0xa0
> [<000000003eba3fdf>] do_syscall_64+0x43/0x90
>
> In case brtfs_qgroup_reserve_data() or btrfs_delalloc_reserve_metadata()
> fail the allocated extent_changeset will not be freed.
>
> So in btrfs_check_data_free_space() and btrfs_delalloc_reserve_space()
> free the allocated extent_changeset to get rid of the allocated memory.
>
> Cc: Filipe Manana <fdmanana@suse.com>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Added to devel with Filipe's comment, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-06 17:49 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-03 10:55 [PATCH] btrfs: free exchange changeset on failures Johannes Thumshirn
2021-12-03 11:25 ` Filipe Manana
2021-12-06 17:48 ` David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.