From: Goldwyn Rodrigues <rgoldwyn@suse.com> In case of a block device error, iomap code returns 0 as opposed to the amount of submitted I/O, which may have completed before the error occurred. Return the count of submitted I/O for correct accounting. Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com> --- fs/iomap/direct-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 23837926c0c5..a980b7b7660f 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -260,7 +260,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.24.1
On Thu, Feb 13, 2020 at 01:25:03PM -0600, Goldwyn Rodrigues wrote:
> From: Goldwyn Rodrigues <rgoldwyn@suse.com>
>
> In case of a block device error, iomap code returns 0 as opposed to
> the amount of submitted I/O, which may have completed before the
> error occurred. Return the count of submitted I/O for correct
> accounting.
Haven't we traditionally failed direct I/O syscalls that don't fully
complete and never supported short writes (or reads)?
On 5:17 17/02, Christoph Hellwig wrote:
> On Thu, Feb 13, 2020 at 01:25:03PM -0600, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues <rgoldwyn@suse.com>
> >
> > In case of a block device error, iomap code returns 0 as opposed to
> > the amount of submitted I/O, which may have completed before the
> > error occurred. Return the count of submitted I/O for correct
> > accounting.
>
> Haven't we traditionally failed direct I/O syscalls that don't fully
> complete and never supported short writes (or reads)?
Yes, but I think that decision should be with the filesystem what to do
with it and not the iomap layer.
The reason we need this patch for btrfs is that we need to account for
updating the allocations. iomap_end() returns written as zero while
iomap_dio_rw loop has submitted part of the I/O. So btrfs has no idea
as to how much has been submitted before the failure and how much of
the allocation to update.
This was exhibited by generic/250 in some of the runs where it fails the
underlying storage.
--
Goldwyn
On Mon, Feb 17, 2020 at 07:44:17AM -0600, Goldwyn Rodrigues wrote:
> > Haven't we traditionally failed direct I/O syscalls that don't fully
> > complete and never supported short writes (or reads)?
>
> Yes, but I think that decision should be with the filesystem what to do
> with it and not the iomap layer.
But then you also need to fix up the existing callers to do the
conversion.
On 6:02 17/02, Christoph Hellwig wrote:
> On Mon, Feb 17, 2020 at 07:44:17AM -0600, Goldwyn Rodrigues wrote:
> > > Haven't we traditionally failed direct I/O syscalls that don't fully
> > > complete and never supported short writes (or reads)?
> >
> > Yes, but I think that decision should be with the filesystem what to do
> > with it and not the iomap layer.
>
> But then you also need to fix up the existing callers to do the
> conversion.
The error returned is set in iomap_dio_complete() which happens after.
I checked all instances and the the only place which uses
written in direct I/O is ext4. I will put in the change.
Thanks!
--
Goldwyn