| Message ID | 20220623175157.1715274-7-shr@fb.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io-uring/xfs: support async buffered writes | expand |
On Thu, Jun 23, 2022 at 10:51:49AM -0700, Stefan Roesch wrote: > If iomap_write_iter() encounters -EAGAIN, return -EAGAIN to the caller. > > Signed-off-by: Stefan Roesch <shr@fb.com> > --- > fs/iomap/buffered-io.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index 83cf093fcb92..f2e36240079f 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -830,7 +830,13 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i) > length -= status; > } while (iov_iter_count(i) && length); > > - return written ? written : status; > + if (status == -EAGAIN) { > + iov_iter_revert(i, written); > + return -EAGAIN; > + } > + if (written) > + return written; > + return status; Any particular reason for decomposing the ternary into this? It still looks correct, but it doesn't seem totally necessary... Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > } > > ssize_t > -- > 2.30.2 >
On 6/23/22 1:18 PM, Darrick J. Wong wrote: > On Thu, Jun 23, 2022 at 10:51:49AM -0700, Stefan Roesch wrote: >> If iomap_write_iter() encounters -EAGAIN, return -EAGAIN to the caller. >> >> Signed-off-by: Stefan Roesch <shr@fb.com> >> --- >> fs/iomap/buffered-io.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c >> index 83cf093fcb92..f2e36240079f 100644 >> --- a/fs/iomap/buffered-io.c >> +++ b/fs/iomap/buffered-io.c >> @@ -830,7 +830,13 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i) >> length -= status; >> } while (iov_iter_count(i) && length); >> >> - return written ? written : status; >> + if (status == -EAGAIN) { >> + iov_iter_revert(i, written); >> + return -EAGAIN; >> + } >> + if (written) >> + return written; >> + return status; > > Any particular reason for decomposing the ternary into this? It still > looks correct, but it doesn't seem totally necessary... > Do you prefer this version? + if (status == -EAGAIN) { + iov_iter_revert(i, written); + return -EAGAIN; + } return written ? written : status; > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > --D > >> } >> >> ssize_t >> -- >> 2.30.2 >>
On Thu, Jun 23, 2022 at 01:23:02PM -0700, Stefan Roesch wrote: > > > On 6/23/22 1:18 PM, Darrick J. Wong wrote: > > On Thu, Jun 23, 2022 at 10:51:49AM -0700, Stefan Roesch wrote: > >> If iomap_write_iter() encounters -EAGAIN, return -EAGAIN to the caller. > >> > >> Signed-off-by: Stefan Roesch <shr@fb.com> > >> --- > >> fs/iomap/buffered-io.c | 8 +++++++- > >> 1 file changed, 7 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > >> index 83cf093fcb92..f2e36240079f 100644 > >> --- a/fs/iomap/buffered-io.c > >> +++ b/fs/iomap/buffered-io.c > >> @@ -830,7 +830,13 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i) > >> length -= status; > >> } while (iov_iter_count(i) && length); > >> > >> - return written ? written : status; > >> + if (status == -EAGAIN) { > >> + iov_iter_revert(i, written); > >> + return -EAGAIN; > >> + } > >> + if (written) > >> + return written; > >> + return status; > > > > Any particular reason for decomposing the ternary into this? It still > > looks correct, but it doesn't seem totally necessary... > > > > Do you prefer this version? > > + if (status == -EAGAIN) { > + iov_iter_revert(i, written); > + return -EAGAIN; > + } > return written ? written : status; Yes, because it /does/ make it a lot more obvious that the only change is intercepting EAGAIN to rewind the iov_iter. :) --D > > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > > > --D > > > >> } > >> > >> ssize_t > >> -- > >> 2.30.2 > >>
On Thu, Jun 23, 2022 at 10:51:49AM -0700, Stefan Roesch wrote: > If iomap_write_iter() encounters -EAGAIN, return -EAGAIN to the caller. > > Signed-off-by: Stefan Roesch <shr@fb.com> Looks good: Reviewed-by: Christoph Hellwig <hch@lst.de>
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index 83cf093fcb92..f2e36240079f 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -830,7 +830,13 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i) length -= status; } while (iov_iter_count(i) && length); - return written ? written : status; + if (status == -EAGAIN) { + iov_iter_revert(i, written); + return -EAGAIN; + } + if (written) + return written; + return status; } ssize_t
If iomap_write_iter() encounters -EAGAIN, return -EAGAIN to the caller. Signed-off-by: Stefan Roesch <shr@fb.com> --- fs/iomap/buffered-io.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)