diff mbox series

[2/4] splice: simplify a conditional in copy_splice_read

Message ID 20230614140341.521331-3-hch@lst.de (mailing list archive)
State New, archived
Headers show
Series [1/4] splice: don't call file_accessed in copy_splice_read | expand

Commit Message

Christoph Hellwig June 14, 2023, 2:03 p.m. UTC 
Check for -EFAULT instead of wrapping the check in an ret < 0 block.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 fs/splice.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Comments

Johannes Thumshirn June 14, 2023, 6:01 p.m. UTC | #1 
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
David Howells June 16, 2023, 9:44 a.m. UTC | #2 
Christoph Hellwig <hch@lst.de> wrote:

> +	if (ret == -EFAULT)

I wonder if that should be prefaced with "else".  On the other hand, the
optimiser ought to spot that it's mutually exclusive with the prior "if".

David
Christian Brauner June 16, 2023, 10:46 a.m. UTC | #3 
On Wed, Jun 14, 2023 at 04:03:39PM +0200, Christoph Hellwig wrote:
> Check for -EFAULT instead of wrapping the check in an ret < 0 block.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---

Looks good to me,
Reviewed-by: Christian Brauner <brauner@kernel.org>
diff mbox series

Patch

diff --git a/fs/splice.c b/fs/splice.c
index 87c69fdb333dab..7a9565d8ec4f9d 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -368,15 +368,15 @@  ssize_t copy_splice_read(struct file *in, loff_t *ppos,
 	if (ret > 0) {
 		keep = DIV_ROUND_UP(ret, PAGE_SIZE);
 		*ppos = kiocb.ki_pos;
-	} else if (ret < 0) {
-		/*
-		 * callers of ->splice_read() expect -EAGAIN on
-		 * "can't put anything in there", rather than -EFAULT.
-		 */
-		if (ret == -EFAULT)
-			ret = -EAGAIN;
 	}
 
+	/*
+	 * Callers of ->splice_read() expect -EAGAIN on "can't put anything in
+	 * there", rather than -EFAULT.
+	 */
+	if (ret == -EFAULT)
+		ret = -EAGAIN;
+
 	/* Free any pages that didn't get touched at all. */
 	if (keep < npages)
 		release_pages(pages + keep, npages - keep);