Re: [f2fs-dev] [PATCH v2] f2fs: fix hungtask when decompressed fail

[Eric Biggers <ebiggers@kernel.org>]

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