linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jamie Lokier <jamie@shareable.org>
To: Nick Piggin <nickpiggin@yahoo.com.au>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	torvalds@linux-foundation.org, jens.axboe@oracle.com,
	akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [patch v3] splice: fix race with page invalidation
Date: Mon, 4 Aug 2008 16:29:49 +0100	[thread overview]
Message-ID: <20080804152949.GH18868@shareable.org> (raw)
In-Reply-To: <200808021426.50436.nickpiggin@yahoo.com.au>

Nick Piggin wrote:
> On Saturday 02 August 2008 04:28, Miklos Szeredi wrote:
> > On Fri, 1 Aug 2008, Nick Piggin wrote:
> > > 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

  reply	other threads:[~2008-08-04 15:30 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-30  9:43 [patch v3] splice: fix race with page invalidation Miklos Szeredi
2008-07-30 17:00 ` Linus Torvalds
2008-07-30 17:29   ` Miklos Szeredi
2008-07-30 17:54     ` Jens Axboe
2008-07-30 18:32       ` Miklos Szeredi
2008-07-30 18:43         ` Miklos Szeredi
2008-07-30 19:45           ` Jens Axboe
2008-07-30 20:05             ` Miklos Szeredi
2008-07-30 20:13               ` Linus Torvalds
2008-07-30 20:45                 ` Miklos Szeredi
2008-07-30 20:51                   ` Linus Torvalds
2008-07-30 21:16                     ` Miklos Szeredi
2008-07-30 21:22                       ` Linus Torvalds
2008-07-30 21:46                         ` Miklos Szeredi
2008-07-30 21:56                           ` Linus Torvalds
2008-07-31  0:11                   ` Jamie Lokier
2008-07-31  0:42                     ` Jamie Lokier
2008-07-31  0:51                       ` Linus Torvalds
2008-07-31  0:54                         ` Linus Torvalds
2008-07-31  6:12                         ` Jamie Lokier
2008-07-31 10:26                           ` Evgeniy Polyakov
2008-07-31 12:33                             ` Jamie Lokier
2008-07-31 12:49                               ` Nick Piggin
2008-07-31 13:29                               ` Evgeniy Polyakov
2008-07-31 16:56                                 ` Linus Torvalds
2008-07-31 16:34                           ` Linus Torvalds
2008-07-31 17:21                             ` Jamie Lokier
2008-07-31 18:54                               ` Linus Torvalds
2008-07-31  7:30                     ` Miklos Szeredi
2008-07-31  2:16       ` Nick Piggin
2008-07-31 12:59 ` Nick Piggin
2008-07-31 17:00   ` Linus Torvalds
2008-07-31 18:13     ` Miklos Szeredi
2008-08-01  1:22       ` Nick Piggin
2008-08-01 18:28         ` Miklos Szeredi
2008-08-01 18:32           ` Linus Torvalds
2008-08-02  4:26           ` Nick Piggin
2008-08-04 15:29             ` Jamie Lokier [this message]
2008-08-05  2:57               ` Nick Piggin
2008-08-11  3:22                 ` Michael Kerrisk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080804152949.GH18868@shareable.org \
    --to=jamie@shareable.org \
    --cc=akpm@linux-foundation.org \
    --cc=jens.axboe@oracle.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=miklos@szeredi.hu \
    --cc=nickpiggin@yahoo.com.au \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).