On Tue, Jun 24 2008, Nick Piggin wrote: > 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. That's a good point, moving those blocks of code to the other end makes a lot of sense. Or just kill the read-ahead, or at least do it differently. It's definitely an oversight/bug having splice from file block on the pages it just issued read-ahead for. > > 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? Precisely! > > 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...) Yep, that's the whole premise for the dynpipe branch I've been carrying around for some time. -- Jens Axboe -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
