linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] iomap: return partial I/O count on error in iomap_dio_bio_actor
@ 2020-02-20 15:23 Goldwyn Rodrigues
  2020-02-20 17:42 ` Matthew Wilcox
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Goldwyn Rodrigues @ 2020-02-20 15:23 UTC (permalink / raw)
  To: linux-ext4; +Cc: linux-fsdevel, hch, darrick.wong

In case of a block device error, written parameter in iomap_end()
is zero as opposed to the amount of submitted I/O.
Filesystems such as btrfs need to account for the I/O in ordered
extents, even if it resulted in an error. Having (incomplete)
submitted bytes in written gives the filesystem the amount of data
which has been submitted before the error occurred, and the
filesystem code can choose how to use it.

The final returned error for iomap_dio_rw() is set by
iomap_dio_complete().

Partial writes in direct I/O are considered an error. So,
->iomap_end() using written == 0 as error must be changed
to written < length. In this case, ext4 is the only user.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/ext4/inode.c      | 2 +-
 fs/iomap/direct-io.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index e60aca791d3f..e50e7414351a 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3475,7 +3475,7 @@ static int ext4_iomap_end(struct inode *inode, loff_t offset, loff_t length,
 	 * the I/O. Any blocks that may have been allocated in preparation for
 	 * the direct I/O will be reused during buffered I/O.
 	 */
-	if (flags & (IOMAP_WRITE | IOMAP_DIRECT) && written == 0)
+	if (flags & (IOMAP_WRITE | IOMAP_DIRECT) && written < length)
 		return -ENOTBLK;
 
 	return 0;
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 41c1e7c20a1f..01865db1bd09 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -264,7 +264,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
 		size_t n;
 		if (dio->error) {
 			iov_iter_revert(dio->submit.iter, copied);
-			copied = ret = 0;
+			ret = 0;
 			goto out;
 		}
 
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH v2] iomap: return partial I/O count on error in iomap_dio_bio_actor
@ 2020-02-20 15:24 Goldwyn Rodrigues
  0 siblings, 0 replies; 19+ messages in thread
From: Goldwyn Rodrigues @ 2020-02-20 15:24 UTC (permalink / raw)
  To: linux-ext4; +Cc: linux-fsdevel, hch, darrick.wong

In case of a block device error, written parameter in iomap_end()
is zero as opposed to the amount of submitted I/O.
Filesystems such as btrfs need to account for the I/O in ordered
extents, even if it resulted in an error. Having (incomplete)
submitted bytes in written gives the filesystem the amount of data
which has been submitted before the error occurred, and the
filesystem code can choose how to use it.

The final returned error for iomap_dio_rw() is set by
iomap_dio_complete().

Partial writes in direct I/O are considered an error. So,
->iomap_end() using written == 0 as error must be changed
to written < length. In this case, ext4 is the only user.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
---
 fs/ext4/inode.c      | 2 +-
 fs/iomap/direct-io.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index e60aca791d3f..e50e7414351a 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3475,7 +3475,7 @@ static int ext4_iomap_end(struct inode *inode, loff_t offset, loff_t length,
 	 * the I/O. Any blocks that may have been allocated in preparation for
 	 * the direct I/O will be reused during buffered I/O.
 	 */
-	if (flags & (IOMAP_WRITE | IOMAP_DIRECT) && written == 0)
+	if (flags & (IOMAP_WRITE | IOMAP_DIRECT) && written < length)
 		return -ENOTBLK;
 
 	return 0;
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 41c1e7c20a1f..01865db1bd09 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -264,7 +264,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
 		size_t n;
 		if (dio->error) {
 			iov_iter_revert(dio->submit.iter, copied);
-			copied = ret = 0;
+			ret = 0;
 			goto out;
 		}
 
-- 
2.25.0


-- 
Goldwyn

^ permalink raw reply related	[flat|nested] 19+ messages in thread
* [PATCH v2] iomap: return partial I/O count on error in iomap_dio_bio_actor
@ 2020-03-19 15:08 Goldwyn Rodrigues
  2020-03-20 14:05 ` Christoph Hellwig
  0 siblings, 1 reply; 19+ messages in thread
From: Goldwyn Rodrigues @ 2020-03-19 15:08 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: riteshh, linux-ext4, hch, darrick.wong, willy

Currently, I/Os that complete with an error indicate this by passing
written == 0 to the iomap_end function.  However, btrfs needs to know how
many bytes were written for its own accounting.  Change the convention
to pass the number of bytes which were actually written, and change the
only user (ext4) to check for a short write instead of a zero length
write.

For filesystems that do not define ->iomap_end(), check for
dio->error again after the iomap_apply() call to diagnose the error.

Changes since v1:
 - Considerate of iov_iter rollback functions
 - Double check errors for filesystems not implementing iomap_end()

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index fa0ff78..d52c70f 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3475,7 +3475,7 @@ static int ext4_iomap_end(struct inode *inode, loff_t offset, loff_t length,
 	 * the I/O. Any blocks that may have been allocated in preparation for
 	 * the direct I/O will be reused during buffered I/O.
 	 */
-	if (flags & (IOMAP_WRITE | IOMAP_DIRECT) && written == 0)
+	if (flags & (IOMAP_WRITE | IOMAP_DIRECT) && written < length)
 		return -ENOTBLK;
 
 	return 0;
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c
index 41c1e7c..b5f4d4a 100644
--- a/fs/iomap/direct-io.c
+++ b/fs/iomap/direct-io.c
@@ -264,7 +264,7 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
 		size_t n;
 		if (dio->error) {
 			iov_iter_revert(dio->submit.iter, copied);
-			copied = ret = 0;
+			ret = dio->error;
 			goto out;
 		}
 
@@ -325,8 +325,17 @@ iomap_dio_bio_actor(struct inode *inode, loff_t pos, loff_t length,
 			iomap_dio_zero(dio, iomap, pos, fs_block_size - pad);
 	}
 out:
-	/* Undo iter limitation to current extent */
-	iov_iter_reexpand(dio->submit.iter, orig_count - copied);
+	/*
+	 * Undo iter limitation to current extent
+	 * If there is an error, undo the entire extent. However, return the
+	 * bytes copied so far for filesystems such as btrfs to account for
+	 * submitted I/O.
+	 */
+	if (ret < 0)
+		iov_iter_reexpand(dio->submit.iter, orig_count);
+	else
+		iov_iter_reexpand(dio->submit.iter, orig_count - copied);
+
 	if (copied)
 		return copied;
 	return ret;
@@ -499,6 +508,10 @@ iomap_dio_rw(struct kiocb *iocb, struct iov_iter *iter,
 	do {
 		ret = iomap_apply(inode, pos, count, flags, ops, dio,
 				iomap_dio_actor);
+
+		if (ret >= 0 && dio->error)
+			ret = dio->error;
+
 		if (ret <= 0) {
 			/* magic error code to fall back to buffered I/O */
 			if (ret == -ENOTBLK) {

-- 
Goldwyn

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2020-03-20 15:35 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-20 15:23 [PATCH v2] iomap: return partial I/O count on error in iomap_dio_bio_actor Goldwyn Rodrigues
2020-02-20 17:42 ` Matthew Wilcox
2020-02-21  2:06   ` Goldwyn Rodrigues
2020-02-21  4:51 ` Ritesh Harjani
2020-02-21 12:48   ` Goldwyn Rodrigues
2020-02-25 20:53   ` Christoph Hellwig
2020-02-26  2:12     ` Damien Le Moal
2020-02-26  2:55     ` Goldwyn Rodrigues
2020-02-28 19:44     ` Goldwyn Rodrigues
2020-02-28 19:59       ` Matthew Wilcox
2020-02-28 20:35         ` Goldwyn Rodrigues
2020-03-02 13:31           ` Christoph Hellwig
2020-02-21 13:14 ` David Sterba
2020-02-20 15:24 Goldwyn Rodrigues
2020-03-19 15:08 Goldwyn Rodrigues
2020-03-20 14:05 ` Christoph Hellwig
2020-03-20 14:23   ` Josef Bacik
2020-03-20 14:35     ` Christoph Hellwig
2020-03-20 15:35       ` Goldwyn Rodrigues

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).