Re: [PATCH] iomap: fix short copy in iomap_write_iter()

From: Christoph Hellwig
Date: Wed Oct 18 2023 - 08:22:26 EST


On Wed, Oct 18, 2023 at 10:24:20AM +0200, Jan Stancek wrote:
> Make next iteration retry with amount of bytes we managed to copy.

The observation and logic fix look good. But I wonder if simply
using a goto instead of the extra variable would be a tad cleaner?
Something like this?

---
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 061f3d14c12001..2d491590795aa4 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -881,8 +881,10 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
size_t bytes; /* Bytes to write to folio */
size_t copied; /* Bytes copied from user */

+ bytes = iov_iter_count(i);
+retry:
offset = pos & (chunk - 1);
- bytes = min(chunk - offset, iov_iter_count(i));
+ bytes = min(chunk - offset, bytes);
status = balance_dirty_pages_ratelimited_flags(mapping,
bdp_flags);
if (unlikely(status))
@@ -933,10 +935,12 @@ static loff_t iomap_write_iter(struct iomap_iter *iter, struct iov_iter *i)
* halfway through, might be a race with munmap,
* might be severe memory pressure.
*/
- if (copied)
- bytes = copied;
if (chunk > PAGE_SIZE)
chunk /= 2;
+ if (copied) {
+ bytes = copied;
+ goto retry;
+ }
} else {
pos += status;
written += status;