Re: [PATCH] splice: add support for SPLICE_F_MOVE flag

[Jens Axboe <axboe@suse.de>]

On Thu, Mar 30 2006, Andrew Morton wrote:
> Andrew Morton <akpm@osdl.org> wrote:
> >
> >   static void page_cache_pipe_buf_unmap(struct pipe_inode_info *info,
> >  >  				      struct pipe_buffer *buf)
> >  >  {
> >  > -	unlock_page(buf->page);
> >  > +	if (!buf->stolen)
> >  > +		unlock_page(buf->page);
> >  >  	kunmap(buf->page);
> >  >  }
> > 
> >  There go our chances of ever getting rid of kmap().  Is it not feasible to
> >  use atomic kmaps throughout this code?
> 
> What are the kmaps for, anyway?  afaict they're doing the
> kmap-the-page-while-we-run-some-a_ops thing which ceased being a
> requirement 3-4 years ago.

Not quite so, it's because the ->map serve as both a pin and address map
helper. The splice stuff doesn't actually need the kmap(), only for the
case where we have to copy the page. And it would be just as easy to
map that ourselves in that case.

There's a comment about that in the splice file.

> The general approach we should take is that the code which actually
> modifies a page's contents is the code which is responsible for kmapping
> that page.  Use an atomic kmap, memcpy-or-memset, atomic kunmap.  Just four
> or five lines.
> 
> If we can do that, pipe_buf_operations.map/unmap can be removed.

The page cache stuff still needs to sanity check the page, before
allowing the caller to map it, so it cannot go. But it can be optimized.

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