Messages in this thread | | | Date | Fri, 31 Mar 2006 09:08:14 +0200 | From | Jens Axboe <> | Subject | Re: [PATCH] splice: add support for SPLICE_F_MOVE flag |
| |
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/
| |