On Wed, 2022-06-22 at 05:15 +0100, Al Viro wrote:
> ... and don't mangle maxsize there - turn the loop into counting
> one instead. Easier to see that we won't run out of array that
> way. Note that special treatment of the partial buffer in that
> thing is an artifact of the non-advancing semantics of
> iov_iter_get_pages() - if not for that, it would be append_pipe(),
> same as the body of the loop that follows it. IOW, once we make
> iov_iter_get_pages() advancing, the whole thing will turn into
> calculate how many pages do we want
> allocate an array (if needed)
> call append_pipe() that many times.
>
> Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> ---
> lib/iov_iter.c | 75 +++++++++++++++++++++++++-------------------------
> 1 file changed, 38 insertions(+), 37 deletions(-)
>
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index f455b8ee0d76..9280f865fd6a 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -1192,60 +1192,61 @@ static struct page **get_pages_array(size_t n)
> return kvmalloc_array(n, sizeof(struct page *), GFP_KERNEL);
> }
>
> -static inline ssize_t __pipe_get_pages(struct iov_iter *i,
> - size_t maxsize,
> - struct page **pages,
> - size_t off)
> -{
> - struct pipe_inode_info *pipe = i->pipe;
> - ssize_t left = maxsize;
> -
> - if (off) {
> - struct pipe_buffer *buf = pipe_buf(pipe, pipe->head - 1);
> -
> - get_page(*pages++ = buf->page);
> - left -= PAGE_SIZE - off;
> - if (left <= 0) {
> - buf->len += maxsize;
> - return maxsize;
> - }
> - buf->len = PAGE_SIZE;
> - }
> - while (!pipe_full(pipe->head, pipe->tail, pipe->max_usage)) {
> - struct page *page = push_anon(pipe,
> - min_t(ssize_t, left, PAGE_SIZE));
> - if (!page)
> - break;
> - get_page(*pages++ = page);
> - left -= PAGE_SIZE;
> - if (left <= 0)
> - return maxsize;
> - }
> - return maxsize - left ? : -EFAULT;
> -}
> -
> static ssize_t pipe_get_pages(struct iov_iter *i,
> struct page ***pages, size_t maxsize, unsigned maxpages,
> size_t *start)
> {
> + struct pipe_inode_info *pipe = i->pipe;
> unsigned int npages, off;
> struct page **p;
> - size_t capacity;
> + ssize_t left;
> + int count;
>
> if (!sanity(i))
> return -EFAULT;
>
> *start = off = pipe_npages(i, &npages);
> - capacity = min(npages, maxpages) * PAGE_SIZE - off;
> - maxsize = min(maxsize, capacity);
> + count = DIV_ROUND_UP(maxsize + off, PAGE_SIZE);
> + if (count > npages)
> + count = npages;
> + if (count > maxpages)
> + count = maxpages;
> p = *pages;
> if (!p) {
> - *pages = p = get_pages_array(DIV_ROUND_UP(maxsize + off, PAGE_SIZE));
> + *pages = p = get_pages_array(count);
> if (!p)
> return -ENOMEM;
> }
>
> - return __pipe_get_pages(i, maxsize, p, off);
> + left = maxsize;
> + npages = 0;
> + if (off) {
> + struct pipe_buffer *buf = pipe_buf(pipe, pipe->head - 1);
> +
> + get_page(*p++ = buf->page);
> + left -= PAGE_SIZE - off;
> + if (left <= 0) {
> + buf->len += maxsize;
> + return maxsize;
> + }
> + buf->len = PAGE_SIZE;
> + npages = 1;
> + }
> + for ( ; npages < count; npages++) {
> + struct page *page;
> + unsigned int size = min_t(ssize_t, left, PAGE_SIZE);
> +
> + if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
> + break;
> + page = push_anon(pipe, size);
> + if (!page)
> + break;
> + get_page(*p++ = page);
> + left -= size;
> + }
> + if (!npages)
> + return -EFAULT;
> + return maxsize - left;
> }
>
> static ssize_t iter_xarray_populate_pages(struct page **pages, struct xarray *xa,
Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx>
