Well, a) it probably makes sense in that case to provide another mode
of operation which fills the data synchronously from the sender and
copys it to the pipe (although the sender might just use read/write)
And b) we could *also* look at clearing PG_uptodate as an optimisation
iff that is found to help.
IMO it's not worth it to complicate the API just for the sake of
correctness in the so-very-rare read error case. Users of the splice
API will simply ignore this requirement, because things will work fine
on ext3 and friends, and will break only rarely on NFS and FUSE.
So I think it's much better to make the API simple: invalid pages are
OK, and for I/O errors we return -EIO on the pipe. It's not 100%
correct, but all in all it will result in less buggy programs.
Thanks,
Miklos
----
Subject: mm: dont clear PG_uptodate on truncate/invalidate
From: Miklos Szeredi <redacted>
Brian Wang reported that a FUSE filesystem exported through NFS could return
I/O errors on read. This was traced to splice_direct_to_actor() returning a
short or zero count when racing with page invalidation.
However this is not FUSE or NFSD specific, other filesystems (notably NFS)
also call invalidate_inode_pages2() to purge stale data from the cache.
If this happens while such pages are sitting in a pipe buffer, then splice(2)
from the pipe can return zero, and read(2) from the pipe can return ENODATA.
The zero return is especially bad, since it implies end-of-file or
disconnected pipe/socket, and is documented as such for splice. But returning
an error for read() is also nasty, when in fact there was no error (data
becoming stale is not an error).
The same problems can be triggered by "hole punching" with
madvise(MADV_REMOVE).
Fix this by not clearing the PG_uptodate flag on truncation and
invalidation.
Signed-off-by: Miklos Szeredi <redacted>
---
mm/truncate.c | 2 --
1 file changed, 2 deletions(-)
Index: linux-2.6/mm/truncate.c
===================================================================
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Subject: mm: dont clear PG_uptodate on truncate/invalidate
From: Miklos Szeredi <redacted>
Ok, this one I have no problems with what-so-ever. I'd like Ack's for this
kind of change (and obviously hope that it's tested), but it looks clean
and I think the new rules are better (not just for this case).
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Nick Piggin <hidden> Date: 2008-08-02 04:26:50
On Saturday 02 August 2008 04:28, Miklos Szeredi wrote:
On Fri, 1 Aug 2008, Nick Piggin wrote:
quoted
Well, a) it probably makes sense in that case to provide another mode
of operation which fills the data synchronously from the sender and
copys it to the pipe (although the sender might just use read/write)
And b) we could *also* look at clearing PG_uptodate as an optimisation
iff that is found to help.
IMO it's not worth it to complicate the API just for the sake of
correctness in the so-very-rare read error case. Users of the splice
API will simply ignore this requirement, because things will work fine
on ext3 and friends, and will break only rarely on NFS and FUSE.
So I think it's much better to make the API simple: invalid pages are
OK, and for I/O errors we return -EIO on the pipe. It's not 100%
correct, but all in all it will result in less buggy programs.
That's true, but I hate how we always (in the VM, at least) just brush
error handling under the carpet because it is too hard :(
I guess your patch is OK, though. I don't see any reasons it could cause
problems...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Saturday 02 August 2008 04:28, Miklos Szeredi wrote:
quoted
On Fri, 1 Aug 2008, Nick Piggin wrote:
quoted
Well, a) it probably makes sense in that case to provide another mode
of operation which fills the data synchronously from the sender and
copys it to the pipe (although the sender might just use read/write)
And b) we could *also* look at clearing PG_uptodate as an optimisation
iff that is found to help.
IMO it's not worth it to complicate the API just for the sake of
correctness in the so-very-rare read error case. Users of the splice
API will simply ignore this requirement, because things will work fine
on ext3 and friends, and will break only rarely on NFS and FUSE.
So I think it's much better to make the API simple: invalid pages are
OK, and for I/O errors we return -EIO on the pipe. It's not 100%
correct, but all in all it will result in less buggy programs.
That's true, but I hate how we always (in the VM, at least) just brush
error handling under the carpet because it is too hard :(
I guess your patch is OK, though. I don't see any reasons it could cause
problems...
At least, if there are situations where the data received is not what
a common sense programmer would expect (e.g. blocks of zeros, data
from an unexpected time in syscall sequence, or something, or just
"reliable except with FUSE and NFS"), please ensure it's documented in
splice.txt or wherever.
-- Jamie
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Nick Piggin <hidden> Date: 2008-08-05 02:57:12
On Tuesday 05 August 2008 01:29, Jamie Lokier wrote:
Nick Piggin wrote:
quoted
On Saturday 02 August 2008 04:28, Miklos Szeredi wrote:
quoted
On Fri, 1 Aug 2008, Nick Piggin wrote:
quoted
Well, a) it probably makes sense in that case to provide another mode
of operation which fills the data synchronously from the sender and
copys it to the pipe (although the sender might just use read/write)
And b) we could *also* look at clearing PG_uptodate as an
optimisation iff that is found to help.
IMO it's not worth it to complicate the API just for the sake of
correctness in the so-very-rare read error case. Users of the splice
API will simply ignore this requirement, because things will work fine
on ext3 and friends, and will break only rarely on NFS and FUSE.
So I think it's much better to make the API simple: invalid pages are
OK, and for I/O errors we return -EIO on the pipe. It's not 100%
correct, but all in all it will result in less buggy programs.
That's true, but I hate how we always (in the VM, at least) just brush
error handling under the carpet because it is too hard :(
I guess your patch is OK, though. I don't see any reasons it could cause
problems...
At least, if there are situations where the data received is not what
a common sense programmer would expect (e.g. blocks of zeros, data
from an unexpected time in syscall sequence, or something, or just
"reliable except with FUSE and NFS"), please ensure it's documented in
splice.txt or wherever.
Not quite true. Many filesystems can return -EIO, and truncate can
partially zero pages.
Basically the man page should note that until the splice API is
improved, then a) -EIO errors will be seen at the receiever, b)
the pages can see transient zeroes (this is the case with read(2)
as well, but splice has a much bigger window), and c) the sender
does not send a snapshot of data because it can still be modified
until it is recieved.
c is not too surprising for an asynchronous interface, but it is
nice to document in case people are expecting COw or something.
b and c can more or less be worked around by not doing silly things
like truncating or scribbling on data until reciever really has it.
a, I argue, should be fixed in API.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Michael Kerrisk <hidden> Date: 2008-08-11 03:22:32
Nick, Jens
On Tue, Aug 5, 2008 at 4:57 AM, Nick Piggin [off-list ref] wrote:
On Tuesday 05 August 2008 01:29, Jamie Lokier wrote:
quoted
Nick Piggin wrote:
quoted
On Saturday 02 August 2008 04:28, Miklos Szeredi wrote:
quoted
On Fri, 1 Aug 2008, Nick Piggin wrote:
quoted
Well, a) it probably makes sense in that case to provide another mode
of operation which fills the data synchronously from the sender and
copys it to the pipe (although the sender might just use read/write)
And b) we could *also* look at clearing PG_uptodate as an
optimisation iff that is found to help.
IMO it's not worth it to complicate the API just for the sake of
correctness in the so-very-rare read error case. Users of the splice
API will simply ignore this requirement, because things will work fine
on ext3 and friends, and will break only rarely on NFS and FUSE.
So I think it's much better to make the API simple: invalid pages are
OK, and for I/O errors we return -EIO on the pipe. It's not 100%
correct, but all in all it will result in less buggy programs.
That's true, but I hate how we always (in the VM, at least) just brush
error handling under the carpet because it is too hard :(
I guess your patch is OK, though. I don't see any reasons it could cause
problems...
At least, if there are situations where the data received is not what
a common sense programmer would expect (e.g. blocks of zeros, data
from an unexpected time in syscall sequence, or something, or just
"reliable except with FUSE and NFS"), please ensure it's documented in
splice.txt or wherever.
Not quite true. Many filesystems can return -EIO, and truncate can
partially zero pages.
Basically the man page should note that until the splice API is
improved, then a) -EIO errors will be seen at the receiever, b)
the pages can see transient zeroes (this is the case with read(2)
as well, but splice has a much bigger window), and c) the sender
does not send a snapshot of data because it can still be modified
until it is recieved.
c is not too surprising for an asynchronous interface, but it is
nice to document in case people are expecting COw or something.
b and c can more or less be worked around by not doing silly things
like truncating or scribbling on data until reciever really has it.
a, I argue, should be fixed in API.
Nick, could you come up with a patch to the man page for this?
Something that's ACKable by Jens?
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>