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

[Miklos Szeredi]

> On Tue, 24 Jun 2008, Miklos Szeredi wrote:
> > 
> > OK.  But currently we have an implementation that
> > 
> >  1) doesn't do any of this, unless readahead is disabled
> 
> Sure. But removing even the conceptual support? Not a good idea.
> 
> > And in addition, splice-in and splice-out can return a short count or
> > even zero count if the filesystem invalidates the cached pages during
> > the splicing (data became stale for example).  Are these the right
> > semantics?  I'm not sure.
> 
> What does that really have with splice() and removing the features? Why 
> don't you just fix that issue? 

Because it's freakin' difficult, and I'm lazy, that's why :)

Let's start with page_cache_pipe_buf_confirm().  How should we deal
with finding an invalidated page (!PageUptodate(page) &&
!page->mapping)?

We could return zero to use the contents even though it was
invalidated, not good, but if the page was originally uptodate, then
it should be OK to use the stale data.  But it could have been
invalidated before becoming uptodate, so the contents could be total
crap, and that's not good.  So now we have to tweak page invalidation
to differentiate between was-uptodate and was-never-uptodate pages.

The other is __generic_file_splice_read().  Currently it just bails
out if it finds an invalidated page.  That could be rewritten to throw
away the page, look it up again in the radix tree, etc, etc...  Lots
of added complexity in an already not-too-simple function.

All for what?  To be able to keep the async-on-no-readahead behavior
of generic_file_splice_read()?  The current implementation is not even
close to what would be required to do the async splicing properly.

Conclusion: I think we are better off with a simple
do_generic_file_read() based implementation until someone gives this
the proper thought and effort, than to leave all the complex and dead
code to rot and cause people (me) headaches... :)

Miklos
--
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/