* [PATCH] ceph: drop special-casing for ITER_PIPE in ceph_sync_read
@ 2020-08-25 20:13 Jeff Layton
2020-08-25 20:28 ` John Hubbard
0 siblings, 1 reply; 3+ messages in thread
From: Jeff Layton @ 2020-08-25 20:13 UTC (permalink / raw)
To: ceph-devel; +Cc: idryomov, ukernel, jhubbard, viro
From: John Hubbard <jhubbard@nvidia.com>
This special casing was added in 7ce469a53e71 (ceph: fix splice
read for no Fc capability case). The confirm callback for ITER_PIPE
expects that the page is Uptodate or a pagecache page and and returns
an error otherwise.
A simpler workaround is just to use the Uptodate bit, which has no
meaning for anonymous pages. Rip out the special casing for ITER_PIPE
and just SetPageUptodate before we copy to the iter.
Cc: "Yan, Zheng" <ukernel@gmail.com>
Cc: John Hubbard <jhubbard@nvidia.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
---
fs/ceph/file.c | 71 +++++++++++++++++---------------------------------
1 file changed, 24 insertions(+), 47 deletions(-)
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index fb3ea715a19d..ed8fbfe3bddc 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -863,6 +863,8 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
size_t page_off;
u64 i_size;
bool more;
+ int idx;
+ size_t left;
req = ceph_osdc_new_request(osdc, &ci->i_layout,
ci->i_vino, off, &len, 0, 1,
@@ -876,29 +878,13 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
more = len < iov_iter_count(to);
- if (unlikely(iov_iter_is_pipe(to))) {
- ret = iov_iter_get_pages_alloc(to, &pages, len,
- &page_off);
- if (ret <= 0) {
- ceph_osdc_put_request(req);
- ret = -ENOMEM;
- break;
- }
- num_pages = DIV_ROUND_UP(ret + page_off, PAGE_SIZE);
- if (ret < len) {
- len = ret;
- osd_req_op_extent_update(req, 0, len);
- more = false;
- }
- } else {
- num_pages = calc_pages_for(off, len);
- page_off = off & ~PAGE_MASK;
- pages = ceph_alloc_page_vector(num_pages, GFP_KERNEL);
- if (IS_ERR(pages)) {
- ceph_osdc_put_request(req);
- ret = PTR_ERR(pages);
- break;
- }
+ num_pages = calc_pages_for(off, len);
+ page_off = off & ~PAGE_MASK;
+ pages = ceph_alloc_page_vector(num_pages, GFP_KERNEL);
+ if (IS_ERR(pages)) {
+ ceph_osdc_put_request(req);
+ ret = PTR_ERR(pages);
+ break;
}
osd_req_op_extent_osd_data_pages(req, 0, pages, len, page_off,
@@ -929,32 +915,23 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
ret += zlen;
}
- if (unlikely(iov_iter_is_pipe(to))) {
- if (ret > 0) {
- iov_iter_advance(to, ret);
- off += ret;
- } else {
- iov_iter_advance(to, 0);
- }
- ceph_put_page_vector(pages, num_pages, false);
- } else {
- int idx = 0;
- size_t left = ret > 0 ? ret : 0;
- while (left > 0) {
- size_t len, copied;
- page_off = off & ~PAGE_MASK;
- len = min_t(size_t, left, PAGE_SIZE - page_off);
- copied = copy_page_to_iter(pages[idx++],
- page_off, len, to);
- off += copied;
- left -= copied;
- if (copied < len) {
- ret = -EFAULT;
- break;
- }
+ idx = 0;
+ left = ret > 0 ? ret : 0;
+ while (left > 0) {
+ size_t len, copied;
+ page_off = off & ~PAGE_MASK;
+ len = min_t(size_t, left, PAGE_SIZE - page_off);
+ SetPageUptodate(pages[idx]);
+ copied = copy_page_to_iter(pages[idx++],
+ page_off, len, to);
+ off += copied;
+ left -= copied;
+ if (copied < len) {
+ ret = -EFAULT;
+ break;
}
- ceph_release_page_vector(pages, num_pages);
}
+ ceph_release_page_vector(pages, num_pages);
if (ret < 0) {
if (ret == -EBLACKLISTED)
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] ceph: drop special-casing for ITER_PIPE in ceph_sync_read
2020-08-25 20:13 [PATCH] ceph: drop special-casing for ITER_PIPE in ceph_sync_read Jeff Layton
@ 2020-08-25 20:28 ` John Hubbard
2020-08-26 12:07 ` Jeff Layton
0 siblings, 1 reply; 3+ messages in thread
From: John Hubbard @ 2020-08-25 20:28 UTC (permalink / raw)
To: Jeff Layton, ceph-devel; +Cc: idryomov, ukernel, viro
On 8/25/20 1:13 PM, Jeff Layton wrote:
> From: John Hubbard <jhubbard@nvidia.com>
>
I think that's meant to be, "From: Jeff Layton <jlayton@kernel.org>".
This looks much nicer than what I came up with. :)
> This special casing was added in 7ce469a53e71 (ceph: fix splice
> read for no Fc capability case). The confirm callback for ITER_PIPE
> expects that the page is Uptodate or a pagecache page and and returns
> an error otherwise.
>
> A simpler workaround is just to use the Uptodate bit, which has no
> meaning for anonymous pages. Rip out the special casing for ITER_PIPE
> and just SetPageUptodate before we copy to the iter.
>
> Cc: "Yan, Zheng" <ukernel@gmail.com>
> Cc: John Hubbard <jhubbard@nvidia.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> fs/ceph/file.c | 71 +++++++++++++++++---------------------------------
> 1 file changed, 24 insertions(+), 47 deletions(-)
>
> diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> index fb3ea715a19d..ed8fbfe3bddc 100644
> --- a/fs/ceph/file.c
> +++ b/fs/ceph/file.c
> @@ -863,6 +863,8 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
> size_t page_off;
> u64 i_size;
> bool more;
> + int idx;
> + size_t left;
>
> req = ceph_osdc_new_request(osdc, &ci->i_layout,
> ci->i_vino, off, &len, 0, 1,
> @@ -876,29 +878,13 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
>
> more = len < iov_iter_count(to);
>
> - if (unlikely(iov_iter_is_pipe(to))) {
> - ret = iov_iter_get_pages_alloc(to, &pages, len,
> - &page_off);
+1 for removing a call to iov_iter_get_pages_alloc()! My list is shorter now.
thanks,
--
John Hubbard
NVIDIA
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ceph: drop special-casing for ITER_PIPE in ceph_sync_read
2020-08-25 20:28 ` John Hubbard
@ 2020-08-26 12:07 ` Jeff Layton
0 siblings, 0 replies; 3+ messages in thread
From: Jeff Layton @ 2020-08-26 12:07 UTC (permalink / raw)
To: John Hubbard, ceph-devel; +Cc: idryomov, ukernel, viro
On Tue, 2020-08-25 at 13:28 -0700, John Hubbard wrote:
> On 8/25/20 1:13 PM, Jeff Layton wrote:
> > From: John Hubbard <jhubbard@nvidia.com>
> >
>
> I think that's meant to be, "From: Jeff Layton <jlayton@kernel.org>".
Yeah, sorry -- artifact from squashing patches together. I noticed this
after I sent it out. It's fixed in tree though.
> This looks much nicer than what I came up with. :)
>
> > This special casing was added in 7ce469a53e71 (ceph: fix splice
> > read for no Fc capability case). The confirm callback for ITER_PIPE
> > expects that the page is Uptodate or a pagecache page and and returns
> > an error otherwise.
> >
> > A simpler workaround is just to use the Uptodate bit, which has no
> > meaning for anonymous pages. Rip out the special casing for ITER_PIPE
> > and just SetPageUptodate before we copy to the iter.
> >
> > Cc: "Yan, Zheng" <ukernel@gmail.com>
> > Cc: John Hubbard <jhubbard@nvidia.com>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> > ---
> > fs/ceph/file.c | 71 +++++++++++++++++---------------------------------
> > 1 file changed, 24 insertions(+), 47 deletions(-)
> >
> > diff --git a/fs/ceph/file.c b/fs/ceph/file.c
> > index fb3ea715a19d..ed8fbfe3bddc 100644
> > --- a/fs/ceph/file.c
> > +++ b/fs/ceph/file.c
> > @@ -863,6 +863,8 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
> > size_t page_off;
> > u64 i_size;
> > bool more;
> > + int idx;
> > + size_t left;
> >
> > req = ceph_osdc_new_request(osdc, &ci->i_layout,
> > ci->i_vino, off, &len, 0, 1,
> > @@ -876,29 +878,13 @@ static ssize_t ceph_sync_read(struct kiocb *iocb, struct iov_iter *to,
> >
> > more = len < iov_iter_count(to);
> >
> > - if (unlikely(iov_iter_is_pipe(to))) {
> > - ret = iov_iter_get_pages_alloc(to, &pages, len,
> > - &page_off);
>
> +1 for removing a call to iov_iter_get_pages_alloc()! My list is shorter now.
>
Yep, and we got rid of some special-casing in ceph to boot. Thanks for
bringing it to our attention!
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-08-26 12:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-25 20:13 [PATCH] ceph: drop special-casing for ITER_PIPE in ceph_sync_read Jeff Layton
2020-08-25 20:28 ` John Hubbard
2020-08-26 12:07 ` Jeff Layton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.