[PATCH 0/4] ext4: Allow writeback IO without io_end

[PATCH 0/4] ext4: Allow writeback IO without io_end

[Jan Kara]

Hello,

this patch series allows ext4 to do page writeback without having io_end
allocated for the cases when block allocation is not required (it is the part
of my writepages improvements that didn't get merged in the first round). In
such cases io_end is not really necessary (it is really required only when
extent conversion has to happen after IO has finished) so just tweak the IO
completion code to allow IO submission without io_end allocated. It saves us
memory allocation on writeback path - it is not a big deal for performance but
it is nice in close-to-ENOMEM situations.

The patches have passed xfstests for ext4 and ext3 configurations,
specifically I have taken care to verify IO error handling still works.

								Honza

[PATCH 1/4] ext4: Move error reporting on bio completion to ext4_finish_bio()

[Jan Kara]

Currently we were reporting errors from IO completion in ext4_end_bio().
Additionally we were setting page error bit in ext4_finish_bio() and
also unnecessarily calling mapping_set_error() again. Move all the error
reporting from IO completion to ext4_finish_bio().

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

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 1a82138ba739..61e4e25b8e95 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -62,6 +62,7 @@ static void ext4_finish_bio(struct bio *bio)
 {
 	int i;
 	struct bio_vec *bvec;
+	bool error_reported = false;
 
 	bio_for_each_segment_all(bvec, bio, i) {
 		struct page *page = bvec->bv_page;
@@ -87,7 +88,19 @@ static void ext4_finish_bio(struct bio *bio)
 
 		if (bio->bi_error) {
 			SetPageError(page);
-			mapping_set_error(page->mapping, -EIO);
+			if (!error_reported) {
+				struct inode *inode = page->mapping->host;
+				sector_t bi_sector = bio->bi_iter.bi_sector;
+
+				ext4_warning(inode->i_sb,
+					"I/O error %d writing to inode %lu "
+					"(starting block %llu)",
+					bio->bi_error, inode->i_ino,
+					(unsigned long long)
+					bi_sector >> (inode->i_blkbits - 9));
+				mapping_set_error(page->mapping, bio->bi_error);
+				error_reported = true;
+			}
 		}
 		bh = head = page_buffers(page);
 		/*
@@ -296,7 +309,6 @@ ext4_io_end_t *ext4_get_io_end(ext4_io_end_t *io_end)
 static void ext4_end_bio(struct bio *bio)
 {
 	ext4_io_end_t *io_end = bio->bi_private;
-	sector_t bi_sector = bio->bi_iter.bi_sector;
 	char b[BDEVNAME_SIZE];
 
 	if (WARN_ONCE(!io_end, "io_end is NULL: %s: sector %Lu len %u err %d\n",
@@ -310,19 +322,6 @@ static void ext4_end_bio(struct bio *bio)
 	}
 	bio->bi_end_io = NULL;
 
-	if (bio->bi_error) {
-		struct inode *inode = io_end->inode;
-
-		ext4_warning(inode->i_sb, "I/O error %d writing to inode %lu "
-			     "(offset %llu size %ld starting block %llu)",
-			     bio->bi_error, inode->i_ino,
-			     (unsigned long long) io_end->offset,
-			     (long) io_end->size,
-			     (unsigned long long)
-			     bi_sector >> (inode->i_blkbits - 9));
-		mapping_set_error(inode->i_mapping, bio->bi_error);
-	}

[PATCH 2/4] ext4: Allow IO submission without io_end

[Jan Kara]

Allow submission of bio through ext4_bio_write_page() without io_end.
For the case where we are not converting unwritten extents we don't need
io_end for anything anyway.

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

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 61e4e25b8e95..34c5ec4c6572 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -309,20 +309,10 @@ ext4_io_end_t *ext4_get_io_end(ext4_io_end_t *io_end)
 static void ext4_end_bio(struct bio *bio)
 {
 	ext4_io_end_t *io_end = bio->bi_private;
-	char b[BDEVNAME_SIZE];
 
-	if (WARN_ONCE(!io_end, "io_end is NULL: %s: sector %Lu len %u err %d\n",
-		      bdevname(bio->bi_bdev, b),
-		      (long long) bio->bi_iter.bi_sector,
-		      (unsigned) bio_sectors(bio),
-		      bio->bi_error)) {
-		ext4_finish_bio(bio);
-		bio_put(bio);
-		return;
-	}
 	bio->bi_end_io = NULL;
 
-	if (io_end->flag & EXT4_IO_END_UNWRITTEN) {
+	if (io_end && io_end->flag & EXT4_IO_END_UNWRITTEN) {
 		/*
 		 * Link bio into list hanging from io_end. We have to do it
 		 * atomically as bio completions can be racing against each
@@ -330,15 +320,17 @@ static void ext4_end_bio(struct bio *bio)
 		 */
 		bio->bi_private = xchg(&io_end->bio, bio);
 		ext4_put_io_end_defer(io_end);
-	} else {
+		return;
+	}
+	if (io_end) {
 		/*
 		 * Drop io_end reference early. Inode can get freed once
 		 * we finish the bio.
 		 */
 		ext4_put_io_end_defer(io_end);
-		ext4_finish_bio(bio);
-		bio_put(bio);
 	}
+	ext4_finish_bio(bio);
+	bio_put(bio);
 }
 
 void ext4_io_submit(struct ext4_io_submit *io)
@@ -374,7 +366,8 @@ static int io_submit_init_bio(struct ext4_io_submit *io,
 	bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9);
 	bio->bi_bdev = bh->b_bdev;
 	bio->bi_end_io = ext4_end_bio;
-	bio->bi_private = ext4_get_io_end(io->io_end);
+	if (io->io_end)
+		bio->bi_private = ext4_get_io_end(io->io_end);
 	io->io_bio = bio;
 	io->io_next_block = bh->b_blocknr;
 	return 0;
-- 
2.12.0

[PATCH 3/4] ext4: Don't allocate io_end for writeback from ext4_writepage()

[Jan Kara]

ext4_writepage() writes out only mapped buffers with allocated
underlying blocks. Thus there's no need for io_end structure
and we can avoid allocating it.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/inode.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 5834c4d76be8..6bce4520d569 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -2108,16 +2108,8 @@ static int ext4_writepage(struct page *page,
 		return __ext4_journalled_writepage(page, len);
 
 	ext4_io_submit_init(&io_submit, wbc);
-	io_submit.io_end = ext4_init_io_end(inode, GFP_NOFS);
-	if (!io_submit.io_end) {
-		redirty_page_for_writepage(wbc, page);
-		unlock_page(page);
-		return -ENOMEM;
-	}
 	ret = ext4_bio_write_page(&io_submit, page, len, wbc, keep_towrite);
 	ext4_io_submit(&io_submit);
-	/* Drop io_end reference we got from init */
-	ext4_put_io_end_defer(io_submit.io_end);
 	return ret;
 }
 
-- 
2.12.0

[PATCH 4/4] ext4: Remove unnecessary io_end allocation from ext4_writepages()

[Jan Kara]

We don't have to allocate io_end when doing writeback from
ext4_writepages() without doing allocation. Remove the unnecessary
allocation. Also do_map argument is now unnecessary since we have io_end
allocated only if we want blocks to be allocated so remove it.

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

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 6bce4520d569..83cddd9d7fb2 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -1643,7 +1643,6 @@ struct mpage_da_data {
 	 */
 	struct ext4_map_blocks map;
 	struct ext4_io_submit io_submit;	/* IO submission data */
-	unsigned int do_map:1;
 };
 
 static void mpage_release_unused_pages(struct mpage_da_data *mpd,
@@ -2173,7 +2172,7 @@ static bool mpage_add_bh_to_extent(struct mpage_da_data *mpd, ext4_lblk_t lblk,
 	/* First block in the extent? */
 	if (map->m_len == 0) {
 		/* We cannot map unless handle is started... */
-		if (!mpd->do_map)
+		if (!mpd->io_submit.io_end)
 			return false;
 		map->m_lblk = lblk;
 		map->m_len = 1;
@@ -2228,7 +2227,7 @@ static int mpage_process_page_bufs(struct mpage_da_data *mpd,
 			if (mpd->map.m_len)
 				return 0;
 			/* Buffer needs mapping and handle is not started? */
-			if (!mpd->do_map)
+			if (!mpd->io_submit.io_end)
 				return 0;
 			/* Everything mapped so far and we hit EOF */
 			break;
@@ -2753,17 +2752,9 @@ static int ext4_writepages(struct address_space *mapping,
 	 * in the block layer on device congestion while having transaction
 	 * started.
 	 */
-	mpd.do_map = 0;
-	mpd.io_submit.io_end = ext4_init_io_end(inode, GFP_KERNEL);
-	if (!mpd.io_submit.io_end) {
-		ret = -ENOMEM;
-		goto unplug;
-	}
 	ret = mpage_prepare_extent_to_map(&mpd);
 	/* Submit prepared bio */
 	ext4_io_submit(&mpd.io_submit);
-	ext4_put_io_end_defer(mpd.io_submit.io_end);
-	mpd.io_submit.io_end = NULL;
 	/* Unlock pages we didn't use */
 	mpage_release_unused_pages(&mpd, false);
 	if (ret < 0)
@@ -2800,7 +2791,6 @@ static int ext4_writepages(struct address_space *mapping,
 			mpd.io_submit.io_end = NULL;
 			break;
 		}
-		mpd.do_map = 1;
 
 		trace_ext4_da_write_pages(inode, mpd.first_page, mpd.wbc);
 		ret = mpage_prepare_extent_to_map(&mpd);
@@ -2831,7 +2821,6 @@ static int ext4_writepages(struct address_space *mapping,
 		if (!ext4_handle_valid(handle) || handle->h_sync == 0) {
 			ext4_journal_stop(handle);
 			handle = NULL;
-			mpd.do_map = 0;
 		}
 		/* Submit prepared bio */
 		ext4_io_submit(&mpd.io_submit);
-- 
2.12.0