diff mbox series

[f2fs-dev,2/3] f2fs: clear writeback when compression failed

Message ID 20240409203411.1885121-2-jaegeuk@kernel.org (mailing list archive)
State New
Headers show
Series [f2fs-dev,1/3] f2fs: use folio_test_writeback | expand

Commit Message

Jaegeuk Kim April 9, 2024, 8:34 p.m. UTC 
Let's stop issuing compressed writes and clear their writeback flags.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
 fs/f2fs/compress.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

Comments

Chao Yu April 11, 2024, 9:05 a.m. UTC | #1 
On 2024/4/10 4:34, Jaegeuk Kim wrote:
> Let's stop issuing compressed writes and clear their writeback flags.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>   fs/f2fs/compress.c | 33 +++++++++++++++++++++++++++++++--
>   1 file changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index d67c471ab5df..3a8ecc6aee84 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1031,6 +1031,25 @@ static void set_cluster_writeback(struct compress_ctx *cc)
>   	}
>   }
>   
> +static void cancel_cluster_writeback(struct compress_ctx *cc, int submitted)
> +{
> +	int i;
> +
> +	for (i = 0; i < cc->cluster_size; i++) {
> +		if (!cc->rpages[i])
> +			continue;
> +		if (i < submitted) {
> +			if (i)
> +				f2fs_wait_on_page_writeback(cc->rpages[i],
> +						DATA, true, true);
> +			inode_inc_dirty_pages(cc->inode);
> +			lock_page(cc->rpages[i]);
> +		}
> +		clear_page_private_gcing(cc->rpages[i]);
> +		end_page_writeback(cc->rpages[i]);
> +	}
> +}
> +
>   static void set_cluster_dirty(struct compress_ctx *cc)
>   {
>   	int i;
> @@ -1232,7 +1251,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>   		.page = NULL,
>   		.encrypted_page = NULL,
>   		.compressed_page = NULL,
> -		.submitted = 0,
>   		.io_type = io_type,
>   		.io_wbc = wbc,
>   		.encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
> @@ -1358,7 +1376,15 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>   			fio.compressed_page = cc->cpages[i - 1];
>   
>   		cc->cpages[i - 1] = NULL;
> +		fio.submitted = 0;
>   		f2fs_outplace_write_data(&dn, &fio);
> +		if (unlikely(!fio.submitted)) {
> +			cancel_cluster_writeback(cc, i);
> +
> +			/* To call fscrypt_finalize_bounce_page */
> +			i = cc->valid_nr_cpages;

*submitted = 0; ?

Thanks,

> +			goto out_destroy_crypt;
> +		}
>   		(*submitted)++;
>   unlock_continue:
>   		inode_dec_dirty_pages(cc->inode);
> @@ -1392,8 +1418,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>   out_destroy_crypt:
>   	page_array_free(cc->inode, cic->rpages, cc->cluster_size);
>   
> -	for (--i; i >= 0; i--)
> +	for (--i; i >= 0; i--) {
> +		if (!cc->cpages[i])
> +			continue;
>   		fscrypt_finalize_bounce_page(&cc->cpages[i]);
> +	}
>   out_put_cic:
>   	kmem_cache_free(cic_entry_slab, cic);
>   out_put_dnode:
Jaegeuk Kim April 12, 2024, 8:57 p.m. UTC | #2 
On 04/11, Chao Yu wrote:
> On 2024/4/10 4:34, Jaegeuk Kim wrote:
> > Let's stop issuing compressed writes and clear their writeback flags.
> > 
> > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> > ---
> >   fs/f2fs/compress.c | 33 +++++++++++++++++++++++++++++++--
> >   1 file changed, 31 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> > index d67c471ab5df..3a8ecc6aee84 100644
> > --- a/fs/f2fs/compress.c
> > +++ b/fs/f2fs/compress.c
> > @@ -1031,6 +1031,25 @@ static void set_cluster_writeback(struct compress_ctx *cc)
> >   	}
> >   }
> > +static void cancel_cluster_writeback(struct compress_ctx *cc, int submitted)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < cc->cluster_size; i++) {
> > +		if (!cc->rpages[i])
> > +			continue;
> > +		if (i < submitted) {
> > +			if (i)
> > +				f2fs_wait_on_page_writeback(cc->rpages[i],
> > +						DATA, true, true);
> > +			inode_inc_dirty_pages(cc->inode);
> > +			lock_page(cc->rpages[i]);
> > +		}
> > +		clear_page_private_gcing(cc->rpages[i]);
> > +		end_page_writeback(cc->rpages[i]);
> > +	}
> > +}
> > +
> >   static void set_cluster_dirty(struct compress_ctx *cc)
> >   {
> >   	int i;
> > @@ -1232,7 +1251,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> >   		.page = NULL,
> >   		.encrypted_page = NULL,
> >   		.compressed_page = NULL,
> > -		.submitted = 0,
> >   		.io_type = io_type,
> >   		.io_wbc = wbc,
> >   		.encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
> > @@ -1358,7 +1376,15 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> >   			fio.compressed_page = cc->cpages[i - 1];
> >   		cc->cpages[i - 1] = NULL;
> > +		fio.submitted = 0;
> >   		f2fs_outplace_write_data(&dn, &fio);
> > +		if (unlikely(!fio.submitted)) {
> > +			cancel_cluster_writeback(cc, i);
> > +
> > +			/* To call fscrypt_finalize_bounce_page */
> > +			i = cc->valid_nr_cpages;
> 
> *submitted = 0; ?

And, it seems this is not enough to address kernel hang on wait_on_writeback
while running fsstress + shutdown test. Stay tuned.

> 
> Thanks,
> 
> > +			goto out_destroy_crypt;
> > +		}
> >   		(*submitted)++;
> >   unlock_continue:
> >   		inode_dec_dirty_pages(cc->inode);
> > @@ -1392,8 +1418,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
> >   out_destroy_crypt:
> >   	page_array_free(cc->inode, cic->rpages, cc->cluster_size);
> > -	for (--i; i >= 0; i--)
> > +	for (--i; i >= 0; i--) {
> > +		if (!cc->cpages[i])
> > +			continue;
> >   		fscrypt_finalize_bounce_page(&cc->cpages[i]);
> > +	}
> >   out_put_cic:
> >   	kmem_cache_free(cic_entry_slab, cic);
> >   out_put_dnode:
Jaegeuk Kim April 16, 2024, 4:49 p.m. UTC | #3 
Let's stop issuing compressed writes and clear their writeback flags.

Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---

 Now, I don't see any kernel hang for 24hours.

 Change log from v1:
  - fix bugs

 fs/f2fs/compress.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d67c471ab5df..b12d3a49bfda 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1031,6 +1031,31 @@ static void set_cluster_writeback(struct compress_ctx *cc)
 	}
 }
 
+static void cancel_cluster_writeback(struct compress_ctx *cc,
+			struct compress_io_ctx *cic, int submitted)
+{
+	int i;
+
+	/* Wait for submitted IOs. */
+	if (submitted > 1) {
+		f2fs_submit_merged_write(F2FS_I_SB(cc->inode), DATA);
+		while (atomic_read(&cic->pending_pages) !=
+					(cc->valid_nr_cpages - submitted + 1))
+			f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
+	}
+
+	/* Cancel writeback and stay locked. */
+	for (i = 0; i < cc->cluster_size; i++) {
+		if (i < submitted) {
+			inode_inc_dirty_pages(cc->inode);
+			lock_page(cc->rpages[i]);
+		}
+		clear_page_private_gcing(cc->rpages[i]);
+		if (folio_test_writeback(page_folio(cc->rpages[i])))
+			end_page_writeback(cc->rpages[i]);
+	}
+}
+
 static void set_cluster_dirty(struct compress_ctx *cc)
 {
 	int i;
@@ -1232,7 +1257,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
 		.page = NULL,
 		.encrypted_page = NULL,
 		.compressed_page = NULL,
-		.submitted = 0,
 		.io_type = io_type,
 		.io_wbc = wbc,
 		.encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
@@ -1358,7 +1382,16 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
 			fio.compressed_page = cc->cpages[i - 1];
 
 		cc->cpages[i - 1] = NULL;
+		fio.submitted = 0;
 		f2fs_outplace_write_data(&dn, &fio);
+		if (unlikely(!fio.submitted)) {
+			cancel_cluster_writeback(cc, cic, i);
+
+			/* To call fscrypt_finalize_bounce_page */
+			i = cc->valid_nr_cpages;
+			*submitted = 0;
+			goto out_destroy_crypt;
+		}
 		(*submitted)++;
 unlock_continue:
 		inode_dec_dirty_pages(cc->inode);
@@ -1392,8 +1425,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
 out_destroy_crypt:
 	page_array_free(cc->inode, cic->rpages, cc->cluster_size);
 
-	for (--i; i >= 0; i--)
+	for (--i; i >= 0; i--) {
+		if (!cc->cpages[i])
+			continue;
 		fscrypt_finalize_bounce_page(&cc->cpages[i]);
+	}
 out_put_cic:
 	kmem_cache_free(cic_entry_slab, cic);
 out_put_dnode:
Daeho Jeong April 23, 2024, 7:47 p.m. UTC | #4 
On Tue, Apr 16, 2024 at 9:50 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote:
>
> Let's stop issuing compressed writes and clear their writeback flags.
>
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
> ---
>
>  Now, I don't see any kernel hang for 24hours.
>
>  Change log from v1:
>   - fix bugs
>
>  fs/f2fs/compress.c | 40 ++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index d67c471ab5df..b12d3a49bfda 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1031,6 +1031,31 @@ static void set_cluster_writeback(struct compress_ctx *cc)
>         }
>  }
>
> +static void cancel_cluster_writeback(struct compress_ctx *cc,
> +                       struct compress_io_ctx *cic, int submitted)
> +{
> +       int i;
> +
> +       /* Wait for submitted IOs. */
> +       if (submitted > 1) {
> +               f2fs_submit_merged_write(F2FS_I_SB(cc->inode), DATA);
> +               while (atomic_read(&cic->pending_pages) !=
> +                                       (cc->valid_nr_cpages - submitted + 1))
> +                       f2fs_io_schedule_timeout(DEFAULT_IO_TIMEOUT);
> +       }
> +
> +       /* Cancel writeback and stay locked. */
> +       for (i = 0; i < cc->cluster_size; i++) {
> +               if (i < submitted) {
> +                       inode_inc_dirty_pages(cc->inode);
> +                       lock_page(cc->rpages[i]);
> +               }
> +               clear_page_private_gcing(cc->rpages[i]);
> +               if (folio_test_writeback(page_folio(cc->rpages[i])))
> +                       end_page_writeback(cc->rpages[i]);
> +       }
> +}
> +
>  static void set_cluster_dirty(struct compress_ctx *cc)
>  {
>         int i;
> @@ -1232,7 +1257,6 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>                 .page = NULL,
>                 .encrypted_page = NULL,
>                 .compressed_page = NULL,
> -               .submitted = 0,
>                 .io_type = io_type,
>                 .io_wbc = wbc,
>                 .encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
> @@ -1358,7 +1382,16 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>                         fio.compressed_page = cc->cpages[i - 1];
>
>                 cc->cpages[i - 1] = NULL;
> +               fio.submitted = 0;
>                 f2fs_outplace_write_data(&dn, &fio);
> +               if (unlikely(!fio.submitted)) {
> +                       cancel_cluster_writeback(cc, cic, i);
> +
> +                       /* To call fscrypt_finalize_bounce_page */
> +                       i = cc->valid_nr_cpages;
> +                       *submitted = 0;
> +                       goto out_destroy_crypt;
> +               }
>                 (*submitted)++;
>  unlock_continue:
>                 inode_dec_dirty_pages(cc->inode);
> @@ -1392,8 +1425,11 @@ static int f2fs_write_compressed_pages(struct compress_ctx *cc,
>  out_destroy_crypt:
>         page_array_free(cc->inode, cic->rpages, cc->cluster_size);
>
> -       for (--i; i >= 0; i--)
> +       for (--i; i >= 0; i--) {
> +               if (!cc->cpages[i])
> +                       continue;
>                 fscrypt_finalize_bounce_page(&cc->cpages[i]);
> +       }
>  out_put_cic:
>         kmem_cache_free(cic_entry_slab, cic);
>  out_put_dnode:
> --
> 2.44.0.683.g7961c838ac-goog
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reviewed-by: Daeho Jeong <daehojeong@google.com>

Thanks,
Chao Yu April 25, 2024, 2:15 a.m. UTC | #5 
On 2024/4/17 0:49, Jaegeuk Kim wrote:
> Let's stop issuing compressed writes and clear their writeback flags.
> 
> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,
diff mbox series

Patch

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index d67c471ab5df..3a8ecc6aee84 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1031,6 +1031,25 @@  static void set_cluster_writeback(struct compress_ctx *cc)
 	}
 }
 
+static void cancel_cluster_writeback(struct compress_ctx *cc, int submitted)
+{
+	int i;
+
+	for (i = 0; i < cc->cluster_size; i++) {
+		if (!cc->rpages[i])
+			continue;
+		if (i < submitted) {
+			if (i)
+				f2fs_wait_on_page_writeback(cc->rpages[i],
+						DATA, true, true);
+			inode_inc_dirty_pages(cc->inode);
+			lock_page(cc->rpages[i]);
+		}
+		clear_page_private_gcing(cc->rpages[i]);
+		end_page_writeback(cc->rpages[i]);
+	}
+}
+
 static void set_cluster_dirty(struct compress_ctx *cc)
 {
 	int i;
@@ -1232,7 +1251,6 @@  static int f2fs_write_compressed_pages(struct compress_ctx *cc,
 		.page = NULL,
 		.encrypted_page = NULL,
 		.compressed_page = NULL,
-		.submitted = 0,
 		.io_type = io_type,
 		.io_wbc = wbc,
 		.encrypted = fscrypt_inode_uses_fs_layer_crypto(cc->inode) ?
@@ -1358,7 +1376,15 @@  static int f2fs_write_compressed_pages(struct compress_ctx *cc,
 			fio.compressed_page = cc->cpages[i - 1];
 
 		cc->cpages[i - 1] = NULL;
+		fio.submitted = 0;
 		f2fs_outplace_write_data(&dn, &fio);
+		if (unlikely(!fio.submitted)) {
+			cancel_cluster_writeback(cc, i);
+
+			/* To call fscrypt_finalize_bounce_page */
+			i = cc->valid_nr_cpages;
+			goto out_destroy_crypt;
+		}
 		(*submitted)++;
 unlock_continue:
 		inode_dec_dirty_pages(cc->inode);
@@ -1392,8 +1418,11 @@  static int f2fs_write_compressed_pages(struct compress_ctx *cc,
 out_destroy_crypt:
 	page_array_free(cc->inode, cic->rpages, cc->cluster_size);
 
-	for (--i; i >= 0; i--)
+	for (--i; i >= 0; i--) {
+		if (!cc->cpages[i])
+			continue;
 		fscrypt_finalize_bounce_page(&cc->cpages[i]);
+	}
 out_put_cic:
 	kmem_cache_free(cic_entry_slab, cic);
 out_put_dnode: