Messages in this thread Patch in this message | | | Date | Tue, 27 Mar 2007 10:47:54 -0700 | From | "Ken Chen" <> | Subject | Re: [patch] cache pipe buf page address for non-highmem arch |
| |
On 3/26/07, Andrew Morton <akpm@linux-foundation.org> wrote: > LTP's vmsplice01 triggers the below: > > Unable to handle kernel NULL pointer dereference at 0000000000000130 RIP: > [<ffffffff8029e1b6>] pipe_to_file+0x1f3/0x2a6
Ouch, generic_pipe_buf_map() and unmap is used by both pipe and splice, I better constrain all changes within fs/pipe.c because from splice path, the page->private is not initialized.
Double ouch for missing test that code path. Here is an updated patch, tested with LTP and FIO. I will write more test cases to make sure all code path are covered.
diff --git a/fs/pipe.c b/fs/pipe.c index ebafde7..4b55452 100644 --- a/fs/pipe.c +++ b/fs/pipe.c @@ -21,6 +21,21 @@ #include <asm/uaccess.h> #include <asm/ioctls.h>
+#ifdef CONFIG_HIGHMEM +#define pipe_kmap kmap +#define pipe_kmap_atomic kmap_atomic +#define pipe_kunmap kunmap +#define pipe_kunmap_atomic kunmap_atomic +#else /* CONFIG_HIGHMEM */ +static inline void *pipe_kmap(struct page *page) +{ + return (void *) page->private; +} +#define pipe_kmap_atomic(page, type) pipe_kmap(page) +#define pipe_kunmap(page) do { } while (0) +#define pipe_kunmap_atomic(page, type) do { } while (0) +#endif + /* * We use a start+len construction, which provides full use of the * allocated memory. @@ -185,6 +200,27 @@ void generic_pipe_buf_unmap kunmap(buf->page); }
+static void *pipe_buf_map(struct pipe_inode_info *pipe, + struct pipe_buffer *buf, int atomic) +{ + if (atomic) { + buf->flags |= PIPE_BUF_FLAG_ATOMIC; + return pipe_kmap_atomic(buf->page, KM_USER0); + } + + return pipe_kmap(buf->page); +} + +static void pipe_buf_unmap(struct pipe_inode_info *pipe, + struct pipe_buffer *buf, void *map_data) +{ + if (buf->flags & PIPE_BUF_FLAG_ATOMIC) { + buf->flags &= ~PIPE_BUF_FLAG_ATOMIC; + pipe_kunmap_atomic(map_data, KM_USER0); + } else + pipe_kunmap(buf->page); +} + int generic_pipe_buf_steal(struct pipe_inode_info *pipe, struct pipe_buffer *buf) { @@ -210,8 +246,8 @@ int generic_pipe_buf_pin
static const struct pipe_buf_operations anon_pipe_buf_ops = { .can_merge = 1, - .map = generic_pipe_buf_map, - .unmap = generic_pipe_buf_unmap, + .map = pipe_buf_map, + .unmap = pipe_buf_unmap, .pin = generic_pipe_buf_pin, .release = anon_pipe_buf_release, .steal = generic_pipe_buf_steal, @@ -316,6 +352,7 @@ redo: if (do_wakeup) { wake_up_interruptible_sync(&pipe->wait); kill_fasync(&pipe->fasync_writers, SIGIO, POLL_OUT); + do_wakeup = 0; } pipe_wait(pipe); } @@ -423,6 +460,8 @@ redo1: ret = ret ? : -ENOMEM; break; } + page->private = (unsigned long) + page_address(page); pipe->tmp_page = page; } /* Always wake up, even if the copy fails. Otherwise @@ -438,16 +477,16 @@ redo1: iov_fault_in_pages_read(iov, chars); redo2: if (atomic) - src = kmap_atomic(page, KM_USER0); + src = pipe_kmap_atomic(page, KM_USER0); else - src = kmap(page); + src = pipe_kmap(page);
error = pipe_iov_copy_from_user(src, iov, chars, atomic); if (atomic) - kunmap_atomic(src, KM_USER0); + pipe_kunmap_atomic(src, KM_USER0); else - kunmap(page); + pipe_kunmap(page);
if (unlikely(error)) { if (atomic) { - 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/
| |