| Message ID | 20220816024143.519027-1-xiubli@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | libceph: advancing variants of iov_iter_get_pages() | expand |
On Tue, 2022-08-16 at 10:41 +0800, xiubli@redhat.com wrote: > From: Xiubo Li <xiubli@redhat.com> > > The upper layer has changed it to iov_iter_get_pages2(). And this > should be folded into the previous commit. > > Signed-off-by: Xiubo Li <xiubli@redhat.com> > --- > net/ceph/messenger.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > index 945f6d1a9efa..020474cf137c 100644 > --- a/net/ceph/messenger.c > +++ b/net/ceph/messenger.c > @@ -985,25 +985,12 @@ static struct page *ceph_msg_data_iter_next(struct ceph_msg_data_cursor *cursor, > if (cursor->lastlen) > iov_iter_revert(&cursor->iov_iter, cursor->lastlen); > > - len = iov_iter_get_pages(&cursor->iov_iter, &page, PAGE_SIZE, > - 1, page_offset); > + len = iov_iter_get_pages2(&cursor->iov_iter, &page, PAGE_SIZE, > + 1, page_offset); > BUG_ON(len < 0); > > cursor->lastlen = len; > > - /* > - * FIXME: Al Viro says that he will soon change iov_iter_get_pages > - * to auto-advance the iterator. Emulate that here for now. > - */ > - iov_iter_advance(&cursor->iov_iter, len); > - > - /* > - * FIXME: The assumption is that the pages represented by the iov_iter > - * are pinned, with the references held by the upper-level > - * callers, or by virtue of being under writeback. Eventually, > - * we'll get an iov_iter_get_pages variant that doesn't take page > - * refs. Until then, just put the page ref. > - */ I think the comment above should not be removed. Eventually Al plans to add a version of this that doesn't take page refs, and this patch doesn't change that. > VM_BUG_ON_PAGE(!PageWriteback(page) && page_count(page) < 2, page); > put_page(page); > Patch itself looks fine though. Thanks for fixing it up! Reviewed-by: Jeff Layton <jlayton@kernel.org>
On 8/17/22 6:17 PM, Jeff Layton wrote: > On Tue, 2022-08-16 at 10:41 +0800, xiubli@redhat.com wrote: >> From: Xiubo Li <xiubli@redhat.com> >> >> The upper layer has changed it to iov_iter_get_pages2(). And this >> should be folded into the previous commit. >> >> Signed-off-by: Xiubo Li <xiubli@redhat.com> >> --- >> net/ceph/messenger.c | 17 ++--------------- >> 1 file changed, 2 insertions(+), 15 deletions(-) >> >> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c >> index 945f6d1a9efa..020474cf137c 100644 >> --- a/net/ceph/messenger.c >> +++ b/net/ceph/messenger.c >> @@ -985,25 +985,12 @@ static struct page *ceph_msg_data_iter_next(struct ceph_msg_data_cursor *cursor, >> if (cursor->lastlen) >> iov_iter_revert(&cursor->iov_iter, cursor->lastlen); >> >> - len = iov_iter_get_pages(&cursor->iov_iter, &page, PAGE_SIZE, >> - 1, page_offset); >> + len = iov_iter_get_pages2(&cursor->iov_iter, &page, PAGE_SIZE, >> + 1, page_offset); >> BUG_ON(len < 0); >> >> cursor->lastlen = len; >> >> - /* >> - * FIXME: Al Viro says that he will soon change iov_iter_get_pages >> - * to auto-advance the iterator. Emulate that here for now. >> - */ >> - iov_iter_advance(&cursor->iov_iter, len); >> - >> - /* >> - * FIXME: The assumption is that the pages represented by the iov_iter >> - * are pinned, with the references held by the upper-level >> - * callers, or by virtue of being under writeback. Eventually, >> - * we'll get an iov_iter_get_pages variant that doesn't take page >> - * refs. Until then, just put the page ref. >> - */ > I think the comment above should not be removed. Eventually Al plans to > add a version of this that doesn't take page refs, and this patch > doesn't change that. Okay, I will remove this. > >> VM_BUG_ON_PAGE(!PageWriteback(page) && page_count(page) < 2, page); >> put_page(page); >> > Patch itself looks fine though. Thanks for fixing it up! > > Reviewed-by: Jeff Layton <jlayton@kernel.org> > Thanks Jeff.
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c index 945f6d1a9efa..020474cf137c 100644 --- a/net/ceph/messenger.c +++ b/net/ceph/messenger.c @@ -985,25 +985,12 @@ static struct page *ceph_msg_data_iter_next(struct ceph_msg_data_cursor *cursor, if (cursor->lastlen) iov_iter_revert(&cursor->iov_iter, cursor->lastlen); - len = iov_iter_get_pages(&cursor->iov_iter, &page, PAGE_SIZE, - 1, page_offset); + len = iov_iter_get_pages2(&cursor->iov_iter, &page, PAGE_SIZE, + 1, page_offset); BUG_ON(len < 0); cursor->lastlen = len; - /* - * FIXME: Al Viro says that he will soon change iov_iter_get_pages - * to auto-advance the iterator. Emulate that here for now. - */ - iov_iter_advance(&cursor->iov_iter, len); - - /* - * FIXME: The assumption is that the pages represented by the iov_iter - * are pinned, with the references held by the upper-level - * callers, or by virtue of being under writeback. Eventually, - * we'll get an iov_iter_get_pages variant that doesn't take page - * refs. Until then, just put the page ref. - */ VM_BUG_ON_PAGE(!PageWriteback(page) && page_count(page) < 2, page); put_page(page);