fs: __generic_file_splice_read retry lookup on AOP_TRUNCATED_PAGE

Orabug: 26797298

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. This
behavior is now similar to do_generic_file_read().

Signed-off-by: Abhi Das <[email protected]>
Reviewed-by: Jan Kara <[email protected]>
Cc: Bob Peterson <[email protected]>
Cc: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>

AOP_TRUNCATED_PAGE is not used much in the kernel now, but ocfs2 uses it to
avoid deadlocks. Specifically, ocfs2_readpage() fails the read and returns
AOP_TRUNCATED_PAGE in order to avoid deadlock on page lock with the
downconvert thread, if it fails to get the inode cluster lock. It also uses
this return value to avoid livelock on the ip_alloc_sem semaphore. This is
done with the expectation that the VFS will check for this return value and
retry the read on the page and do_generic_file_read() does exactly this.
However, in case of splice read, __generic_file_splice_read() fails the read
and returns a partial/zero-length read back. This causes upper layers that use
splice read (such as nfs) to return EIO or other failures to userspace. Saar
ran into this issue while testing database workloads over knfs with ocfs2 as
the backend fs on the nfs server. This issue is fixed with this patch in place.

(cherrypicked from commit 90330e6)
Tested-by: Saar Maoz <[email protected]>
Signed-off-by: Ashish Samant <[email protected]>
Reviewed-by: Junxiao Bi <[email protected]>

Author: Abhi Das

fs/splice.c

@@ -417,6 +417,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));
 
@@ -441,13 +442,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;
 			}