* [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail
@ 2022-06-08 13:48 Fengnan Chang
2022-10-14 8:46 ` fengnan chang
2022-11-02 2:05 ` Eric Biggers
0 siblings, 2 replies; 9+ messages in thread
From: Fengnan Chang @ 2022-06-08 13:48 UTC (permalink / raw)
To: jaegeuk, chao; +Cc: linux-f2fs-devel
When decompressed failed, f2fs_prepare_compress_overwrite will enter
endless loop, may casue hungtask.
[ 14.088665] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
[ 14.089851] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
Signed-off-by: Fengnan Chang <fengnanchang@gmail.com>
---
fs/f2fs/compress.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 24824cd96f36..1764e3859262 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1060,7 +1060,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
sector_t last_block_in_bio;
unsigned fgp_flag = FGP_LOCK | FGP_WRITE | FGP_CREAT;
pgoff_t start_idx = start_idx_of_cluster(cc);
- int i, ret;
+ int i, ret, retry_count = 3;
retry:
ret = f2fs_is_compressed_cluster(cc->inode, start_idx);
@@ -1120,7 +1120,12 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
f2fs_put_rpages(cc);
f2fs_unlock_rpages(cc, i + 1);
f2fs_destroy_compress_ctx(cc, true);
- goto retry;
+ retry_count--;
+ if (PageError(page) && !retry_count) {
+ ret = -EIO;
+ goto out;
+ } else
+ goto retry;
}
}
@@ -1657,10 +1662,16 @@ static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
if (!rpage)
continue;
- /* PG_error was set if verity failed. */
- if (failed || PageError(rpage)) {
+ if (failed) {
+ /* decompress page failed */
+ ClearPageUptodate(rpage);
+ SetPageError(rpage);
+ } else if (PageError(rpage)) {
+ /*
+ * PG_error was set if verity failed.
+ * will re-read again later.
+ */
ClearPageUptodate(rpage);
- /* will re-read again later */
ClearPageError(rpage);
} else {
SetPageUptodate(rpage);
--
2.25.1
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail
2022-06-08 13:48 [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail Fengnan Chang
@ 2022-10-14 8:46 ` fengnan chang
2022-11-01 9:41 ` fengnan chang
2022-11-02 2:05 ` Eric Biggers
1 sibling, 1 reply; 9+ messages in thread
From: fengnan chang @ 2022-10-14 8:46 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel
ping, it seems this had been forgotten.
> 2022年6月8日 21:48，Fengnan Chang <fengnanchang@gmail.com> 写道：
>
> When decompressed failed, f2fs_prepare_compress_overwrite will enter
> endless loop, may casue hungtask.
>
> [ 14.088665] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
> [ 14.089851] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
>
> Signed-off-by: Fengnan Chang <fengnanchang@gmail.com>
> ---
> fs/f2fs/compress.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 24824cd96f36..1764e3859262 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1060,7 +1060,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> sector_t last_block_in_bio;
> unsigned fgp_flag = FGP_LOCK | FGP_WRITE | FGP_CREAT;
> pgoff_t start_idx = start_idx_of_cluster(cc);
> - int i, ret;
> + int i, ret, retry_count = 3;
>
> retry:
> ret = f2fs_is_compressed_cluster(cc->inode, start_idx);
> @@ -1120,7 +1120,12 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> f2fs_put_rpages(cc);
> f2fs_unlock_rpages(cc, i + 1);
> f2fs_destroy_compress_ctx(cc, true);
> - goto retry;
> + retry_count--;
> + if (PageError(page) && !retry_count) {
> + ret = -EIO;
> + goto out;
> + } else
> + goto retry;
> }
> }
>
> @@ -1657,10 +1662,16 @@ static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
> if (!rpage)
> continue;
>
> - /* PG_error was set if verity failed. */
> - if (failed || PageError(rpage)) {
> + if (failed) {
> + /* decompress page failed */
> + ClearPageUptodate(rpage);
> + SetPageError(rpage);
> + } else if (PageError(rpage)) {
> + /*
> + * PG_error was set if verity failed.
> + * will re-read again later.
> + */
> ClearPageUptodate(rpage);
> - /* will re-read again later */
> ClearPageError(rpage);
> } else {
> SetPageUptodate(rpage);
> --
> 2.25.1
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail
2022-10-14 8:46 ` fengnan chang
@ 2022-11-01 9:41 ` fengnan chang
2022-11-01 15:06 ` Chao Yu
0 siblings, 1 reply; 9+ messages in thread
From: fengnan chang @ 2022-11-01 9:41 UTC (permalink / raw)
To: Jaegeuk Kim, Chao Yu; +Cc: linux-f2fs-devel
friendly ping...
fengnan chang <fengnanchang@gmail.com> 于2022年10月14日周五 16:46写道：
>
> ping, it seems this had been forgotten.
>
> > 2022年6月8日 21:48，Fengnan Chang <fengnanchang@gmail.com> 写道：
> >
> > When decompressed failed, f2fs_prepare_compress_overwrite will enter
> > endless loop, may casue hungtask.
> >
> > [ 14.088665] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
> > [ 14.089851] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
> >
> > Signed-off-by: Fengnan Chang <fengnanchang@gmail.com>
> > ---
> > fs/f2fs/compress.c | 21 ++++++++++++++++-----
> > 1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > index 24824cd96f36..1764e3859262 100644
> > --- a/fs/f2fs/compress.c
> > +++ b/fs/f2fs/compress.c
> > @@ -1060,7 +1060,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> > sector_t last_block_in_bio;
> > unsigned fgp_flag = FGP_LOCK | FGP_WRITE | FGP_CREAT;
> > pgoff_t start_idx = start_idx_of_cluster(cc);
> > - int i, ret;
> > + int i, ret, retry_count = 3;
> >
> > retry:
> > ret = f2fs_is_compressed_cluster(cc->inode, start_idx);
> > @@ -1120,7 +1120,12 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> > f2fs_put_rpages(cc);
> > f2fs_unlock_rpages(cc, i + 1);
> > f2fs_destroy_compress_ctx(cc, true);
> > - goto retry;
> > + retry_count--;
> > + if (PageError(page) && !retry_count) {
> > + ret = -EIO;
> > + goto out;
> > + } else
> > + goto retry;
> > }
> > }
> >
> > @@ -1657,10 +1662,16 @@ static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
> > if (!rpage)
> > continue;
> >
> > - /* PG_error was set if verity failed. */
> > - if (failed || PageError(rpage)) {
> > + if (failed) {
> > + /* decompress page failed */
> > + ClearPageUptodate(rpage);
> > + SetPageError(rpage);
> > + } else if (PageError(rpage)) {
> > + /*
> > + * PG_error was set if verity failed.
> > + * will re-read again later.
> > + */
> > ClearPageUptodate(rpage);
> > - /* will re-read again later */
> > ClearPageError(rpage);
> > } else {
> > SetPageUptodate(rpage);
> > --
> > 2.25.1
> >
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail
2022-11-01 9:41 ` fengnan chang
@ 2022-11-01 15:06 ` Chao Yu
0 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2022-11-01 15:06 UTC (permalink / raw)
To: fengnan chang, Jaegeuk Kim; +Cc: linux-f2fs-devel
Hi Fengnan,
Sorry for the delay.
I guess we can merge this patch, but let me check whether there
is another solution, since mm guys want to remove PG_error usage
to save one bit slot in page.flags.
On 2022/11/1 17:41, fengnan chang wrote:
> friendly ping...
>
> fengnan chang <fengnanchang@gmail.com> 于2022年10月14日周五 16:46写道：
>>
>> ping, it seems this had been forgotten.
>>
>>> 2022年6月8日 21:48，Fengnan Chang <fengnanchang@gmail.com> 写道：
>>>
>>> When decompressed failed, f2fs_prepare_compress_overwrite will enter
>>> endless loop, may casue hungtask.
>>>
>>> [ 14.088665] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
>>> [ 14.089851] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
>>>
>>> Signed-off-by: Fengnan Chang <fengnanchang@gmail.com>
>>> ---
>>> fs/f2fs/compress.c | 21 ++++++++++++++++-----
>>> 1 file changed, 16 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>>> index 24824cd96f36..1764e3859262 100644
>>> --- a/fs/f2fs/compress.c
>>> +++ b/fs/f2fs/compress.c
>>> @@ -1060,7 +1060,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>>> sector_t last_block_in_bio;
>>> unsigned fgp_flag = FGP_LOCK | FGP_WRITE | FGP_CREAT;
>>> pgoff_t start_idx = start_idx_of_cluster(cc);
>>> - int i, ret;
>>> + int i, ret, retry_count = 3;
Wrap magic number w/ macro?
>>>
>>> retry:
>>> ret = f2fs_is_compressed_cluster(cc->inode, start_idx);
>>> @@ -1120,7 +1120,12 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>>> f2fs_put_rpages(cc);
>>> f2fs_unlock_rpages(cc, i + 1);
>>> f2fs_destroy_compress_ctx(cc, true);
>>> - goto retry;
>>> + retry_count--;
>>> + if (PageError(page) && !retry_count) {
>>> + ret = -EIO;
>>> + goto out;
>>> + } else
>>> + goto retry;
} else {
goto retry;
}
>>> }
>>> }
>>>
>>> @@ -1657,10 +1662,16 @@ static void __f2fs_decompress_end_io(struct decompress_io_ctx *dic, bool failed)
>>> if (!rpage)
>>> continue;
>>>
>>> - /* PG_error was set if verity failed. */
>>> - if (failed || PageError(rpage)) {
>>> + if (failed) {
>>> + /* decompress page failed */
>>> + ClearPageUptodate(rpage);
>>> + SetPageError(rpage);
>>> + } else if (PageError(rpage)) {
>>> + /*
>>> + * PG_error was set if verity failed.
>>> + * will re-read again later.
>>> + */
>>> ClearPageUptodate(rpage);
>>> - /* will re-read again later */
>>> ClearPageError(rpage);
>>> } else {
>>> SetPageUptodate(rpage);
>>> --
>>> 2.25.1
>>>
>>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail
2022-06-08 13:48 [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail Fengnan Chang
2022-10-14 8:46 ` fengnan chang
@ 2022-11-02 2:05 ` Eric Biggers
2022-11-02 3:06 ` fengnan chang
1 sibling, 1 reply; 9+ messages in thread
From: Eric Biggers @ 2022-11-02 2:05 UTC (permalink / raw)
To: Fengnan Chang; +Cc: jaegeuk, linux-f2fs-devel
On Wed, Jun 08, 2022 at 09:48:52PM +0800, Fengnan Chang wrote:
> When decompressed failed, f2fs_prepare_compress_overwrite will enter
> endless loop, may casue hungtask.
>
> [ 14.088665] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
> [ 14.089851] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
>
> Signed-off-by: Fengnan Chang <fengnanchang@gmail.com>
This commit message only explains the problem, not the solution. It should
explain the solution too.
Also, if it's fixing a bug, it needs 'Fixes' and 'Cc stable' tags.
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 24824cd96f36..1764e3859262 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1060,7 +1060,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> sector_t last_block_in_bio;
> unsigned fgp_flag = FGP_LOCK | FGP_WRITE | FGP_CREAT;
> pgoff_t start_idx = start_idx_of_cluster(cc);
> - int i, ret;
> + int i, ret, retry_count = 3;
>
> retry:
> ret = f2fs_is_compressed_cluster(cc->inode, start_idx);
> @@ -1120,7 +1120,12 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> f2fs_put_rpages(cc);
> f2fs_unlock_rpages(cc, i + 1);
> f2fs_destroy_compress_ctx(cc, true);
> - goto retry;
> + retry_count--;
> + if (PageError(page) && !retry_count) {
> + ret = -EIO;
> + goto out;
> + } else
What is the purpose of using PG_error here?
- Eric
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail
2022-11-02 2:05 ` Eric Biggers
@ 2022-11-02 3:06 ` fengnan chang
2022-11-02 3:18 ` Eric Biggers
0 siblings, 1 reply; 9+ messages in thread
From: fengnan chang @ 2022-11-02 3:06 UTC (permalink / raw)
To: Eric Biggers; +Cc: Jaegeuk Kim, linux-f2fs-devel
> 2022年11月2日 10:05，Eric Biggers <ebiggers@kernel.org> 写道：
>
> On Wed, Jun 08, 2022 at 09:48:52PM +0800, Fengnan Chang wrote:
>> When decompressed failed, f2fs_prepare_compress_overwrite will enter
>> endless loop, may casue hungtask.
>>
>> [ 14.088665] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
>> [ 14.089851] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
>>
>> Signed-off-by: Fengnan Chang <fengnanchang@gmail.com>
>
> This commit message only explains the problem, not the solution. It should
> explain the solution too.
>
> Also, if it's fixing a bug, it needs 'Fixes' and 'Cc stable' tags.
I’ll add more explanation and tags, thanks.
>
>> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
>> index 24824cd96f36..1764e3859262 100644
>> --- a/fs/f2fs/compress.c
>> +++ b/fs/f2fs/compress.c
>> @@ -1060,7 +1060,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>> sector_t last_block_in_bio;
>> unsigned fgp_flag = FGP_LOCK | FGP_WRITE | FGP_CREAT;
>> pgoff_t start_idx = start_idx_of_cluster(cc);
>> - int i, ret;
>> + int i, ret, retry_count = 3;
>>
>> retry:
>> ret = f2fs_is_compressed_cluster(cc->inode, start_idx);
>> @@ -1120,7 +1120,12 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
>> f2fs_put_rpages(cc);
>> f2fs_unlock_rpages(cc, i + 1);
>> f2fs_destroy_compress_ctx(cc, true);
>> - goto retry;
>> + retry_count--;
>> + if (PageError(page) && !retry_count) {
>> + ret = -EIO;
>> + goto out;
>> + } else
>
> What is the purpose of using PG_error here?
In this version, we set PG_error when compressed failed, so check PG_error here.
Maybe we can remove PG_error in later?
>
> - Eric
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail
2022-11-02 3:06 ` fengnan chang
@ 2022-11-02 3:18 ` Eric Biggers
2022-11-02 3:43 ` fengnan chang
0 siblings, 1 reply; 9+ messages in thread
From: Eric Biggers @ 2022-11-02 3:18 UTC (permalink / raw)
To: fengnan chang; +Cc: Jaegeuk Kim, linux-f2fs-devel
On Wed, Nov 02, 2022 at 11:06:17AM +0800, fengnan chang wrote:
>
>
> > 2022年11月2日 10:05，Eric Biggers <ebiggers@kernel.org> 写道：
> >
> > On Wed, Jun 08, 2022 at 09:48:52PM +0800, Fengnan Chang wrote:
> >> When decompressed failed, f2fs_prepare_compress_overwrite will enter
> >> endless loop, may casue hungtask.
> >>
> >> [ 14.088665] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
> >> [ 14.089851] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
> >>
> >> Signed-off-by: Fengnan Chang <fengnanchang@gmail.com>
> >
> > This commit message only explains the problem, not the solution. It should
> > explain the solution too.
> >
> > Also, if it's fixing a bug, it needs 'Fixes' and 'Cc stable' tags.
>
> I’ll add more explanation and tags, thanks.
>
> >
> >> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> >> index 24824cd96f36..1764e3859262 100644
> >> --- a/fs/f2fs/compress.c
> >> +++ b/fs/f2fs/compress.c
> >> @@ -1060,7 +1060,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> >> sector_t last_block_in_bio;
> >> unsigned fgp_flag = FGP_LOCK | FGP_WRITE | FGP_CREAT;
> >> pgoff_t start_idx = start_idx_of_cluster(cc);
> >> - int i, ret;
> >> + int i, ret, retry_count = 3;
> >>
> >> retry:
> >> ret = f2fs_is_compressed_cluster(cc->inode, start_idx);
> >> @@ -1120,7 +1120,12 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> >> f2fs_put_rpages(cc);
> >> f2fs_unlock_rpages(cc, i + 1);
> >> f2fs_destroy_compress_ctx(cc, true);
> >> - goto retry;
> >> + retry_count--;
> >> + if (PageError(page) && !retry_count) {
> >> + ret = -EIO;
> >> + goto out;
> >> + } else
> >
> > What is the purpose of using PG_error here?
>
> In this version, we set PG_error when compressed failed, so check PG_error here.
> Maybe we can remove PG_error in later?
>
Read I/O errors can be detected via PG_uptodate not being set. There shouldn't
be any need for PG_error here.
- Eric
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail
2022-11-02 3:18 ` Eric Biggers
@ 2022-11-02 3:43 ` fengnan chang
2022-11-02 3:48 ` Eric Biggers
0 siblings, 1 reply; 9+ messages in thread
From: fengnan chang @ 2022-11-02 3:43 UTC (permalink / raw)
To: Eric Biggers; +Cc: Jaegeuk Kim, linux-f2fs-devel
Eric Biggers <ebiggers@kernel.org> 于2022年11月2日周三 11:18写道：
>
> On Wed, Nov 02, 2022 at 11:06:17AM +0800, fengnan chang wrote:
> >
> >
> > > 2022年11月2日 10:05，Eric Biggers <ebiggers@kernel.org> 写道：
> > >
> > > On Wed, Jun 08, 2022 at 09:48:52PM +0800, Fengnan Chang wrote:
> > >> When decompressed failed, f2fs_prepare_compress_overwrite will enter
> > >> endless loop, may casue hungtask.
> > >>
> > >> [ 14.088665] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
> > >> [ 14.089851] F2FS-fs (nvme0n1): lz4 decompress failed, ret:-4155
> > >>
> > >> Signed-off-by: Fengnan Chang <fengnanchang@gmail.com>
> > >
> > > This commit message only explains the problem, not the solution. It should
> > > explain the solution too.
> > >
> > > Also, if it's fixing a bug, it needs 'Fixes' and 'Cc stable' tags.
> >
> > I’ll add more explanation and tags, thanks.
> >
> > >
> > >> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > >> index 24824cd96f36..1764e3859262 100644
> > >> --- a/fs/f2fs/compress.c
> > >> +++ b/fs/f2fs/compress.c
> > >> @@ -1060,7 +1060,7 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> > >> sector_t last_block_in_bio;
> > >> unsigned fgp_flag = FGP_LOCK | FGP_WRITE | FGP_CREAT;
> > >> pgoff_t start_idx = start_idx_of_cluster(cc);
> > >> - int i, ret;
> > >> + int i, ret, retry_count = 3;
> > >>
> > >> retry:
> > >> ret = f2fs_is_compressed_cluster(cc->inode, start_idx);
> > >> @@ -1120,7 +1120,12 @@ static int prepare_compress_overwrite(struct compress_ctx *cc,
> > >> f2fs_put_rpages(cc);
> > >> f2fs_unlock_rpages(cc, i + 1);
> > >> f2fs_destroy_compress_ctx(cc, true);
> > >> - goto retry;
> > >> + retry_count--;
> > >> + if (PageError(page) && !retry_count) {
> > >> + ret = -EIO;
> > >> + goto out;
> > >> + } else
> > >
> > > What is the purpose of using PG_error here?
> >
> > In this version, we set PG_error when compressed failed, so check PG_error here.
> > Maybe we can remove PG_error in later?
> >
>
> Read I/O errors can be detected via PG_uptodate not being set. There shouldn't
> be any need for PG_error here.
Yeah, I get it now. Maybe we can remove PG_error in f2fs_verify_cluster too.
diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d315c2de136f..13c0bfe45804 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1727,10 +1727,9 @@ static void __f2fs_decompress_end_io(struct
decompress_io_ctx *dic, bool failed,
continue;
/* PG_error was set if verity failed. */
- if (failed || PageError(rpage)) {
- ClearPageUptodate(rpage);
+ if (failed) {
/* will re-read again later */
- ClearPageError(rpage);
+ ClearPageUptodate(rpage);
} else {
SetPageUptodate(rpage);
}
@@ -1745,13 +1744,14 @@ static void f2fs_verify_cluster(struct
work_struct *work)
struct decompress_io_ctx *dic =
container_of(work, struct decompress_io_ctx, verity_work);
int i;
+ bool failed = false;
/* Verify the cluster's decompressed pages with fs-verity. */
for (i = 0; i < dic->cluster_size; i++) {
struct page *rpage = dic->rpages[i];
if (rpage && !fsverity_verify_page(rpage))
- SetPageError(rpage);
+ failed = true;
}
__f2fs_decompress_end_io(dic, false, true);
>
> - Eric
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail
2022-11-02 3:43 ` fengnan chang
@ 2022-11-02 3:48 ` Eric Biggers
0 siblings, 0 replies; 9+ messages in thread
From: Eric Biggers @ 2022-11-02 3:48 UTC (permalink / raw)
To: fengnan chang; +Cc: Jaegeuk Kim, linux-f2fs-devel
On Wed, Nov 02, 2022 at 11:43:11AM +0800, fengnan chang wrote:
> > > > What is the purpose of using PG_error here?
> > >
> > > In this version, we set PG_error when compressed failed, so check PG_error here.
> > > Maybe we can remove PG_error in later?
> > >
> >
> > Read I/O errors can be detected via PG_uptodate not being set. There shouldn't
> > be any need for PG_error here.
>
> Yeah, I get it now. Maybe we can remove PG_error in f2fs_verify_cluster too.
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index d315c2de136f..13c0bfe45804 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1727,10 +1727,9 @@ static void __f2fs_decompress_end_io(struct
> decompress_io_ctx *dic, bool failed,
> continue;
>
> /* PG_error was set if verity failed. */
> - if (failed || PageError(rpage)) {
> - ClearPageUptodate(rpage);
> + if (failed) {
> /* will re-read again later */
> - ClearPageError(rpage);
> + ClearPageUptodate(rpage);
> } else {
> SetPageUptodate(rpage);
> }
> @@ -1745,13 +1744,14 @@ static void f2fs_verify_cluster(struct
> work_struct *work)
> struct decompress_io_ctx *dic =
> container_of(work, struct decompress_io_ctx, verity_work);
> int i;
> + bool failed = false;
>
> /* Verify the cluster's decompressed pages with fs-verity. */
> for (i = 0; i < dic->cluster_size; i++) {
> struct page *rpage = dic->rpages[i];
>
> if (rpage && !fsverity_verify_page(rpage))
> - SetPageError(rpage);
> + failed = true;
> }
No, PG_error is still used to notify f2fs_finish_read_bio() and
__f2fs_decompress_end_io() of verity errors. My patch
https://lore.kernel.org/r/20221028175807.55495-1-ebiggers@kernel.org changes
that. Please leave that to my patch. For your patch, please just don't add a
new use of PG_error, as it doesn't seem to be necessary.
- Eric
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-11-02 3:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-06-08 13:48 [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail Fengnan Chang
2022-10-14 8:46 ` fengnan chang
2022-11-01 9:41 ` fengnan chang
2022-11-01 15:06 ` Chao Yu
2022-11-02 2:05 ` Eric Biggers
2022-11-02 3:06 ` fengnan chang
2022-11-02 3:18 ` Eric Biggers
2022-11-02 3:43 ` fengnan chang
2022-11-02 3:48 ` Eric Biggers
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.