| Message ID | 20230714085124.548920-1-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] iomap: fix a regression for partial write errors | expand |
On Fri, Jul 14, 2023 at 10:51:23AM +0200, Christoph Hellwig wrote: > When write* wrote some data it should return the amount of written data > and not the error code that caused it to stop. Fix a recent regression > in iomap_file_buffered_write that caused it to return the errno instead. > > Fixes: 219580eea1ee ("iomap: update ki_pos in iomap_file_buffered_write") > Reported-by: kernel test robot <oliver.sang@intel.com> > Reported-by: Cyril Hrubis <chrubis@suse.cz> > Signed-off-by: Christoph Hellwig <hch@lst.de> Simple enough reversion, sorry for the breakage... Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > --- > fs/iomap/buffered-io.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index adb92cdb24b009..7cc9f7274883a5 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -872,7 +872,7 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i, > while ((ret = iomap_iter(&iter, ops)) > 0) > iter.processed = iomap_write_iter(&iter, i); > > - if (unlikely(ret < 0)) > + if (unlikely(iter.pos == iocb->ki_pos)) > return ret; > ret = iter.pos - iocb->ki_pos; > iocb->ki_pos += ret; > -- > 2.39.2 >
Christoph Hellwig <hch@lst.de> writes: > When write* wrote some data it should return the amount of written data > and not the error code that caused it to stop. Fix a recent regression > in iomap_file_buffered_write that caused it to return the errno instead. > Agreed. Reviewed the change and it looks right to me. Feel free to add - Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > Fixes: 219580eea1ee ("iomap: update ki_pos in iomap_file_buffered_write") > Reported-by: kernel test robot <oliver.sang@intel.com> > Reported-by: Cyril Hrubis <chrubis@suse.cz> > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- In case anyone wanted to see before and after test failures output of ./runltp -f syscalls -s writev07 -d /mnt1/test <without this patch> tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s writev07.c:50: TINFO: starting test with initial file offset: 0 writev07.c:73: TINFO: got EFAULT writev07.c:78: TFAIL: file was written to writev07.c:84: TPASS: offset stayed unchanged writev07.c:50: TINFO: starting test with initial file offset: 65 writev07.c:73: TINFO: got EFAULT writev07.c:78: TFAIL: file was written to writev07.c:84: TPASS: offset stayed unchanged writev07.c:50: TINFO: starting test with initial file offset: 4096 writev07.c:73: TINFO: got EFAULT writev07.c:80: TPASS: file stayed untouched writev07.c:84: TPASS: offset stayed unchanged writev07.c:50: TINFO: starting test with initial file offset: 4097 writev07.c:73: TINFO: got EFAULT writev07.c:80: TPASS: file stayed untouched writev07.c:84: TPASS: offset stayed unchanged Summary: passed 6 failed 2 broken 0 skipped 0 warnings 0 <with this patch> tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s writev07.c:50: TINFO: starting test with initial file offset: 0 writev07.c:94: TINFO: writev() has written 64 bytes writev07.c:105: TPASS: file has expected content writev07.c:116: TPASS: offset at 64 as expected writev07.c:50: TINFO: starting test with initial file offset: 65 writev07.c:94: TINFO: writev() has written 64 bytes writev07.c:105: TPASS: file has expected content writev07.c:116: TPASS: offset at 129 as expected writev07.c:50: TINFO: starting test with initial file offset: 4096 writev07.c:94: TINFO: writev() has written 64 bytes writev07.c:105: TPASS: file has expected content writev07.c:116: TPASS: offset at 4160 as expected writev07.c:50: TINFO: starting test with initial file offset: 4097 writev07.c:94: TINFO: writev() has written 64 bytes writev07.c:105: TPASS: file has expected content writev07.c:116: TPASS: offset at 4161 as expected Summary: passed 8 failed 0 broken 0 skipped 0 warnings 0 > fs/iomap/buffered-io.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index adb92cdb24b009..7cc9f7274883a5 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -872,7 +872,7 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i, > while ((ret = iomap_iter(&iter, ops)) > 0) > iter.processed = iomap_write_iter(&iter, i); > > - if (unlikely(ret < 0)) > + if (unlikely(iter.pos == iocb->ki_pos)) > return ret; > ret = iter.pos - iocb->ki_pos; > iocb->ki_pos += ret; > -- > 2.39.2 -ritesh
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index adb92cdb24b009..7cc9f7274883a5 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -872,7 +872,7 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i, while ((ret = iomap_iter(&iter, ops)) > 0) iter.processed = iomap_write_iter(&iter, i); - if (unlikely(ret < 0)) + if (unlikely(iter.pos == iocb->ki_pos)) return ret; ret = iter.pos - iocb->ki_pos; iocb->ki_pos += ret;
When write* wrote some data it should return the amount of written data and not the error code that caused it to stop. Fix a recent regression in iomap_file_buffered_write that caused it to return the errno instead. Fixes: 219580eea1ee ("iomap: update ki_pos in iomap_file_buffered_write") Reported-by: kernel test robot <oliver.sang@intel.com> Reported-by: Cyril Hrubis <chrubis@suse.cz> Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/iomap/buffered-io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)