iov_iter: Add ITER_MAPPING

Add an iterator, ITER_MAPPING, that walks through a set of pages attached to an address_space, starting at a given file position and length.

The caller must guarantee that the pages are all present and they must be locked is some way[*] (eg. ref, PG_locked, PG_writeback or PG_fscache) to prevent them from going away or being migrated whilst they're being accessed.

[*] Note that I'm assuming that a page cannot get detached from or replaced
in a mapping if it is so 'locked' without permission from the appropriate address_space operation.

This is useful for copying data from socket buffers to inodes in network filesystems and for transferring data between those inodes and the cache using direct I/O.

Whilst it is true that ITER_BVEC could be used instead, the following issues arise:

(1) A bio_vec array would need to be allocated to list to all the pages -

     which is redundant if inode->i_pages *also* points to all these pages.

(2) A bio_vec array might exceed the size of a page (a page-sized bio_vec array would correspond to a 1M payload on a 64-bit machine with 4K pages).

(3) The offset and length fields in the bio_vec elements are superfluous in this situation.

An alternative could be to have an "ITER_ARRAY" that just has the page pointers and not the offset/length info. This decreases the redundancy and increases the max payload-per-array-page to 2M.

I've been using this in my rewrite of fscache/cachefiles, which can be found here:

https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=fscache-iter

---
 include/linux/uio.h |   11 ++
 lib/iov_iter.c      |  282 +++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 270 insertions(+), 23 deletions(-)

index 9576fd8158d7..a0321a740f51 100644
@@ -11,6 +11,7 @@
 #include <uapi/linux/uio.h>
 
 struct page;
+struct address_space;
 struct pipe_inode_info;
 
 struct kvec {

@@ -25,6 +26,7 @@ enum iter_type {
 	ITER_BVEC = 16,
 	ITER_PIPE = 32,
 	ITER_DISCARD = 64,
+	ITER_MAPPING = 128,
 };
 
 struct iov_iter {

@@ -40,6 +42,7 @@ struct iov_iter {
 		const struct iovec *iov;
 		const struct kvec *kvec;
 		const struct bio_vec *bvec;
+		struct address_space *mapping;
 		struct pipe_inode_info *pipe;
 	};
 	union {

@@ -48,6 +51,7 @@ struct iov_iter {
 			unsigned int head;
 			unsigned int start_head;
 		};
+		loff_t mapping_start;
 	};
 };
 

@@ -81,6 +85,11 @@ static inline bool iov_iter_is_discard(const struct iov_iter *i)
 	return iov_iter_type(i) == ITER_DISCARD;
 }
 
+static inline bool iov_iter_is_mapping(const struct iov_iter *i)
+{
+	return iov_iter_type(i) == ITER_MAPPING;
+}
+
 static inline unsigned char iov_iter_rw(const struct iov_iter *i)
 {
 	return i->type & (READ | WRITE);

@@ -222,6 +231,8 @@ void iov_iter_bvec(struct iov_iter *i, unsigned int direction, const struct bio_
 void iov_iter_pipe(struct iov_iter *i, unsigned int direction, struct pipe_inode_info *pipe,
 			size_t count);
 void iov_iter_discard(struct iov_iter *i, unsigned int direction, size_t count);
+void iov_iter_mapping(struct iov_iter *i, unsigned int direction, struct address_space *mapping,
+		      loff_t start, size_t count);
 ssize_t iov_iter_get_pages(struct iov_iter *i, struct page **pages,
 			size_t maxsize, unsigned maxpages, size_t *start);
 ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages,

We could also have an ITER_XARRAY which you just pass &mapping->i_pages to. I don't think you use any other part of the mapping, so that would be a more generic version that is equally efficient.

Well, I stated that one of the preconditions for using this was that the caller made sure that segment of the mapping was fully populated, so the check ought to be unnecessary.

Yeah, I'm just not sure how to deal with them.

Ok.

It would mean that the STEP function would have to handle multiple pages, some part(s) of which might need to be ignored and wouldn't be able to simply call memcpy_from/to_page().

Yes - as a strictly temporary thing. I need a struct contains a page pointer, a start and a length, and constructing a struct bio_vec on the fly here allows the caller to potentially share code. For example:

    size_t _copy_from_iter_nocache(void *addr, size_t bytes, struct iov_iter *i)
    {
	char *to = addr;
	if (unlikely(iov_iter_is_pipe(i))) {
		WARN_ON(1);
		return 0;
	}
	iterate_and_advance(i, bytes, v,
		__copy_from_user_inatomic_nocache((to += v.iov_len) - v.iov_len,
					 v.iov_base, v.iov_len),
		memcpy_from_page((to += v.bv_len) - v.bv_len, v.bv_page,
				 v.bv_offset, v.bv_len),
ITER_BVEC ^^^^
		memcpy((to += v.iov_len) - v.iov_len, v.iov_base, v.iov_len),
		memcpy_from_page((to += v.bv_len) - v.bv_len, v.bv_page,
				 v.bv_offset, v.bv_len)
ITER_MAPPING ^^^^
	)

	return bytes;
    }

David

Like this?

#define iterate_mapping(i, n, __v, skip, STEP) {		\
	struct page *page;					\
	size_t wanted = n, seg, offset;				\
	loff_t start = i->mapping_start + skip;			\
	pgoff_t index = start >> PAGE_SHIFT;			\
								\
	XA_STATE(xas, &i->mapping->i_pages, index);		\
								\
	rcu_read_lock();						\
	xas_for_each(&xas, page, ULONG_MAX) {				\
		if (xas_retry(&xas, page) || xa_is_value(page)) {	\
			WARN_ON(1);					\
			break;						\
		}							\
		__v.bv_page = find_subpage(page, xas.xa_index);		\
		offset = (i->mapping_start + skip) & ~PAGE_MASK;	\
		seg = PAGE_SIZE - offset;			\
		__v.bv_offset = offset;				\
		__v.bv_len = min(n, seg);			\
		(void)(STEP);					\
		n -= __v.bv_len;				\
		skip += __v.bv_len;				\
		if (n == 0)					\
			break;					\
	}							\
	rcu_read_unlock();					\
	n = wanted - n;						\
}

Note that the walk is not restartable - and the array is supposed to have been fully populated by the caller for the range specified - so I've made it print a warning and end the loop if xas_retry() or xa_is_value() return true (which takes care of the !page case too). Possibly I could just leave it to fault in this case and not check.

If PageHuge(page) is true, I presume I need to support that too. How do I find out how big the page is?

David

I actually quite liked the iterator you had before; I was thinking of wrapping it up as xas_for_each_contig().

Actually, xas_retry() can happen, even with the page itself pinned. It indicates the xarray data structure changed under you while walking it and you need to restart the walk from the top (arguably this shouldn't be exposed to callers at all, and in the future it may not be ... it's something inherited from the radix tree interface).

So this should be:

		if (xas_retry(&xas, page))
			continue;
		if (WARN_ON(xa_is_value(page)))
			break;

Yes.

PageHuge() is only going to be true for hugetlbfs mappings. I'm OK with not supporting those for now ... eventually I want to get rid of the special cases in the page cache for hugetlbfs, but there's about six other projects standing between me and that.

Okay, so this then?

#define iterate_mapping(i, n, __v, skip, STEP) {		\
	struct page *page;					\
	size_t wanted = n, seg, offset;				\
	loff_t start = i->mapping_start + skip;			\
	pgoff_t index = start >> PAGE_SHIFT;			\
								\
	XA_STATE(xas, &i->mapping->i_pages, index);		\
								\
	rcu_read_lock();						\
	for (page = xas_load(&xas); page; page = xas_next(&xas)) {	\
		if (xas_retry(&xas, page))				\
			continue;					\
		if (WARN_ON(xa_is_value(page)))				\
			break;						\
		if (WARN_ON(PageHuge(page)))				\
			break;						\
		__v.bv_page = find_subpage(page, xas.xa_index);		\
		offset = (i->mapping_start + skip) & ~PAGE_MASK;	\
		seg = PAGE_SIZE - offset;			\
		__v.bv_offset = offset;				\
		__v.bv_len = min(n, seg);			\
		(void)(STEP);					\
		n -= __v.bv_len;				\
		skip += __v.bv_len;				\
		if (n == 0)					\
			break;					\
	}							\
	rcu_read_unlock();					\
	n = wanted - n;						\
}

I could give that a go. Out of interest, are there any other users of xarray that might use it that don't have a mapping handy?

David