2011-02-10 16:34:16

by Minchan Kim

[permalink] [raw]
Subject: [PATCH] mm: optimize replace_page_cache_page

This patch optmizes replace_page_cache_page.

1) remove radix_tree_preload
2) single radix_tree_lookup_slot and replace radix tree slot
3) page accounting optimization if both pages are in same zone.

Cc: Miklos Szeredi <[email protected]>
Cc: Rik van Riel <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Cc: Mel Gorman <[email protected]>
Signed-off-by: Minchan Kim <[email protected]>
---
mm/filemap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 51 insertions(+), 10 deletions(-)

Hi Miklos,
This patch is totally not tested.
Could you test this patch?

diff --git a/mm/filemap.c b/mm/filemap.c
index a25c898..918ef1e 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -148,6 +148,56 @@ void __remove_from_page_cache(struct page *page)
}
}

+/*
+ * Replace a page from the page cache with a new one.
+ * Both the old and new page must be locked.
+ * The caller must hold the mapping's tree_lock.
+ */
+void __replace_page_cache_page(struct page *old, struct page *new)
+{
+ void **pslot;
+ struct address_space *mapping = old->mapping;
+ struct zone *old_zone, *new_zone;
+
+ old_zone = page_zone(old);
+ new_zone = page_zone(new);
+ /*
+ * if we're uptodate, flush out into the cleancache, otherwise
+ * invalidate any existing cleancache entries. We can't leave
+ * stale data around in the cleancache once our page is gone
+ */
+ if (PageUptodate(old))
+ cleancache_put_page(old);
+ else
+ cleancache_flush_page(mapping, old);
+
+ pslot = radix_tree_lookup_slot(&mapping->page_tree,
+ page_index(old));
+ get_page(new); /* add cache reference */
+ radix_tree_replace_slot(pslot, new);
+ old->mapping = NULL;
+ if (old_zone != new_zone) {
+ __dec_zone_page_state(old, NR_FILE_PAGES);
+ __inc_zone_page_state(new, NR_FILE_PAGES);
+ if (PageSwapBacked(old)) {
+ __dec_zone_page_state(old, NR_SHMEM);
+ __inc_zone_page_state(new, NR_SHMEM);
+ }
+ }
+ BUG_ON(page_mapped(old));
+ /*
+ * Some filesystems seem to re-dirty the page even after
+ * the VM has canceled the dirty bit (eg ext3 journaling).
+ *
+ * Fix it up by doing a final dirty accounting check after
+ * having removed the page entirely.
+ */
+ if (PageDirty(old) && mapping_cap_account_dirty(mapping)) {
+ dec_zone_page_state(old, NR_FILE_DIRTY);
+ dec_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
+ }
+}
+
void remove_from_page_cache(struct page *page)
{
struct address_space *mapping = page->mapping;
@@ -433,7 +483,6 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask)
if (error)
return error;

- error = radix_tree_preload(gfp_mask & ~__GFP_HIGHMEM);
if (!error) {
struct address_space *mapping = old->mapping;
void (*freepage)(struct page *);
@@ -441,20 +490,12 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask)
pgoff_t offset = old->index;
freepage = mapping->a_ops->freepage;

- page_cache_get(new);
new->mapping = mapping;
new->index = offset;

spin_lock_irq(&mapping->tree_lock);
- __remove_from_page_cache(old);
- error = radix_tree_insert(&mapping->page_tree, offset, new);
- BUG_ON(error);
- mapping->nrpages++;
- __inc_zone_page_state(new, NR_FILE_PAGES);
- if (PageSwapBacked(new))
- __inc_zone_page_state(new, NR_SHMEM);
+ __replace_page_cache_page(old, new);
spin_unlock_irq(&mapping->tree_lock);
- radix_tree_preload_end();
if (freepage)
freepage(old);
page_cache_release(old);
--
1.7.1


2011-02-19 23:41:32

by Minchan Kim

[permalink] [raw]
Subject: Re: [PATCH] mm: optimize replace_page_cache_page

Resend.

he patch is based on mmotm-2011-02-04 +
mm-add-replace_page_cache_page-function-add-freepage-hook.patch.

On Fri, Feb 11, 2011 at 01:33:46AM +0900, Minchan Kim wrote:
> This patch optmizes replace_page_cache_page.
>
> 1) remove radix_tree_preload
> 2) single radix_tree_lookup_slot and replace radix tree slot
> 3) page accounting optimization if both pages are in same zone.
>
> Cc: Miklos Szeredi <[email protected]>
> Cc: Rik van Riel <[email protected]>
> Cc: KAMEZAWA Hiroyuki <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Signed-off-by: Minchan Kim <[email protected]>
> ---
> mm/filemap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++---------
> 1 files changed, 51 insertions(+), 10 deletions(-)
>
> Hi Miklos,
> This patch is totally not tested.
> Could you test this patch?
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index a25c898..918ef1e 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -148,6 +148,56 @@ void __remove_from_page_cache(struct page *page)
> }
> }
>
> +/*
> + * Replace a page from the page cache with a new one.
> + * Both the old and new page must be locked.
> + * The caller must hold the mapping's tree_lock.
> + */
> +void __replace_page_cache_page(struct page *old, struct page *new)
> +{
> + void **pslot;
> + struct address_space *mapping = old->mapping;
> + struct zone *old_zone, *new_zone;
> +
> + old_zone = page_zone(old);
> + new_zone = page_zone(new);
> + /*
> + * if we're uptodate, flush out into the cleancache, otherwise
> + * invalidate any existing cleancache entries. We can't leave
> + * stale data around in the cleancache once our page is gone
> + */
> + if (PageUptodate(old))
> + cleancache_put_page(old);
> + else
> + cleancache_flush_page(mapping, old);
> +
> + pslot = radix_tree_lookup_slot(&mapping->page_tree,
> + page_index(old));
> + get_page(new); /* add cache reference */
> + radix_tree_replace_slot(pslot, new);
> + old->mapping = NULL;
> + if (old_zone != new_zone) {
> + __dec_zone_page_state(old, NR_FILE_PAGES);
> + __inc_zone_page_state(new, NR_FILE_PAGES);
> + if (PageSwapBacked(old)) {
> + __dec_zone_page_state(old, NR_SHMEM);
> + __inc_zone_page_state(new, NR_SHMEM);
> + }
> + }
> + BUG_ON(page_mapped(old));
> + /*
> + * Some filesystems seem to re-dirty the page even after
> + * the VM has canceled the dirty bit (eg ext3 journaling).
> + *
> + * Fix it up by doing a final dirty accounting check after
> + * having removed the page entirely.
> + */
> + if (PageDirty(old) && mapping_cap_account_dirty(mapping)) {
> + dec_zone_page_state(old, NR_FILE_DIRTY);
> + dec_bdi_stat(mapping->backing_dev_info, BDI_RECLAIMABLE);
> + }
> +}
> +
> void remove_from_page_cache(struct page *page)
> {
> struct address_space *mapping = page->mapping;
> @@ -433,7 +483,6 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask)
> if (error)
> return error;
>
> - error = radix_tree_preload(gfp_mask & ~__GFP_HIGHMEM);
> if (!error) {
> struct address_space *mapping = old->mapping;
> void (*freepage)(struct page *);
> @@ -441,20 +490,12 @@ int replace_page_cache_page(struct page *old, struct page *new, gfp_t gfp_mask)
> pgoff_t offset = old->index;
> freepage = mapping->a_ops->freepage;
>
> - page_cache_get(new);
> new->mapping = mapping;
> new->index = offset;
>
> spin_lock_irq(&mapping->tree_lock);
> - __remove_from_page_cache(old);
> - error = radix_tree_insert(&mapping->page_tree, offset, new);
> - BUG_ON(error);
> - mapping->nrpages++;
> - __inc_zone_page_state(new, NR_FILE_PAGES);
> - if (PageSwapBacked(new))
> - __inc_zone_page_state(new, NR_SHMEM);
> + __replace_page_cache_page(old, new);
> spin_unlock_irq(&mapping->tree_lock);
> - radix_tree_preload_end();
> if (freepage)
> freepage(old);
> page_cache_release(old);
> --
> 1.7.1
>

--
Kind regards,
Minchan Kim

2011-02-23 22:46:01

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] mm: optimize replace_page_cache_page

On Sun, 20 Feb 2011 08:41:21 +0900
Minchan Kim <[email protected]> wrote:

> Resend.

Reignore.

> he patch is based on mmotm-2011-02-04 +
> mm-add-replace_page_cache_page-function-add-freepage-hook.patch.
>
> On Fri, Feb 11, 2011 at 01:33:46AM +0900, Minchan Kim wrote:
> > This patch optmizes replace_page_cache_page.
> >
> > 1) remove radix_tree_preload
> > 2) single radix_tree_lookup_slot and replace radix tree slot
> > 3) page accounting optimization if both pages are in same zone.
> >
> > Cc: Miklos Szeredi <[email protected]>
> > Cc: Rik van Riel <[email protected]>
> > Cc: KAMEZAWA Hiroyuki <[email protected]>
> > Cc: Mel Gorman <[email protected]>
> > Signed-off-by: Minchan Kim <[email protected]>
> > ---
> > mm/filemap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++---------
> > 1 files changed, 51 insertions(+), 10 deletions(-)
> >
> > Hi Miklos,
> > This patch is totally not tested.
> > Could you test this patch?

^^^ Because of this.

Is it tested yet?

2011-02-23 23:37:29

by Minchan Kim

[permalink] [raw]
Subject: Re: [PATCH] mm: optimize replace_page_cache_page

On Thu, Feb 24, 2011 at 7:44 AM, Andrew Morton
<[email protected]> wrote:
> On Sun, 20 Feb 2011 08:41:21 +0900
> Minchan Kim <[email protected]> wrote:
>
>> Resend.
>
> Reignore.
>
>> he patch is based on mmotm-2011-02-04 +
>> mm-add-replace_page_cache_page-function-add-freepage-hook.patch.
>>
>> On Fri, Feb 11, 2011 at 01:33:46AM +0900, Minchan Kim wrote:
>> > This patch optmizes replace_page_cache_page.
>> >
>> > 1) remove radix_tree_preload
>> > 2) single radix_tree_lookup_slot and replace radix tree slot
>> > 3) page accounting optimization if both pages are in same zone.
>> >
>> > Cc: Miklos Szeredi <[email protected]>
>> > Cc: Rik van Riel <[email protected]>
>> > Cc: KAMEZAWA Hiroyuki <[email protected]>
>> > Cc: Mel Gorman <[email protected]>
>> > Signed-off-by: Minchan Kim <[email protected]>
>> > ---
>> >  mm/filemap.c |   61 ++++++++++++++++++++++++++++++++++++++++++++++++---------
>> >  1 files changed, 51 insertions(+), 10 deletions(-)
>> >
>> > Hi Miklos,
>> > This patch is totally not tested.
>> > Could you test this patch?
>
> ^^^ Because of this.
>
> Is it tested yet?
>

Miklos. Could you test this?
If you are busy, let me know how to test it. I will.
Thanks.


--
Kind regards,
Minchan Kim

2011-02-24 10:57:06

by Miklos Szeredi

[permalink] [raw]
Subject: Re: [PATCH] mm: optimize replace_page_cache_page

On Thu, 2011-02-24 at 08:37 +0900, Minchan Kim wrote:
> On Thu, Feb 24, 2011 at 7:44 AM, Andrew Morton
> <[email protected]> wrote:
> > On Sun, 20 Feb 2011 08:41:21 +0900
> > Minchan Kim <[email protected]> wrote:
> >
> >> Resend.
> >
> > Reignore.
> >
> >> he patch is based on mmotm-2011-02-04 +
> >> mm-add-replace_page_cache_page-function-add-freepage-hook.patch.
> >>
> >> On Fri, Feb 11, 2011 at 01:33:46AM +0900, Minchan Kim wrote:
> >> > This patch optmizes replace_page_cache_page.
> >> >
> >> > 1) remove radix_tree_preload
> >> > 2) single radix_tree_lookup_slot and replace radix tree slot
> >> > 3) page accounting optimization if both pages are in same zone.
> >> >
> >> > Cc: Miklos Szeredi <[email protected]>
> >> > Cc: Rik van Riel <[email protected]>
> >> > Cc: KAMEZAWA Hiroyuki <[email protected]>
> >> > Cc: Mel Gorman <[email protected]>
> >> > Signed-off-by: Minchan Kim <[email protected]>
> >> > ---
> >> > mm/filemap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++---------
> >> > 1 files changed, 51 insertions(+), 10 deletions(-)
> >> >
> >> > Hi Miklos,
> >> > This patch is totally not tested.
> >> > Could you test this patch?
> >
> > ^^^ Because of this.
> >
> > Is it tested yet?
> >
>
> Miklos. Could you test this?
> If you are busy, let me know how to test it. I will.
> Thanks.

Grab git version of libfuse and do something like this:

fuse/example/fusexmp_fh -obig_writes /mnt/fuse/
dd if=/tmp/random0 of=/mnt/fuse/tmp/random1 bs=1M
md5sum /tmp/radom0 /tmp/random1

This should exercise the page moving.

I'll review and test the patch when I have some time.

Thanks,
Miklos

2011-03-21 22:07:30

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] mm: optimize replace_page_cache_page

On Thu, 24 Feb 2011 11:56:55 +0100
Miklos Szeredi <[email protected]> wrote:

> On Thu, 2011-02-24 at 08:37 +0900, Minchan Kim wrote:
> > On Thu, Feb 24, 2011 at 7:44 AM, Andrew Morton
> > <[email protected]> wrote:
> > > On Sun, 20 Feb 2011 08:41:21 +0900
> > > Minchan Kim <[email protected]> wrote:
> > >
> > >> Resend.
> > >
> > > Reignore.
> > >
> > >> he patch is based on mmotm-2011-02-04 +
> > >> mm-add-replace_page_cache_page-function-add-freepage-hook.patch.
> > >>
> > >> On Fri, Feb 11, 2011 at 01:33:46AM +0900, Minchan Kim wrote:
> > >> > This patch optmizes replace_page_cache_page.
> > >> >
> > >> > 1) remove radix_tree_preload
> > >> > 2) single radix_tree_lookup_slot and replace radix tree slot
> > >> > 3) page accounting optimization if both pages are in same zone.
> > >> >
> > >> > Cc: Miklos Szeredi <[email protected]>
> > >> > Cc: Rik van Riel <[email protected]>
> > >> > Cc: KAMEZAWA Hiroyuki <[email protected]>
> > >> > Cc: Mel Gorman <[email protected]>
> > >> > Signed-off-by: Minchan Kim <[email protected]>
> > >> > ---
> > >> > mm/filemap.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++---------
> > >> > 1 files changed, 51 insertions(+), 10 deletions(-)
> > >> >
> > >> > Hi Miklos,
> > >> > This patch is totally not tested.
> > >> > Could you test this patch?
> > >
> > > ^^^ Because of this.
> > >
> > > Is it tested yet?
> > >
> >
> > Miklos. Could you test this?
> > If you are busy, let me know how to test it. I will.
> > Thanks.
>
> Grab git version of libfuse and do something like this:
>
> fuse/example/fusexmp_fh -obig_writes /mnt/fuse/
> dd if=/tmp/random0 of=/mnt/fuse/tmp/random1 bs=1M
> md5sum /tmp/radom0 /tmp/random1
>
> This should exercise the page moving.
>
> I'll review and test the patch when I have some time.
>

Too late. It took months to half-finish Miklos's patch and now I'm
sitting on other patches which depend on it, so it's going in.