diff mbox series

[1/9] ext4: Handle redirtying in ext4_bio_write_page()

Message ID 20221130163608.29034-1-jack@suse.cz
State Superseded
Headers show
Series ext4: Stop using ext4_writepage() for writeout of ordered data | expand

Commit Message

Jan Kara Nov. 30, 2022, 4:35 p.m. UTC
Since we want to transition transaction commits to use ext4_writepages()
for writing back ordered, add handling of page redirtying into
ext4_bio_write_page(). Also move buffer dirty bit clearing into the same
place other buffer state handling.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/page-io.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Ritesh Harjani (IBM) Dec. 1, 2022, 8:57 a.m. UTC | #1
On 22/11/30 05:35PM, Jan Kara wrote:
> Since we want to transition transaction commits to use ext4_writepages()
> for writing back ordered, add handling of page redirtying into
> ext4_bio_write_page(). Also move buffer dirty bit clearing into the same
> place other buffer state handling.
>

So when we will move away from ext4_writepage() and will instead call
ext4_writepages() (for transaction commits requiring ordered write handling),
this patch should help with redirtying the page, if any of the page buffer
cannot be written back which is when either the buffer is either marked delayed
(which will require block allocation) or is unwritten (which may also
require some block allocation during unwritten to written conversion).

Also, one other good thing about this patch is, we don't have to loop over all
the buffers seperately to identify whether a page needs to be set redirty or not.
With this change we will redirty the page in the same loop (if required)
and identify all the mapped buffers for writeback.


Moving the clearing of buffer dirty state also looks right to me.

Please feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>


> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ext4/page-io.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 97fa7b4c645f..4e68ace86f11 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -482,6 +482,13 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
>  			/* A hole? We can safely clear the dirty bit */
>  			if (!buffer_mapped(bh))
>  				clear_buffer_dirty(bh);
> +			/*
> +			 * Keeping dirty some buffer we cannot write? Make
> +			 * sure to redirty the page. This happens e.g. when
> +			 * doing writeout for transaction commit.
> +			 */
> +			if (buffer_dirty(bh) && !PageDirty(page))
> +				redirty_page_for_writepage(wbc, page);
>  			if (io->io_bio)
>  				ext4_io_submit(io);
>  			continue;
> @@ -489,6 +496,7 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
>  		if (buffer_new(bh))
>  			clear_buffer_new(bh);
>  		set_buffer_async_write(bh);
> +		clear_buffer_dirty(bh);
>  		nr_to_submit++;
>  	} while ((bh = bh->b_this_page) != head);
>
> @@ -532,7 +540,10 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
>  			printk_ratelimited(KERN_ERR "%s: ret = %d\n", __func__, ret);
>  			redirty_page_for_writepage(wbc, page);
>  			do {
> -				clear_buffer_async_write(bh);
> +				if (buffer_async_write(bh)) {
> +					clear_buffer_async_write(bh);
> +					set_buffer_dirty(bh);
> +				}
>  				bh = bh->b_this_page;
>  			} while (bh != head);
>  			goto unlock;
> @@ -546,7 +557,6 @@ int ext4_bio_write_page(struct ext4_io_submit *io,
>  		io_submit_add_bh(io, inode,
>  				 bounce_page ? bounce_page : page, bh);
>  		nr_submitted++;
> -		clear_buffer_dirty(bh);
>  	} while ((bh = bh->b_this_page) != head);
>
>  unlock:
> --
> 2.35.3
>
diff mbox series

Patch

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 97fa7b4c645f..4e68ace86f11 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -482,6 +482,13 @@  int ext4_bio_write_page(struct ext4_io_submit *io,
 			/* A hole? We can safely clear the dirty bit */
 			if (!buffer_mapped(bh))
 				clear_buffer_dirty(bh);
+			/*
+			 * Keeping dirty some buffer we cannot write? Make
+			 * sure to redirty the page. This happens e.g. when
+			 * doing writeout for transaction commit.
+			 */
+			if (buffer_dirty(bh) && !PageDirty(page))
+				redirty_page_for_writepage(wbc, page);
 			if (io->io_bio)
 				ext4_io_submit(io);
 			continue;
@@ -489,6 +496,7 @@  int ext4_bio_write_page(struct ext4_io_submit *io,
 		if (buffer_new(bh))
 			clear_buffer_new(bh);
 		set_buffer_async_write(bh);
+		clear_buffer_dirty(bh);
 		nr_to_submit++;
 	} while ((bh = bh->b_this_page) != head);
 
@@ -532,7 +540,10 @@  int ext4_bio_write_page(struct ext4_io_submit *io,
 			printk_ratelimited(KERN_ERR "%s: ret = %d\n", __func__, ret);
 			redirty_page_for_writepage(wbc, page);
 			do {
-				clear_buffer_async_write(bh);
+				if (buffer_async_write(bh)) {
+					clear_buffer_async_write(bh);
+					set_buffer_dirty(bh);
+				}
 				bh = bh->b_this_page;
 			} while (bh != head);
 			goto unlock;
@@ -546,7 +557,6 @@  int ext4_bio_write_page(struct ext4_io_submit *io,
 		io_submit_add_bh(io, inode,
 				 bounce_page ? bounce_page : page, bh);
 		nr_submitted++;
-		clear_buffer_dirty(bh);
 	} while ((bh = bh->b_this_page) != head);
 
 unlock: