| Message ID | 20250313171310.1886394-3-john.g.garry@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | large atomic writes for xfs with CoW | expand |
On Thu, Mar 13, 2025 at 05:12:59PM +0000, John Garry wrote: > if (iter->flags & IOMAP_ATOMIC_HW) { > + /* > + * Ensure that the mapping covers the full write length, > + * otherwise we will submit multiple BIOs, which is > + * disallowed. > + */ "disallowed" doesn't really explain anything, why is it disallowed? Maybe: * Ensure that the mapping covers the full write length, * otherwise it can't be submitted as a single bio, * which is required to use hardware atomics.
On 17/03/2025 06:08, Christoph Hellwig wrote: > On Thu, Mar 13, 2025 at 05:12:59PM +0000, John Garry wrote: >> if (iter->flags & IOMAP_ATOMIC_HW) { >> + /* >> + * Ensure that the mapping covers the full write length, >> + * otherwise we will submit multiple BIOs, which is >> + * disallowed. >> + */ > > "disallowed" doesn't really explain anything, why is it disallowed? > > Maybe: > > * Ensure that the mapping covers the full write length, > * otherwise it can't be submitted as a single bio, > * which is required to use hardware atomics. ok, fine. >
John Garry <john.g.garry@oracle.com> writes: > Help explain the code. > > Also clarify the comment for bio size check. > > Signed-off-by: John Garry <john.g.garry@oracle.com> > --- > fs/iomap/direct-io.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c > index 8c1bec473586..9d72b99cb447 100644 > --- a/fs/iomap/direct-io.c > +++ b/fs/iomap/direct-io.c > @@ -350,6 +350,11 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio) > bio_opf |= REQ_OP_WRITE; > > if (iter->flags & IOMAP_ATOMIC_HW) { > + /* > + * Ensure that the mapping covers the full write length, > + * otherwise we will submit multiple BIOs, which is > + * disallowed. > + */ Nit: IMO, this can be clubbed together with your next patch PATCH-03 itself. But either ways, no strong preference. -ritesh > if (length != iter->len) > return -EINVAL; > bio_opf |= REQ_ATOMIC; > @@ -449,7 +454,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio) > n = bio->bi_iter.bi_size; > if (WARN_ON_ONCE((bio_opf & REQ_ATOMIC) && n != length)) { > /* > - * This bio should have covered the complete length, > + * An atomic write bio must cover the complete length, > * which it doesn't, so error. We may need to zero out > * the tail (complete FS block), similar to when > * bio_iov_iter_get_pages() returns an error, above. > -- > 2.31.1
diff --git a/fs/iomap/direct-io.c b/fs/iomap/direct-io.c index 8c1bec473586..9d72b99cb447 100644 --- a/fs/iomap/direct-io.c +++ b/fs/iomap/direct-io.c @@ -350,6 +350,11 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio) bio_opf |= REQ_OP_WRITE; if (iter->flags & IOMAP_ATOMIC_HW) { + /* + * Ensure that the mapping covers the full write length, + * otherwise we will submit multiple BIOs, which is + * disallowed. + */ if (length != iter->len) return -EINVAL; bio_opf |= REQ_ATOMIC; @@ -449,7 +454,7 @@ static int iomap_dio_bio_iter(struct iomap_iter *iter, struct iomap_dio *dio) n = bio->bi_iter.bi_size; if (WARN_ON_ONCE((bio_opf & REQ_ATOMIC) && n != length)) { /* - * This bio should have covered the complete length, + * An atomic write bio must cover the complete length, * which it doesn't, so error. We may need to zero out * the tail (complete FS block), similar to when * bio_iov_iter_get_pages() returns an error, above.
Help explain the code. Also clarify the comment for bio size check. Signed-off-by: John Garry <john.g.garry@oracle.com> --- fs/iomap/direct-io.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)