Re: [rfc patch 3/4] splice: remove confirm from pipe_buf_operations

From: Nick Piggin
Date: Tue Jun 24 2008 - 08:17:20 EST


On Tuesday 24 June 2008 21:36, Miklos Szeredi wrote:
> > It's an unfortunate side effect of the read-ahead, I'd much rather just
> > get rid of that. It _should_ behave like the non-ra case, when a page is
> > added it merely has IO started on it. So we want to have that be
> > something like
> >
> > if (!PageUptodate(page) && !PageInFlight(page))
> > ...
> >
> > basically like PageWriteback(), but for read-in.
>
> OK it could be done, possibly at great pain. But why is it important?

It has been considered, but adding atomic operations on these paths
always really hurts. Adding something like this would basically be
another at least 2 atomic operations that can never be removed again...

Provided that you've done the sync readahead earlier, it presumably
should be a very rare case to have to start new IO in the loop
below, right? In which case, I wonder if we couldn't move that 2nd
loop out of generic_file_splice_read and into
page_cache_pipe_buf_confirm.


> What's the use case where it matters that splice-in should not block
> on the read?

It just makes it generally less able to pipeline IO and computation,
doesn't it?


> And note, after the pipe is full it will block no matter what, since
> the consumer will have to wait until the page is brought uptodate, and
> can only then commence with getting the data out from the pipe.

True, but (especially with patches to variably size the pipe buffer)
I imagine programs could be designed fairly carefully to the size of
the buffer (and not just things that blast bulk data down the pipe...)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/