Messages in this thread Patch in this message | | | Date | Mon, 28 Feb 2022 14:49:11 -0800 | Subject | Re: [PATCH 2/6] iov_iter: new iov_iter_pin_pages*(), for FOLL_PIN pages | From | John Hubbard <> |
| |
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
| |