Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754814AbbLQTXN (ORCPT ); Thu, 17 Dec 2015 14:23:13 -0500 Received: from mx2.suse.de ([195.135.220.15]:37412 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752482AbbLQTXL (ORCPT ); Thu, 17 Dec 2015 14:23:11 -0500 Date: Thu, 17 Dec 2015 20:23:09 +0100 From: Jan Kara To: Abhi Das Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Bob Peterson , Jan Kara Subject: Re: [RFC v3 PATCH] fs: __generic_file_splice_read retry lookup on AOP_TRUNCATED_PAGE Message-ID: <20151217192309.GA8083@quack.suse.cz> References: <1450292572-53344-1-git-send-email-adas@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1450292572-53344-1-git-send-email-adas@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2615 Lines: 75 On Wed 16-12-15 13:02:52, Abhi Das wrote: > During testing, I discovered that __generic_file_splice_read() returns > 0 (EOF) when aops->readpage fails with AOP_TRUNCATED_PAGE on the first > page of a single/multi-page splice read operation. This EOF return code > causes the userspace test to (correctly) report a zero-length read error > when it was expecting otherwise. > > The current strategy of returning a partial non-zero read when ->readpage > returns AOP_TRUNCATED_PAGE works only when the failed page is not the > first of the lot being processed. > > This patch attempts to retry lookup and call ->readpage again on pages > that had previously failed with AOP_TRUNCATED_PAGE. With this patch, my > tests pass and I haven't noticed any unwanted side effects. > > This version removes the thrice-retry loop and instead indefinitely > retries lookups on AOP_TRUNCATED_PAGE errors from ->readpage. > > Signed-off-by: Abhi Das > Cc: Bob Peterson > Cc: Jan Kara The patch looks good to me. You can add: Reviewed-by: Jan Kara BTW, you should send the patch to Al Viro as he is the VFS maintainer so he should merge the patch. It might be also useful to mention in the changelog that do_generic_file_read() behaves the same way. Honza > --- > fs/splice.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/fs/splice.c b/fs/splice.c > index 801c21c..277df71 100644 > --- a/fs/splice.c > +++ b/fs/splice.c > @@ -415,6 +415,7 @@ __generic_file_splice_read(struct file *in, loff_t *ppos, > */ > if (!page->mapping) { > unlock_page(page); > +retry_lookup: > page = find_or_create_page(mapping, index, > mapping_gfp_mask(mapping)); > > @@ -439,13 +440,10 @@ __generic_file_splice_read(struct file *in, loff_t *ppos, > error = mapping->a_ops->readpage(in, page); > if (unlikely(error)) { > /* > - * We really should re-lookup the page here, > - * but it complicates things a lot. Instead > - * lets just do what we already stored, and > - * we'll get it the next time we are called. > + * Re-lookup the page > */ > if (error == AOP_TRUNCATED_PAGE) > - error = 0; > + goto retry_lookup; > > break; > } > -- > 2.4.3 > > -- Jan Kara SUSE Labs, CR -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/