Messages in this thread | | | Date | Mon, 1 May 2006 10:59:53 +0400 | From | Oleg Nesterov <> | Subject | splice(SPLICE_F_MOVE) problems |
| |
I noticed sys_splice() and friends were added. Cool! But I can't understand how SPLICE_F_MOVE is supposed to work.
pipe_to_file:
if (sd->flags & SPLICE_F_MOVE) {
if (buf->ops->steal(info, buf)) goto find_page;
Let's suppose that buf->ops == page_cache_pipe_buf_ops. page_cache_pipe_buf_steal() returns PG_locked page, why?
page = buf->page; if (add_to_page_cache(page, mapping, index, gfp_mask))
This adds entire page to page cache. What about partial pages? This can corrupt sd->file if offset != 0 || this_len != PAGE_SIZE.
goto find_page;
Ok, add_to_page_cache() failed. 'page' is still locked. It will be released later, this should trigger bad_page().
Also, we don't clear PIPE_BUF_FLAG_STOLEN, so we will miss the data copying and page_cache_release(page) below:
if (!(buf->flags & PIPE_BUF_FLAG_STOLEN)) { char *dst = kmap_atomic(page, KM_USER0);
memcpy(dst + offset, src + buf->offset, this_len); flush_dcache_page(page); kunmap_atomic(dst, KM_USER0); }
I can't understand why do we need PIPE_BUF_FLAG_STOLEN at all. It seems to me we need a local boolean in pipe_to_file.
I downloaded splice-git-20060430152503.tar.gz, but was unable to demonstrate these problems until I found that this definition
static inline int splice(int fdin, loff_t *off_in, int fdout, loff_t *off_out, size_t len, unsigned long flags) { return syscall(__NR_splice, fdin, off_in, fdout, off_out, len, flags); }
is not correct. At least on i386 you need _syscall6() here.
Oleg.
- 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/
| |