On Thu, Jul 30, 2020 at 04:08:26PM +0100, Al Viro wrote: > > I think we need to fix that in the instances, as we really expect > > ->splice_read to just work instead of the caller knowing what could > > work and what might not. > > Er... generic_file_splice_read() is a library helper; the decision to use > is up to the filesystem/driver/protocol in question, and so's making sure > it's not used with ->read_iter() that isn't fit for it. Yes, but.. The problem is that while right now generic_file_splice_read is the only user of ITER_PIPE there is absolutely not guarantee that it remains the only user. Having ->read_iter instances lingering that can't deal with it is at best a mine field waiting for victims. Fortunately I think the fix is pretty easy - remove the special pipe zero copy optimization from copy_page_to_iter, and just have the callers actually want it because they have pagecache or similar refcountable pages use it explicitly for the ITER_PIPE case. That gives us a safe default with an opt-in into the optimized variant. I'm currently auditing all the users of for how it is used and that looks pretty promising. > Note that we *do* have instances where we have different ->splice_read() > (sometimes using generic_file_splice_read(), sometimes not) even though > ->read_iter() is there. > > Your patch ignores those (thankfully), but commit message is rather > misleading - it strongly implies that generic_file_splice_read() is > *always* the right thing when ->read_iter() is there, not just that > in such cases it makes a better fallback than default_file_splice_read(). I don't think it always is right. Not without a major audit and more work at least.
