* [PATCH v2 1/4] btrfs: fix extent map leak in unexpected scenario at unpin_extent_cache()
2024-03-13 17:28 ` [PATCH v2 0/4] btrfs: some minor fixes around extent maps fdmanana
@ 2024-03-13 17:28 ` fdmanana
2024-03-14 12:59 ` Anand Jain
2024-03-13 17:28 ` [PATCH v2 2/4] btrfs: fix warning messages not printing interval at unpin_extent_range() fdmanana
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: fdmanana @ 2024-03-13 17:28 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At unpin_extent_cache() if we happen to find an extent map with an
unexpected start offset, we jump to the 'out' label and never release the
reference we added to the extent map through the call to
lookup_extent_mapping(), therefore resulting in a leak. So fix this by
moving the free_extent_map() under the 'out' label.
Fixes: c03c89f821e5 ("btrfs: handle errors returned from unpin_extent_cache()")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/extent_map.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 347ca13d15a9..e03953dbcd5e 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -340,9 +340,9 @@ int unpin_extent_cache(struct btrfs_inode *inode, u64 start, u64 len, u64 gen)
em->mod_len = em->len;
}
- free_extent_map(em);
out:
write_unlock(&tree->lock);
+ free_extent_map(em);
return ret;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 1/4] btrfs: fix extent map leak in unexpected scenario at unpin_extent_cache()
2024-03-13 17:28 ` [PATCH v2 1/4] btrfs: fix extent map leak in unexpected scenario at unpin_extent_cache() fdmanana
@ 2024-03-14 12:59 ` Anand Jain
0 siblings, 0 replies; 12+ messages in thread
From: Anand Jain @ 2024-03-14 12:59 UTC (permalink / raw)
To: fdmanana, linux-btrfs
On 3/13/24 22:58, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> At unpin_extent_cache() if we happen to find an extent map with an
> unexpected start offset, we jump to the 'out' label and never release the
> reference we added to the extent map through the call to
> lookup_extent_mapping(), therefore resulting in a leak. So fix this by
> moving the free_extent_map() under the 'out' label.
>
> Fixes: c03c89f821e5 ("btrfs: handle errors returned from unpin_extent_cache()")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> fs/btrfs/extent_map.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index 347ca13d15a9..e03953dbcd5e 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -340,9 +340,9 @@ int unpin_extent_cache(struct btrfs_inode *inode, u64 start, u64 len, u64 gen)
> em->mod_len = em->len;
> }
>
> - free_extent_map(em);
> out:
> write_unlock(&tree->lock);
> + free_extent_map(em);
> return ret;
>
> }
Looks good.
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Thanks, Anand
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/4] btrfs: fix warning messages not printing interval at unpin_extent_range()
2024-03-13 17:28 ` [PATCH v2 0/4] btrfs: some minor fixes around extent maps fdmanana
2024-03-13 17:28 ` [PATCH v2 1/4] btrfs: fix extent map leak in unexpected scenario at unpin_extent_cache() fdmanana
@ 2024-03-13 17:28 ` fdmanana
2024-03-13 17:28 ` [PATCH v2 3/4] btrfs: fix message not properly printing interval when adding extent map fdmanana
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2024-03-13 17:28 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At unpin_extent_range() we print warning messages that are supposed to
print an interval in the form "[X, Y)", with the first element being an
inclusive start offset and the second element being the exclusive end
offset of a range. However we end up printing the range's length instead
of the range's exclusive end offset, so fix that to avoid having confusing
and non-sense messages in case we hit one of these unexpected scenarios.
Fixes: 00deaf04df35 ("btrfs: log messages at unpin_extent_range() during unexpected cases")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/extent_map.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index e03953dbcd5e..2cfc6e8cf76f 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -309,7 +309,7 @@ int unpin_extent_cache(struct btrfs_inode *inode, u64 start, u64 len, u64 gen)
btrfs_warn(fs_info,
"no extent map found for inode %llu (root %lld) when unpinning extent range [%llu, %llu), generation %llu",
btrfs_ino(inode), btrfs_root_id(inode->root),
- start, len, gen);
+ start, start + len, gen);
ret = -ENOENT;
goto out;
}
@@ -318,7 +318,7 @@ int unpin_extent_cache(struct btrfs_inode *inode, u64 start, u64 len, u64 gen)
btrfs_warn(fs_info,
"found extent map for inode %llu (root %lld) with unexpected start offset %llu when unpinning extent range [%llu, %llu), generation %llu",
btrfs_ino(inode), btrfs_root_id(inode->root),
- em->start, start, len, gen);
+ em->start, start, start + len, gen);
ret = -EUCLEAN;
goto out;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 3/4] btrfs: fix message not properly printing interval when adding extent map
2024-03-13 17:28 ` [PATCH v2 0/4] btrfs: some minor fixes around extent maps fdmanana
2024-03-13 17:28 ` [PATCH v2 1/4] btrfs: fix extent map leak in unexpected scenario at unpin_extent_cache() fdmanana
2024-03-13 17:28 ` [PATCH v2 2/4] btrfs: fix warning messages not printing interval at unpin_extent_range() fdmanana
@ 2024-03-13 17:28 ` fdmanana
2024-03-13 17:28 ` [PATCH v2 4/4] btrfs: use btrfs_warn() to log message at btrfs_add_extent_mapping() fdmanana
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2024-03-13 17:28 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At btrfs_add_extent_mapping(), if we are unable to merge the existing
extent map, we print a warning message that suggests interval ranges in
the form "[X, Y)", where the first element is the inclusive start offset
of a range and the second element is the exclusive end offset. However
we end up printing the length of the ranges instead of the exclusive end
offsets. So fix this by printing the range end offsets.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/extent_map.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 2cfc6e8cf76f..16685cb8a91d 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -634,8 +634,8 @@ int btrfs_add_extent_mapping(struct btrfs_fs_info *fs_info,
*em_in = NULL;
WARN_ONCE(ret,
"extent map merge error existing [%llu, %llu) with em [%llu, %llu) start %llu\n",
- existing->start, existing->len,
- orig_start, orig_len, start);
+ existing->start, extent_map_end(existing),
+ orig_start, orig_start + orig_len, start);
}
free_extent_map(existing);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v2 4/4] btrfs: use btrfs_warn() to log message at btrfs_add_extent_mapping()
2024-03-13 17:28 ` [PATCH v2 0/4] btrfs: some minor fixes around extent maps fdmanana
` (2 preceding siblings ...)
2024-03-13 17:28 ` [PATCH v2 3/4] btrfs: fix message not properly printing interval when adding extent map fdmanana
@ 2024-03-13 17:28 ` fdmanana
2024-03-13 19:22 ` [PATCH v2 0/4] btrfs: some minor fixes around extent maps Qu Wenruo
2024-03-14 13:06 ` Anand Jain
5 siblings, 0 replies; 12+ messages in thread
From: fdmanana @ 2024-03-13 17:28 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At btrfs_add_extent_mapping(), if we failed to merge the extent map, which
is unexpected and theoretically should never happen, we use WARN_ONCE() to
log a message which is not great because we don't get information about
which filesystem it relates to in case we have multiple btrfs filesystems
mounted. So change this to use btrfs_warn() and surround the error check
with WARN_ON() so we always get a useful stack trace and the condition is
flagged as "unlikely" since it's not expected to ever happen.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/extent_map.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 16685cb8a91d..445f7716f1e2 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -629,13 +629,13 @@ int btrfs_add_extent_mapping(struct btrfs_fs_info *fs_info,
*/
ret = merge_extent_mapping(em_tree, existing,
em, start);
- if (ret) {
+ if (WARN_ON(ret)) {
free_extent_map(em);
*em_in = NULL;
- WARN_ONCE(ret,
-"extent map merge error existing [%llu, %llu) with em [%llu, %llu) start %llu\n",
- existing->start, extent_map_end(existing),
- orig_start, orig_start + orig_len, start);
+ btrfs_warn(fs_info,
+"extent map merge error existing [%llu, %llu) with em [%llu, %llu) start %llu",
+ existing->start, extent_map_end(existing),
+ orig_start, orig_start + orig_len, start);
}
free_extent_map(existing);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v2 0/4] btrfs: some minor fixes around extent maps
2024-03-13 17:28 ` [PATCH v2 0/4] btrfs: some minor fixes around extent maps fdmanana
` (3 preceding siblings ...)
2024-03-13 17:28 ` [PATCH v2 4/4] btrfs: use btrfs_warn() to log message at btrfs_add_extent_mapping() fdmanana
@ 2024-03-13 19:22 ` Qu Wenruo
2024-03-14 13:06 ` Anand Jain
5 siblings, 0 replies; 12+ messages in thread
From: Qu Wenruo @ 2024-03-13 19:22 UTC (permalink / raw)
To: fdmanana, linux-btrfs
在 2024/3/14 03:58, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> Some minor fixes around extent maps for unexpected error cases.
> More details in the change logs.
>
> V2: Added patch 4/4.
>
> Filipe Manana (4):
> btrfs: fix extent map leak in unexpected scenario at unpin_extent_cache()
> btrfs: fix warning messages not printing interval at unpin_extent_range()
> btrfs: fix message not properly printing interval when adding extent map
> btrfs: use btrfs_warn() to log message at btrfs_add_extent_mapping()
Reviewed-by: Qu Wenruo <wqu@suse.com>
Thanks,
Qu
>
> fs/btrfs/extent_map.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v2 0/4] btrfs: some minor fixes around extent maps
2024-03-13 17:28 ` [PATCH v2 0/4] btrfs: some minor fixes around extent maps fdmanana
` (4 preceding siblings ...)
2024-03-13 19:22 ` [PATCH v2 0/4] btrfs: some minor fixes around extent maps Qu Wenruo
@ 2024-03-14 13:06 ` Anand Jain
5 siblings, 0 replies; 12+ messages in thread
From: Anand Jain @ 2024-03-14 13:06 UTC (permalink / raw)
To: fdmanana, linux-btrfs
On 3/13/24 22:58, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> Some minor fixes around extent maps for unexpected error cases.
> More details in the change logs.
>
> V2: Added patch 4/4.
>
> Filipe Manana (4):
> btrfs: fix extent map leak in unexpected scenario at unpin_extent_cache() > btrfs: fix warning messages not printing interval at
unpin_extent_range()
> btrfs: fix message not properly printing interval when adding extent map
> btrfs: use btrfs_warn() to log message at btrfs_add_extent_mapping()
For all.
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Thanks, Anand
>
> fs/btrfs/extent_map.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
^ permalink raw reply [flat|nested] 12+ messages in thread