![]() | |||||||||||
Messages in this thread |
(So Linus basically handled everything here, I'll make some scattered comments where I made changes). On Wed, Mar 29 2006, Linus Torvalds wrote: > > - splice() doesn't check for (len < 0), like read() and write() do. > > Should it? > > Umm. More likely better to just do rw_verify_area() instead, which limits > it to MAX_INT. Although it probably doesn't matter, for the above obvious > reason anyway (ie we end up doing everything on a page-granular area > anyway). I've added rw_verify_area() calls now. > > - what does `flags' do, anyway? The whole thing is undocumented and > > almost uncommented. > > Right now "flags" doesn't do anything at all, and you should just pass in > zero. > > But if we ever do a "move" vs "copy" hint, we'll want something. Precisely. I already have something in progress for that... > > - the tmp_page trick in anon_pipe_buf_release() seems to be unrelated to > > the splice() work. It should be a separate patch and any peformance > > testing (needed, please) should be decoupled from that change. > > It's not unrelated. Note the new "page_count() == 1" test. Yes, this is needed to make migrating pages from a pipe to the page cache possible. > > - The logic in do_splice() hurts my brain. "if `in' is a pipe then > > splice from `in-as-a-pipe' to `out' else if `out' is a pipe then splice > > from `in' to 'out-as-a-pipe'. Make sense, I guess, but I do wonder "what > > would happen if those tests were reversed?". Nothing, I guess. > > Why would it matter? If both are pipes, then one is as good as the other. > You just want to pick the version that is potentially more efficient, if > there is any difference (and there is). > > However, I don't think Jens actually did the pipe->pipe case at all (ie > pipes don't have the "splice_read()" function yet). No it's not there yet, coverage will increase soon :) > > > > - In pipe_to_file(): > > > > - Shouldn't it be using GFP_HIGHUSER()? > > Some filesystems may not like having highpages. > > I suspect it should be "mapping_gfp_mask(mapping)". I actually already made it GFP_HIGHUSER yesterday in a non-yet committed patch, so I'll check up on this and make the change. -- Jens Axboe - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ | ||||||||||
| Last update: 2006-03-30 09:18 [from the cache] ©2003-2008 | |||||||||||