From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DC613C433DF for ; Tue, 25 Aug 2020 16:22:23 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7978020786 for ; Tue, 25 Aug 2020 16:22:23 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="yj+xSLQW" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7978020786 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id F22DE8E0007; Tue, 25 Aug 2020 12:22:22 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id ED3D68D0001; Tue, 25 Aug 2020 12:22:22 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DC2B88E0007; Tue, 25 Aug 2020 12:22:22 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0085.hostedemail.com [216.40.44.85]) by kanga.kvack.org (Postfix) with ESMTP id C13028D0001 for ; Tue, 25 Aug 2020 12:22:22 -0400 (EDT) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 4B983180AD804 for ; Tue, 25 Aug 2020 16:22:22 +0000 (UTC) X-FDA: 77189608524.24.elbow85_0016be22705d Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin24.hostedemail.com (Postfix) with ESMTP id 1CD881A4A5 for ; Tue, 25 Aug 2020 16:22:22 +0000 (UTC) X-HE-Tag: elbow85_0016be22705d X-Filterd-Recvd-Size: 6544 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by imf42.hostedemail.com (Postfix) with ESMTP for ; Tue, 25 Aug 2020 16:22:21 +0000 (UTC) Received: from tleilax.poochiereds.net (68-20-15-154.lightspeed.rlghnc.sbcglobal.net [68.20.15.154]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 204672076C; Tue, 25 Aug 2020 16:22:19 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1598372540; bh=Ob6Ya4sG/9TFYtRrP6g9VXPseZ2X38wAlcrlrPayfhY=; h=Subject:From:To:Cc:Date:In-Reply-To:References:From; b=yj+xSLQWrJYsteoxUDzI4203SRM4N7vwTUF/ZLxu3kZZTD61PsVNefCJ2heTYdXez ZjUPhgm7YB7fwVIZoz++Avg+NgquXhY3ARdasGto7IuSRSJT7cNRqt1af9BdF/Zw0o sQQn//fM6Q7RI/4QY5VStkEfy1CYo69EUGd8g2to= Message-ID: <578fb1e557d1990f768b7fdf5c6e4505db4c24e6.camel@kernel.org> Subject: Re: [PATCH v2] fs/ceph: use pipe_get_pages_alloc() for pipe From: Jeff Layton To: John Hubbard , willy@infradead.org Cc: akpm@linux-foundation.org, axboe@kernel.dk, ceph-devel@vger.kernel.org, hch@infradead.org, idryomov@gmail.com, linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-xfs@vger.kernel.org, viro@zeniv.linux.org.uk, "Yan, Zheng" Date: Tue, 25 Aug 2020 12:22:17 -0400 In-Reply-To: <20200825012034.1962362-1-jhubbard@nvidia.com> References: <20200824185400.GE17456@casper.infradead.org> <20200825012034.1962362-1-jhubbard@nvidia.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5 (3.36.5-1.fc32) MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 1CD881A4A5 X-Spamd-Result: default: False [0.00 / 100.00] X-Rspamd-Server: rspam04 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, 2020-08-24 at 18:20 -0700, John Hubbard wrote: > This reduces, by one, the number of callers of iov_iter_get_pages(). > That's helpful because these calls are being audited and converted over > to use iov_iter_pin_user_pages(), where applicable. And this one here is > already known by the caller to be only for ITER_PIPE, so let's just > simplify it now. > > Signed-off-by: John Hubbard > --- > > OK, here's a v2 that does EXPORT_SYMBOL_GPL, instead of EXPORT_SYMBOL, > that's the only change from v1. That should help give this patch a > clear bill of passage. :) > > thanks, > John Hubbard > NVIDIA > > fs/ceph/file.c | 3 +-- > include/linux/uio.h | 3 ++- > lib/iov_iter.c | 6 +++--- > 3 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/fs/ceph/file.c b/fs/ceph/file.c > index d51c3f2fdca0..d3d7dd957390 100644 > --- a/fs/ceph/file.c > +++ b/fs/ceph/file.c > @@ -879,8 +879,7 @@ 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); > + ret = pipe_get_pages_alloc(to, &pages, len, &page_off); > if (ret <= 0) { > ceph_osdc_put_request(req); > ret = -ENOMEM; > diff --git a/include/linux/uio.h b/include/linux/uio.h > index 3835a8a8e9ea..270a4dcf5453 100644 > --- a/include/linux/uio.h > +++ b/include/linux/uio.h > @@ -226,7 +226,8 @@ ssize_t iov_iter_get_pages(struct iov_iter *i, struct page **pages, > ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages, > size_t maxsize, size_t *start); > int iov_iter_npages(const struct iov_iter *i, int maxpages); > - > +ssize_t pipe_get_pages_alloc(struct iov_iter *i, struct page ***pages, > + size_t maxsize, size_t *start); > const void *dup_iter(struct iov_iter *new, struct iov_iter *old, gfp_t flags); > > static inline size_t iov_iter_count(const struct iov_iter *i) > diff --git a/lib/iov_iter.c b/lib/iov_iter.c > index 5e40786c8f12..6290998df480 100644 > --- a/lib/iov_iter.c > +++ b/lib/iov_iter.c > @@ -1355,9 +1355,8 @@ static struct page **get_pages_array(size_t n) > return kvmalloc_array(n, sizeof(struct page *), GFP_KERNEL); > } > > -static ssize_t pipe_get_pages_alloc(struct iov_iter *i, > - struct page ***pages, size_t maxsize, > - size_t *start) > +ssize_t pipe_get_pages_alloc(struct iov_iter *i, struct page ***pages, > + size_t maxsize, size_t *start) > { > struct page **p; > unsigned int iter_head, npages; > @@ -1387,6 +1386,7 @@ static ssize_t pipe_get_pages_alloc(struct iov_iter *i, > kvfree(p); > return n; > } > +EXPORT_SYMBOL_GPL(pipe_get_pages_alloc); > > ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, > struct page ***pages, size_t maxsize, Thanks. I've got a v1 of this in the ceph-client/testing branch and it seems fine so far. I'd prefer an ack from Al on one or the other though, since I'm not sure he wants to expose this primitive, and in the past he hasn't been enamored with EXPORT_SYMBOL_GPL, because its meaning wasn't well defined. Maybe that's changed since. As a side note, Al also asked privately why ceph special cases ITER_PIPE. I wasn't sure either, so I did a bit of git-archaeology. The change was added here: ---------------------------8<--------------------------- commit 7ce469a53e7106acdaca2e25027941d0f7c12a8e Author: Yan, Zheng Date: Tue Nov 8 21:54:34 2016 +0800 ceph: fix splice read for no Fc capability case When iov_iter type is ITER_PIPE, copy_page_to_iter() increases the page's reference and add the page to a pipe_buffer. It also set the pipe_buffer's ops to page_cache_pipe_buf_ops. The comfirm callback in page_cache_pipe_buf_ops expects the page is from page cache and uptodate, otherwise it return error. For ceph_sync_read() case, pages are not from page cache. So we can't call copy_page_to_iter() when iov_iter type is ITER_PIPE. The fix is using iov_iter_get_pages_alloc() to allocate pages for the pipe. (the code is similar to default_file_splice_read) Signed-off-by: Yan, Zheng ---------------------------8<--------------------------- If we don't have Fc (FILE_CACHE) caps then the client's not allowed to cache data and so we can't use the pagecache. I'm not certain special casing pipes in ceph, is the best approach to handle that, but the confirm callback still seems to work the same way today. Cheers, -- Jeff Layton