[PATCH][2.6-mm] split drain_local_pages

[PATCH][2.6-mm] split drain_local_pages

[Zwane Mwaikambo]

CPU hotplug core needs to pass a cpu parameter to drain_local_pages, it's
safe to call __drain_local_pages if the cpu being drained is offline. The
semantics for drain_local_pages do not change.

Index: linux-2.6.3-rc3-mm1/mm/page_alloc.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.3-rc3-mm1/mm/page_alloc.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 page_alloc.c
--- linux-2.6.3-rc3-mm1/mm/page_alloc.c	16 Feb 2004 20:42:50 -0000	1.1.1.1
+++ linux-2.6.3-rc3-mm1/mm/page_alloc.c	16 Feb 2004 21:58:19 -0000
@@ -414,19 +414,19 @@ int is_head_of_free_region(struct page *
 }

 /*
- * Spill all of this CPU's per-cpu pages back into the buddy allocator.
+ * drain_local_pages helper, this is only safe to use when the cpu
+ * being drained isn't currently online.
  */
-void drain_local_pages(void)
+
+void __drain_local_pages(int cpu)
 {
-	unsigned long flags;
 	struct zone *zone;
 	int i;
-
-	local_irq_save(flags);
+
 	for_each_zone(zone) {
 		struct per_cpu_pageset *pset;

-		pset = &zone->pageset[smp_processor_id()];
+		pset = &zone->pageset[cpu];
 		for (i = 0; i < ARRAY_SIZE(pset->pcp); i++) {
 			struct per_cpu_pages *pcp;

@@ -435,7 +435,19 @@ void drain_local_pages(void)
 						&pcp->list, 0);
 		}
 	}
-	local_irq_restore(flags);
+}
+
+/*
+ * Spill all of this CPU's per-cpu pages back into the buddy allocator.
+ */
+
+void drain_local_pages(void)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	__drain_local_pages(smp_processor_id());
+	local_irq_restore(flags);
 }
 #endif /* CONFIG_PM */

@@ -1574,7 +1586,7 @@ static int page_alloc_cpu_notify(struct
 		count = &per_cpu(nr_pagecache_local, cpu);
 		atomic_add(*count, &nr_pagecache);
 		*count = 0;
-		drain_local_pages(cpu);
+		__drain_local_pages(cpu);
 	}
 	return NOTIFY_OK;
 }

Re: [PATCH][2.6-mm] split drain_local_pages

[Pavel Machek]

Hi!


> CPU hotplug core needs to pass a cpu parameter to drain_local_pages, it's
> safe to call __drain_local_pages if the cpu being drained is offline. The
> semantics for drain_local_pages do not change.

The idea looks good to me, but there's something wrong with the patch:

> Index: linux-2.6.3-rc3-mm1/mm/page_alloc.c
> ===================================================================
> RCS file: /home/cvsroot/linux-2.6.3-rc3-mm1/mm/page_alloc.c,v
> retrieving revision 1.1.1.1
> diff -u -p -B -r1.1.1.1 page_alloc.c
> --- linux-2.6.3-rc3-mm1/mm/page_alloc.c	16 Feb 2004 20:42:50 -0000	1.1.1.1
> +++ linux-2.6.3-rc3-mm1/mm/page_alloc.c	16 Feb 2004 21:58:19 -0000
> @@ -414,19 +414,19 @@ int is_head_of_free_region(struct page *
>  }
> 
>  /*
> - * Spill all of this CPU's per-cpu pages back into the buddy allocator.
> + * drain_local_pages helper, this is only safe to use when the cpu
> + * being drained isn't currently online.
>   */
> -void drain_local_pages(void)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It was void before...

> @@ -1574,7 +1586,7 @@ static int page_alloc_cpu_notify(struct
>  		count = &per_cpu(nr_pagecache_local, cpu);
>  		atomic_add(*count, &nr_pagecache);
>  		*count = 0;
> -		drain_local_pages(cpu);

...but code was passing cpu? The old version could not have compiled
according to the patch..
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: [PATCH][2.6-mm] split drain_local_pages

[Zwane Mwaikambo]

On Mon, 16 Feb 2004, Pavel Machek wrote:

> ...but code was passing cpu? The old version could not have compiled
> according to the patch..

No, it didn't compile, but the code wasn't enabled anyway, so that's ok.

Re: [PATCH][2.6-mm] split drain_local_pages

[Andrew Morton]

Pavel Machek <pavel@ucw.cz> wrote:
>
> The idea looks good to me, but there's something wrong with the patch:

Yes, it looks like Rusty fed me a load there.  Zwane's patch fixes it up
though.

There's no way of turning on the hotplug CPU code in 2.6.3-rc1-mm1, so I
assume we're awaiting the arch patches.

I made __drain_local_pages() static to page_alloc.c

Re: [PATCH][2.6-mm] split drain_local_pages

[Rusty Russell]

In message <20040216150654.38935399.akpm@osdl.org> you write:
> Pavel Machek <pavel@ucw.cz> wrote:
> >
> > The idea looks good to me, but there's something wrong with the patch:
> 
> Yes, it looks like Rusty fed me a load there.  Zwane's patch fixes it up
> though.

You knew that saying that would force me to figure out what happened,
didn't you?

Actually, looks like you removed cpuhotplug-02-drain_local_pages.patch.

This fix is fine though, and probably better.
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH][2.6-mm] split drain_local_pages

[Rusty Russell]

In message <Pine.LNX.4.58.0402161720390.11793@montezuma.fsmlabs.com> you write:
> CPU hotplug core needs to pass a cpu parameter to drain_local_pages, it's
> safe to call __drain_local_pages if the cpu being drained is offline. The
> semantics for drain_local_pages do not change.

I prefer this version: it gets the #ifdef correct too.

Name: Introduce __drain_pages() To Take a CPU Number
Author: Rusty Russell
Status: Booted on 2.6.2-rc2-bk2

Hotplug CPU needs to drain pages on a downed CPU (usually it's the
current cpu).  Introduce "__drain_pages", make the CPU an argument,
and expose it if CONFIG_HOTPLUG_CPU as well as CONFIG_PM.

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .26287-linux-2.6.3-rc3-bk1/mm/page_alloc.c .26287-linux-2.6.3-rc3-bk1.updated/mm/page_alloc.c
--- .26287-linux-2.6.3-rc3-bk1/mm/page_alloc.c	2004-02-15 18:17:22.000000000 +1100
+++ .26287-linux-2.6.3-rc3-bk1.updated/mm/page_alloc.c	2004-02-17 17:22:10.000000000 +1100
@@ -390,6 +390,27 @@ static int rmqueue_bulk(struct zone *zon
 	return allocated;
 }
 
+#if defined(CONFIG_PM) || defined(CONFIG_HOTPLUG_CPU)
+static void __drain_pages(unsigned int cpu)
+{
+	struct zone *zone;
+	int i;
+
+	for_each_zone(zone) {
+		struct per_cpu_pageset *pset;
+
+		pset = &zone->pageset[cpu];
+		for (i = 0; i < ARRAY_SIZE(pset->pcp); i++) {
+			struct per_cpu_pages *pcp;
+
+			pcp = &pset->pcp[i];
+			pcp->count -= free_pages_bulk(zone, pcp->count,
+						&pcp->list, 0);
+		}
+	}
+}
+#endif /* CONFIG_PM || CONFIG_HOTPLUG_CPU */
+
 #ifdef CONFIG_PM
 int is_head_of_free_region(struct page *page)
 {
@@ -419,22 +440,9 @@ int is_head_of_free_region(struct page *
 void drain_local_pages(void)
 {
 	unsigned long flags;
-	struct zone *zone;
-	int i;
 
 	local_irq_save(flags);	
-	for_each_zone(zone) {
-		struct per_cpu_pageset *pset;
-
-		pset = &zone->pageset[smp_processor_id()];
-		for (i = 0; i < ARRAY_SIZE(pset->pcp); i++) {
-			struct per_cpu_pages *pcp;
-
-			pcp = &pset->pcp[i];
-			pcp->count -= free_pages_bulk(zone, pcp->count,
-						&pcp->list, 0);
-		}
-	}
+	__drain_pages(smp_processor_id());
 	local_irq_restore(flags);	
 }
 #endif /* CONFIG_PM */
@@ -1574,7 +1586,7 @@ static int page_alloc_cpu_notify(struct
 		count = &per_cpu(nr_pagecache_local, cpu);
 		atomic_add(*count, &nr_pagecache);
 		*count = 0;
-		drain_local_pages(cpu);
+		__drain_pages(cpu);
 	}
 	return NOTIFY_OK;
 }

--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH][2.6-mm] split drain_local_pages

[Zwane Mwaikambo]

On Tue, 17 Feb 2004, Rusty Russell wrote:

> In message <Pine.LNX.4.58.0402161720390.11793@montezuma.fsmlabs.com> you write:
> > CPU hotplug core needs to pass a cpu parameter to drain_local_pages, it's
> > safe to call __drain_local_pages if the cpu being drained is offline. The
> > semantics for drain_local_pages do not change.
>
> I prefer this version: it gets the #ifdef correct too.

Indeed, i favour your version too.