Re: [PATCH] splice SPLICE_F_MOVE support

Nick Piggin
2006-03-31T18:45:50Z

[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]

Jens Axboe wrote:
> Hi,
>
> This applies on top of the splice #3 just posted, adding support for
> moving of pages. The caller can use the SPLICE_F_MOVE flag to the splice
> syscall to ask the kernel to try and move pages, if needed.
>
> Disclaimer: this works for me, but may have vm issues that I missed.
> CC'ing Nick :-)
>

Like Andrew said, you can't check PageLRU without holding zone->lru_lock.
The page release code can get away with it only because the page refcount
is 0 at that point. Also, you can't reliably remove pages from the LRU
unless the refcount is 0. Ever.

The following (untested) is something like what I had in mind, and should
get stealing closer to working. I've only given it a quick review so far
(btw. why do you only unlock the page if it hasn't been stolen?)

With this patch, the ->steal will indicate if the page had been on the
LRU or not. If not, then add it; if yes, then do nothing.

There is no caller of ->steal yet that wants the page off the LRU (is
there?). That's a bit harder.

---

[-- Attachment #2: splice-fix-lruops.patch --]
[-- Type: text/plain, Size: 3251 bytes --]

Index: linux-2.6/fs/pipe.c
===================================================================
--- linux-2.6.orig/fs/pipe.c
+++ linux-2.6/fs/pipe.c
@@ -124,7 +124,7 @@ static void anon_pipe_buf_unmap(struct p
static int anon_pipe_buf_steal(struct pipe_inode_info *info,
struct pipe_buffer *buf)
{
- buf->stolen = 1;
+ buf->flags |= PIPE_BUF_FLAG_STOLEN;
return 0;
}

Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c
+++ linux-2.6/fs/splice.c
@@ -45,17 +45,8 @@ static int page_cache_pipe_buf_steal(str
if (!remove_mapping(page_mapping(page), page))
return 1;

- if (PageLRU(page)) {
- struct zone *zone = page_zone(page);
+ buf->flags |= PIPE_BUF_FLAG_STOLEN | PIPE_BUF_FLAG_LRU;

- spin_lock_irq(&zone->lru_lock);
- BUG_ON(!PageLRU(page));
- __ClearPageLRU(page);
- del_page_from_lru(zone, page);
- spin_unlock_irq(&zone->lru_lock);
- }
-
- buf->stolen = 1;
return 0;
}

@@ -64,7 +55,7 @@ static void page_cache_pipe_buf_release(
{
page_cache_release(buf->page);
buf->page = NULL;
- buf->stolen = 0;
+ buf->flags &= ~(PIPE_BUF_FLAG_STOLEN|PIPE_BUF_FLAG_LRU);
}

static void *page_cache_pipe_buf_map(struct file *file,
@@ -91,7 +82,7 @@ static void *page_cache_pipe_buf_map(str
static void page_cache_pipe_buf_unmap(struct pipe_inode_info *info,
struct pipe_buffer *buf)
{
- if (!buf->stolen)
+ if (!(buf->flags & PIPE_BUF_FLAG_STOLEN))
unlock_page(buf->page);
kunmap(buf->page);
}
@@ -386,11 +377,13 @@ static int pipe_to_file(struct pipe_inod
if (sd->flags & SPLICE_F_MOVE) {
if (buf->ops->steal(info, buf))
goto find_page;
-
page = buf->page;
- if (add_to_page_cache_lru(page, mapping, index,
- mapping_gfp_mask(mapping)))
+ if (add_to_page_cache(page, mapping, index,
+ mapping_gfp_mask(mapping)))
goto find_page;
+
+ if (!(buf->flags & PIPE_BUF_FLAG_LRU))
+ lru_cache_add(page);
} else {
find_page:
ret = -ENOMEM;
@@ -435,7 +428,7 @@ find_page:
if (ret)
goto out;

- if (!buf->stolen) {
+ if (!(buf->flags & PIPE_BUF_FLAG_STOLEN)) {
char *dst = kmap_atomic(page, KM_USER0);

memcpy(dst + offset, src + buf->offset, sd->len);
@@ -452,7 +445,7 @@ find_page:
out:
if (ret < 0)
unlock_page(page);
- if (!buf->stolen)
+ if (!(buf->flags & PIPE_BUF_FLAG_STOLEN))
page_cache_release(page);
buf->ops->unmap(info, buf);
return ret;
Index: linux-2.6/include/linux/pipe_fs_i.h
===================================================================
--- linux-2.6.orig/include/linux/pipe_fs_i.h
+++ linux-2.6/include/linux/pipe_fs_i.h
@@ -5,11 +5,14 @@

#define PIPE_BUFFERS (16)

+#define PIPE_BUF_FLAG_STOLEN 0x01
+#define PIPE_BUF_FLAG_LRU 0x02
+
struct pipe_buffer {
struct page *page;
unsigned int offset, len;
struct pipe_buf_operations *ops;
- unsigned int stolen;
+ unsigned int flags;
};

struct pipe_buf_operations {
Index: linux-2.6/mm/swap.c
===================================================================
--- linux-2.6.orig/mm/swap.c
+++ linux-2.6/mm/swap.c
@@ -177,7 +177,7 @@ void lru_add_drain(void)
put_cpu();
}

-#ifdef CONFIG_NUMA
+#ifdef CONFIG_SMP
static void lru_add_drain_per_cpu(void *dummy)
{
lru_add_drain();