(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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
