* [PATCH] btrfs: Fix return value of btrfs_mark_extent_written() in case of error
@ 2021-05-30 14:54 Ritesh Harjani
2021-05-31 18:49 ` David Sterba
0 siblings, 1 reply; 4+ messages in thread
From: Ritesh Harjani @ 2021-05-30 14:54 UTC (permalink / raw
To: linux-btrfs; +Cc: Qu Wenruo, Anju T Sudhakar, Ritesh Harjani
We always return 0 even in case of an error in btrfs_mark_extent_written().
Fix it to return proper error value in case of a failure.
Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
---
Tested fstests with "-g quick" group. No new failure seen.
fs/btrfs/file.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
index 3b10d98b4ebb..55f68422061d 100644
--- a/fs/btrfs/file.c
+++ b/fs/btrfs/file.c
@@ -1094,7 +1094,7 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
int del_nr = 0;
int del_slot = 0;
int recow;
- int ret;
+ int ret = 0;
u64 ino = btrfs_ino(inode);
path = btrfs_alloc_path();
@@ -1315,7 +1315,7 @@ int btrfs_mark_extent_written(struct btrfs_trans_handle *trans,
}
out:
btrfs_free_path(path);
- return 0;
+ return ret;
}
/*
--
2.31.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: Fix return value of btrfs_mark_extent_written() in case of error
2021-05-30 14:54 [PATCH] btrfs: Fix return value of btrfs_mark_extent_written() in case of error Ritesh Harjani
@ 2021-05-31 18:49 ` David Sterba
2021-06-01 6:01 ` Ritesh Harjani
0 siblings, 1 reply; 4+ messages in thread
From: David Sterba @ 2021-05-31 18:49 UTC (permalink / raw
To: Ritesh Harjani; +Cc: linux-btrfs, Qu Wenruo, Anju T Sudhakar
On Sun, May 30, 2021 at 08:24:05PM +0530, Ritesh Harjani wrote:
> We always return 0 even in case of an error in btrfs_mark_extent_written().
> Fix it to return proper error value in case of a failure.
Oh right, this looks like it got forgotten, the whole function does the
right thing regarding errors and the callers also handle it.
> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> ---
> Tested fstests with "-g quick" group. No new failure seen.
That won't be probably enough to trigger the error paths but the patch
otherwise looks correct to me. Added to misc-next, thanks.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: Fix return value of btrfs_mark_extent_written() in case of error
2021-05-31 18:49 ` David Sterba
@ 2021-06-01 6:01 ` Ritesh Harjani
2021-06-01 6:17 ` Qu Wenruo
0 siblings, 1 reply; 4+ messages in thread
From: Ritesh Harjani @ 2021-06-01 6:01 UTC (permalink / raw
To: dsterba, linux-btrfs, Qu Wenruo, Anju T Sudhakar
On 21/05/31 08:49PM, David Sterba wrote:
> On Sun, May 30, 2021 at 08:24:05PM +0530, Ritesh Harjani wrote:
> > We always return 0 even in case of an error in btrfs_mark_extent_written().
> > Fix it to return proper error value in case of a failure.
>
> Oh right, this looks like it got forgotten, the whole function does the
> right thing regarding errors and the callers also handle it.
>
> > Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
> > ---
> > Tested fstests with "-g quick" group. No new failure seen.
>
> That won't be probably enough to trigger the error paths but the patch
Yes. However, I found the bug while stress testing btrfs/062 failure
with bs < ps patch series from Qu.
On analyzing the kernel panic [1], it looked like it was caused by not
properly returning an error from btrfs_mark_extent_written().
So, I thought of just running "-g quick" test to avoid any obvious issue.
[1]: https://www.spinics.net/lists/linux-btrfs/msg113280.html
> otherwise looks correct to me. Added to misc-next, thanks.
Thanks
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] btrfs: Fix return value of btrfs_mark_extent_written() in case of error
2021-06-01 6:01 ` Ritesh Harjani
@ 2021-06-01 6:17 ` Qu Wenruo
0 siblings, 0 replies; 4+ messages in thread
From: Qu Wenruo @ 2021-06-01 6:17 UTC (permalink / raw
To: Ritesh Harjani, dsterba, linux-btrfs, Qu Wenruo, Anju T Sudhakar
On 2021/6/1 下午2:01, Ritesh Harjani wrote:
> On 21/05/31 08:49PM, David Sterba wrote:
>> On Sun, May 30, 2021 at 08:24:05PM +0530, Ritesh Harjani wrote:
>>> We always return 0 even in case of an error in btrfs_mark_extent_written().
>>> Fix it to return proper error value in case of a failure.
>>
>> Oh right, this looks like it got forgotten, the whole function does the
>> right thing regarding errors and the callers also handle it.
>>
>>> Signed-off-by: Ritesh Harjani <riteshh@linux.ibm.com>
>>> ---
>>> Tested fstests with "-g quick" group. No new failure seen.
>>
>> That won't be probably enough to trigger the error paths but the patch
>
> Yes. However, I found the bug while stress testing btrfs/062 failure
> with bs < ps patch series from Qu.
> On analyzing the kernel panic [1], it looked like it was caused by not
> properly returning an error from btrfs_mark_extent_written().
> So, I thought of just running "-g quick" test to avoid any obvious issue.
In fact for the btrfs/062 bug, it's caused by the full page defrag behavior.
I wrongly thought enabling full page defrag would be OK, as most defrag
tests are fine, but it turns out that full page defrag is buggy, and can
lead to btrfs/062 problem and other ordered extent related problem.
Thus now on subpage branch, subpage defrag is fully disabled.
For subpage defrag support, the incoming subpage defrag patchset would
be the proper way to go.
No some compromise like full page defrag.
The patch you're submitting is completely fine, I guess it's just
btrfs/062 with full page defrag creates a situation to make
btrfs_mark_extent_written() to fail.
Thanks,
Qu
>
> [1]: https://www.spinics.net/lists/linux-btrfs/msg113280.html
>
>> otherwise looks correct to me. Added to misc-next, thanks.
>
> Thanks
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-01 6:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-30 14:54 [PATCH] btrfs: Fix return value of btrfs_mark_extent_written() in case of error Ritesh Harjani
2021-05-31 18:49 ` David Sterba
2021-06-01 6:01 ` Ritesh Harjani
2021-06-01 6:17 ` Qu Wenruo
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.