| Message ID | 20230519093521.133226-8-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/13] iomap: update ki_pos a little later in iomap_dio_complete | expand |
On 5/19/23 18:35, Christoph Hellwig wrote: > All callers of iomap_file_buffered_write need to updated ki_pos, move it > into common code. > > Signed-off-by: Christoph Hellwig <hch@lst.de> One nit below. Acked-by: Damien Le Moal <dlemoal@kernel.org> > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 063133ec77f49e..550525a525c45c 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -864,16 +864,19 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i, > .len = iov_iter_count(i), > .flags = IOMAP_WRITE, > }; > - int ret; > + ssize_t ret; > > if (iocb->ki_flags & IOCB_NOWAIT) > iter.flags |= IOMAP_NOWAIT; > > while ((ret = iomap_iter(&iter, ops)) > 0) > iter.processed = iomap_write_iter(&iter, i); > - if (iter.pos == iocb->ki_pos) > + > + if (unlikely(ret < 0)) Nit: This could be if (unlikely(ret <= 0)), no ? > return ret; > - return iter.pos - iocb->ki_pos; > + ret = iter.pos - iocb->ki_pos; > + iocb->ki_pos += ret; > + return ret;
On Mon, May 22, 2023 at 09:01:05AM +0900, Damien Le Moal wrote: > > - int ret; > > + ssize_t ret; > > > > if (iocb->ki_flags & IOCB_NOWAIT) > > iter.flags |= IOMAP_NOWAIT; > > > > while ((ret = iomap_iter(&iter, ops)) > 0) > > iter.processed = iomap_write_iter(&iter, i); > > - if (iter.pos == iocb->ki_pos) > > + > > + if (unlikely(ret < 0)) > > Nit: This could be if (unlikely(ret <= 0)), no ? No. iomap_iter does not return te amount of bytes written.
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c index 300844f50dcd28..499ef174dec138 100644 --- a/fs/gfs2/file.c +++ b/fs/gfs2/file.c @@ -1046,10 +1046,8 @@ static ssize_t gfs2_file_buffered_write(struct kiocb *iocb, ret = iomap_file_buffered_write(iocb, from, &gfs2_iomap_ops); pagefault_enable(); current->backing_dev_info = NULL; - if (ret > 0) { - iocb->ki_pos += ret; + if (ret > 0) written += ret; - } if (inode == sdp->sd_rindex) gfs2_glock_dq_uninit(statfs_gh); diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 063133ec77f49e..550525a525c45c 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -864,16 +864,19 @@ iomap_file_buffered_write(struct kiocb *iocb, struct iov_iter *i, .len = iov_iter_count(i), .flags = IOMAP_WRITE, }; - int ret; + ssize_t ret; if (iocb->ki_flags & IOCB_NOWAIT) iter.flags |= IOMAP_NOWAIT; while ((ret = iomap_iter(&iter, ops)) > 0) iter.processed = iomap_write_iter(&iter, i); - if (iter.pos == iocb->ki_pos) + + if (unlikely(ret < 0)) return ret; - return iter.pos - iocb->ki_pos; + ret = iter.pos - iocb->ki_pos; + iocb->ki_pos += ret; + return ret; } EXPORT_SYMBOL_GPL(iomap_file_buffered_write); diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index aede746541f8ae..bfba10e0b0f3c2 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -723,8 +723,6 @@ xfs_file_buffered_write( trace_xfs_file_buffered_write(iocb, from); ret = iomap_file_buffered_write(iocb, from, &xfs_buffered_write_iomap_ops); - if (likely(ret >= 0)) - iocb->ki_pos += ret; /* * If we hit a space limit, try to free up some lingering preallocated diff --git a/fs/zonefs/file.c b/fs/zonefs/file.c index 132f01d3461f14..e212d0636f848e 100644 --- a/fs/zonefs/file.c +++ b/fs/zonefs/file.c @@ -643,9 +643,7 @@ static ssize_t zonefs_file_buffered_write(struct kiocb *iocb, goto inode_unlock; ret = iomap_file_buffered_write(iocb, from, &zonefs_write_iomap_ops); - if (ret > 0) - iocb->ki_pos += ret; - else if (ret == -EIO) + if (ret == -EIO) zonefs_io_error(inode, true); inode_unlock:
All callers of iomap_file_buffered_write need to updated ki_pos, move it into common code. Signed-off-by: Christoph Hellwig <hch@lst.de> --- fs/gfs2/file.c | 4 +--- fs/iomap/buffered-io.c | 9 ++++++--- fs/xfs/xfs_file.c | 2 -- fs/zonefs/file.c | 4 +--- 4 files changed, 8 insertions(+), 11 deletions(-)