[1/3] iov_iter: add helper to save iov_iter state

Commit Message

Jens Axboe Sept. 10, 2021, 6:25 p.m. UTC

In an ideal world, when someone is passed an iov_iter and returns X bytes,
then X bytes would have been consumed/advanced from the iov_iter. But we
have use cases that always consume the entire iterator, a few examples
of that are iomap and bdev O_DIRECT. This means we cannot rely on the
state of the iov_iter once we've called ->read_iter() or ->write_iter().

This would be easier if we didn't always have to deal with truncate of
the iov_iter, as rewinding would be trivial without that. We recently
added a commit to track the truncate state, but that grew the iov_iter
by 8 bytes and wasn't the best solution.

Implement a helper to save enough of the iov_iter state to sanely restore
it after we've called the read/write iterator helpers. This currently
only works for IOVEC/BVEC/KVEC as that's all we need, support for other
iterator types are left as an exercise for the reader.

Link: https://lore.kernel.org/linux-fsdevel/CAHk-=[e-mail]/
Signed-off-by: Jens Axboe <[e-mail]>
---
 include/linux/uio.h | 16 ++++++++++++++++
 lib/iov_iter.c      | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

Comments

Al Viro Sept. 10, 2021, 6:50 p.m. UTC | #1

On Fri, Sep 10, 2021 at 12:25:34PM -0600, Jens Axboe wrote:
> +void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state,
> +		      ssize_t did_bytes)
> +{
> +	if (WARN_ON_ONCE(!iov_iter_is_bvec(i) && !iter_is_iovec(i)) &&
> +			 !iov_iter_is_kvec(i))
> +		return;
> +	i->iov_offset = state->iov_offset;
> +	i->count = state->count;
> +	/*
> +	 * For the *vec iters, nr_segs + iov is constant - if we increment
> +	 * the vec, then we also decrement the nr_segs count. Hence we don't
> +	 * need to track both of these, just one is enough and we can deduct
> +	 * the other from that. ITER_{BVEC,IOVEC,KVEC} all have their pointers
> +	 * unionized, so we don't need to handle them individually.
> +	 */
> +	i->iov -= state->nr_segs - i->nr_segs;
> +	i->nr_segs = state->nr_segs;
> +}

No.  You can have iovec and kvec handled jointly (struct iovec and struct kvec
always have the same size).  You can *not* do that to bvec - check sizeof of
struct bio_vec and struct iovec on 32bit targets.

Patch

diff --git a/include/linux/uio.h b/include/linux/uio.h
index 5265024e8b90..6eaedae5ea2f 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -27,6 +27,12 @@  enum iter_type {
 	ITER_DISCARD,
 };
 
+struct iov_iter_state {
+	size_t iov_offset;
+	size_t count;
+	unsigned long nr_segs;
+};
+
 struct iov_iter {
 	u8 iter_type;
 	bool data_source;
@@ -55,6 +61,14 @@  static inline enum iter_type iov_iter_type(const struct iov_iter *i)
 	return i->iter_type;
 }
 
+static inline void iov_iter_save_state(struct iov_iter *iter,
+				       struct iov_iter_state *state)
+{
+	state->iov_offset = iter->iov_offset;
+	state->count = iter->count;
+	state->nr_segs = iter->nr_segs;
+}
+
 static inline bool iter_is_iovec(const struct iov_iter *i)
 {
 	return iov_iter_type(i) == ITER_IOVEC;
@@ -233,6 +247,8 @@  ssize_t iov_iter_get_pages(struct iov_iter *i, struct page **pages,
 ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages,
 			size_t maxsize, size_t *start);
 int iov_iter_npages(const struct iov_iter *i, int maxpages);
+void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state,
+			ssize_t did_bytes);
 
 const void *dup_iter(struct iov_iter *new, struct iov_iter *old, gfp_t flags);
 
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index f2d50d69a6c3..280dbcc523e5 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1972,3 +1972,39 @@  int import_single_range(int rw, void __user *buf, size_t len,
 	return 0;
 }
 EXPORT_SYMBOL(import_single_range);
+
+/**
+ * iov_iter_restore() - Restore a &struct iov_iter to the same state as when
+ *     iov_iter_save_state() was called.
+ *
+ * @i: &struct iov_iter to restore
+ * @state: state to restore from
+ * @did_bytes: bytes to advance @i after restoring it
+ *
+ * Used after iov_iter_save_state() to bring restore @i, if operations may
+ * have advanced it. If @did_bytes is a positive value, then after restoring
+ * @i it is advanced accordingly. This is useful for handling short reads or
+ * writes for retry, if lower down the stack @i was advanced further than the
+ * returned value. If @did_bytes is negative (eg an error), then only the
+ * state restore is done.
+ *
+ * Note: only works on ITER_IOVEC, ITER_BVEC, and ITER_KVEC
+ */
+void iov_iter_restore(struct iov_iter *i, struct iov_iter_state *state,
+		      ssize_t did_bytes)
+{
+	if (WARN_ON_ONCE(!iov_iter_is_bvec(i) && !iter_is_iovec(i)) &&
+			 !iov_iter_is_kvec(i))
+		return;
+	i->iov_offset = state->iov_offset;
+	i->count = state->count;
+	/*
+	 * For the *vec iters, nr_segs + iov is constant - if we increment
+	 * the vec, then we also decrement the nr_segs count. Hence we don't
+	 * need to track both of these, just one is enough and we can deduct
+	 * the other from that. ITER_{BVEC,IOVEC,KVEC} all have their pointers
+	 * unionized, so we don't need to handle them individually.
+	 */
+	i->iov -= state->nr_segs - i->nr_segs;
+	i->nr_segs = state->nr_segs;
+}