On 2/27/22 13:57, Jens Axboe wrote:
+ssize_t iov_iter_pin_pages(struct iov_iter *i, + struct page **pages, size_t maxsize, unsigned int maxpages, + size_t *start) +{ + size_t len; + int n, res; + + if (maxsize > i->count) + maxsize = i->count; + if (!maxsize) + return 0; + + WARN_ON_ONCE(!iter_is_iovec(i)); + + if (likely(iter_is_iovec(i))) { + unsigned int gup_flags = 0; + unsigned long addr; + + if (iov_iter_rw(i) != WRITE) + gup_flags |= FOLL_WRITE; + if (i->nofault) + gup_flags |= FOLL_NOFAULT; + + addr = first_iovec_segment(i, &len, start, maxsize, maxpages); + n = DIV_ROUND_UP(len, PAGE_SIZE); + res = pin_user_pages_fast(addr, n, gup_flags, pages); + if (unlikely(res <= 0)) + return res; + return (res == n ? len : res * PAGE_SIZE) - *start;Trying to be clever like that just makes the code a lot less readable. I should not have to reason about a return value. Same in the other function.
Here is a differential patch on top of this one, and only showing one of the two routines. How does this direction look to you? diff --git a/lib/iov_iter.c b/lib/iov_iter.c index e64e8e4edd0c..8e96f1e9ebc6 100644 --- a/lib/iov_iter.c +++ b/lib/iov_iter.c @@ -1588,7 +1588,17 @@ ssize_t iov_iter_pin_pages(struct iov_iter *i, res = pin_user_pages_fast(addr, n, gup_flags, pages); if (unlikely(res <= 0)) return res; - return (res == n ? len : res * PAGE_SIZE) - *start; + + /* Cap len at the number of pages that were actually pinned: */ + if (res < n) + len = res * PAGE_SIZE; + + /* + * The return value is the amount pinned in bytes that the + * caller will actually use. So, reduce it by the offset into + * the first page: + */ + return len - *start; } return -EFAULT; thanks, -- John Hubbard NVIDIA
