2023-04-19 08:57:12

by David Howells

[permalink] [raw]
Subject: [PATCH] splice: Fix filemap of a blockdev

Fix the new filemap_splice_read() function to get i_size from
in->f_mapping->host, not in->f_inode so that it works with block devices
too (in->f_inode points to the device file, which is typically zero size).

Fixes: 07073eb01c5f ("splice: Add a func to do a splice from a buffered file without ITER_PIPE")
Link: https://lore.kernel.org/r/[email protected]/
Reported-by: Ayush Jain <[email protected]>
cc: Jens Axboe <[email protected]>
cc: Christoph Hellwig <[email protected]>
cc: Al Viro <[email protected]>
cc: David Hildenbrand <[email protected]>
cc: John Hubbard <[email protected]>
cc: Steve French <[email protected]>
cc: [email protected]
cc: [email protected]
cc: [email protected]
---
mm/filemap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/filemap.c b/mm/filemap.c
index 470be06b6096..f86cc8acf33a 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2902,7 +2902,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
do {
cond_resched();

- if (*ppos >= i_size_read(file_inode(in)))
+ if (*ppos >= i_size_read(in->f_mapping->host))
break;

iocb.ki_pos = *ppos;
@@ -2918,7 +2918,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
* part of the page is not copied back to userspace (unless
* another truncate extends the file - this is desired though).
*/
- isize = i_size_read(file_inode(in));
+ isize = i_size_read(in->f_mapping->host);
if (unlikely(*ppos >= isize))
break;
end_offset = min_t(loff_t, isize, *ppos + len);


2023-04-19 11:25:22

by Ayush Jain

[permalink] [raw]
Subject: Re: [PATCH] splice: Fix filemap of a blockdev

Hello David,

This resolves the Fio-test issue that i reported in
Link: https://lore.kernel.org/r/[email protected]/

On 4/19/2023 2:24 PM, David Howells wrote:
> Fix the new filemap_splice_read() function to get i_size from
> in->f_mapping->host, not in->f_inode so that it works with block devices
> too (in->f_inode points to the device file, which is typically zero size).
>
> Fixes: 07073eb01c5f ("splice: Add a func to do a splice from a buffered file without ITER_PIPE")
> Link: https://lore.kernel.org/r/[email protected]/
> Reported-by: Ayush Jain <[email protected]>

Tested-by: Ayush Jain <[email protected]>

> cc: Jens Axboe <[email protected]>
> cc: Christoph Hellwig <[email protected]>
> cc: Al Viro <[email protected]>
> cc: David Hildenbrand <[email protected]>
> cc: John Hubbard <[email protected]>
> cc: Steve French <[email protected]>
> cc: [email protected]
> cc: [email protected]
> cc: [email protected]
> ---
> mm/filemap.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index 470be06b6096..f86cc8acf33a 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -2902,7 +2902,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
> do {
> cond_resched();
>
> - if (*ppos >= i_size_read(file_inode(in)))
> + if (*ppos >= i_size_read(in->f_mapping->host))
> break;
>
> iocb.ki_pos = *ppos;
> @@ -2918,7 +2918,7 @@ ssize_t filemap_splice_read(struct file *in, loff_t *ppos,
> * part of the page is not copied back to userspace (unless
> * another truncate extends the file - this is desired though).
> */
> - isize = i_size_read(file_inode(in));
> + isize = i_size_read(in->f_mapping->host);
> if (unlikely(*ppos >= isize))
> break;
> end_offset = min_t(loff_t, isize, *ppos + len);
>

Regards,
Ayush Jain

2023-04-19 12:52:10

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] splice: Fix filemap of a blockdev


On Wed, 19 Apr 2023 09:54:11 +0100, David Howells wrote:
> Fix the new filemap_splice_read() function to get i_size from
> in->f_mapping->host, not in->f_inode so that it works with block devices
> too (in->f_inode points to the device file, which is typically zero size).
>
>

Applied, thanks!

[1/1] splice: Fix filemap of a blockdev
commit: 5a9515a407d1aec1dd76c24fe99d9981730b74fb

Best regards,
--
Jens Axboe



2023-04-19 13:06:35

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] splice: Fix filemap of a blockdev

Jens Axboe <[email protected]> wrote:

> [1/1] splice: Fix filemap of a blockdev

Actually, would you be able to fix the subject? I left a word out:

splice: Fix filemap splice of a blockdev

or maybe:

splice: Fix buffered splice of a blockdev

Sorry about that,
David

2023-04-19 13:11:18

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] splice: Fix filemap of a blockdev

Note that this shouldn't affect direct_splice_read() as that doesn't look at
the size of the file, but rather just keeps reading from it until the
requested amount is read, the pipe is full or ->read_iter() indicates EOF by
returning 0 (so it could work for copy-splicing from a pipe/socket/chardev
too).

David

2023-04-19 13:18:52

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] splice: Fix filemap of a blockdev

On 4/19/23 6:57 AM, David Howells wrote:
> Jens Axboe <[email protected]> wrote:
>
>> [1/1] splice: Fix filemap of a blockdev
>
> Actually, would you be able to fix the subject? I left a word out:
>
> splice: Fix filemap splice of a blockdev
>
> or maybe:
>
> splice: Fix buffered splice of a blockdev

Fixed it up.

--
Jens Axboe