SMP support for drain local pages.

PATCH In review Nigel Cunningham 4 participants 10 messages May 26, 2004 – May 27, 2004

Subsystems (2)

Memory management Page allocator

lore

Share & tools

lore

Nigel Cunningham

author

2004-05-26 22:22 UTC #0 lore raw

PATCH 1/1 SMP support for drain local pages.

From: Nigel Cunningham <[email protected]>

Hi.

This patch adds SMP support for drain_local_pages, so that suspend implementations can drain pcp structures on all CPUs and thus accurately determine which pages are free.

Please apply.

mm/page_alloc.c 2004-05-26 19:56:19.000000000 +1000 renamed from mm/page_alloc.c 2004-05-26 19:47:15.000000000 +1000 +18 -0

diff -ruN 2.6.6-current-bk/mm/page_alloc.c smp-drain-local-pages/mm/page_alloc.c
@@ -459,6 +459,24 @@
        __drain_pages(smp_processor_id());
        local_irq_restore(flags);
 }
+
+#ifdef CONFIG_SMP
+static void __smp_drain_local_pages(void * data)
+{
+       drain_local_pages();
+}
+
+void smp_drain_local_pages(void)
+{
+       smp_call_function(__smp_drain_local_pages, NULL, 0, 1);
+       drain_local_pages();
+}
+#else
+void smp_drain_local_pages(void)
+{
+       drain_local_pages();
+}
+#endif
 #endif /* CONFIG_PM */
 
 static void zone_statistics(struct zonelist *zonelist, struct zone *z)

Dave Jones

2004-05-26 22:34 UTC #1 lore raw

Quoted 8 lines · Nigel Cunningham

mm/page_alloc.c 2004-05-26 19:56:19.000000000 +1000 renamed from mm/page_alloc.c 2004-05-26 19:47:15.000000000 +1000 +8 -0
+#ifdef CONFIG_SMP
+static void __smp_drain_local_pages(void * data)
+{
+       drain_local_pages();
+}
+
+void smp_drain_local_pages(void)
+{

mm/page_alloc.c 2004-05-26 19:56:19.000000000 +1000 line 480

Quoted 10 lines

mm/page_alloc.c 2004-05-26 19:56:19.000000000 +1000 renamed from mm/page_alloc.c 2004-05-26 19:47:15.000000000 +1000 +9 -0
+       smp_call_function(__smp_drain_local_pages, NULL, 0, 1);
+       drain_local_pages();
+}
+#else
+void smp_drain_local_pages(void)
+{
+       drain_local_pages();
+}
+#endif
 #endif /* CONFIG_PM */

if you use on_each_cpu() you can do away with the ifdef. It also takes care of preemption issues.

Dave

Nigel Cunningham

author

2004-05-26 23:02 UTC #2 lore raw

Re: [PATCH] SMP support for drain local pages v2.

Thanks Dave for educating the ignorant :>.

Is this better?

mm/page_alloc.c 2004-05-27 08:50:36.000000000 +1000 renamed from mm/page_alloc.c 2004-05-26 19:47:15.000000000 +1000 +14 -0

@@ -459,6 +459,20 @@
        __drain_pages(smp_processor_id());
        local_irq_restore(flags);
 }
+
+/*
+ * Spill per-cpu pages on all CPUs back into the buddy allocator.
+ * The first function is just to avoid a compiler warning.
+ */
+static void __smp_drain_local_pages(void * data)
+{
+       drain_local_pages();
+}
+
+void smp_drain_local_pages(void)
+{
+       on_each_cpu(__smp_drain_local_pages, NULL, 0, 1);
+}
 #endif /* CONFIG_PM */
 
 static void zone_statistics(struct zonelist *zonelist, struct zone *z)

Andrew Morton

2004-05-26 23:23 UTC #3 lore raw

Re: [PATCH] SMP support for drain local pages v2.

Quoted 4 lines · Nigel Cunningham

mm/page_alloc.c 2004-05-27 08:50:36.000000000 +1000 renamed from mm/page_alloc.c 2004-05-26 19:47:15.000000000 +1000 +3 -0
+/*
+ * Spill per-cpu pages on all CPUs back into the buddy allocator.
+ * The first function is just to avoid a compiler warning.

mm/page_alloc.c 2004-05-27 08:50:36.000000000 +1000 line 475

Quoted 10 lines

mm/page_alloc.c 2004-05-27 08:50:36.000000000 +1000 renamed from mm/page_alloc.c 2004-05-26 19:47:15.000000000 +1000 +10 -0
+ */
+static void __smp_drain_local_pages(void * data)
+{
+       drain_local_pages();
+}
+
+void smp_drain_local_pages(void)
+{
+       on_each_cpu(__smp_drain_local_pages, NULL, 0, 1);
+}

I think we only need a single entry point. Make it a new "drain_percpu_pages()" or such to break unconverted callers, switch callers of drain_local_pages()
over to the new function. It needs no SMP ifdefs in it - on_each_cpu() will
do the right thing on UP.

But until something which needs this change is merged into the tree I'd say that this patch should live with the patch which requires it.

Nigel Cunningham

author

2004-05-26 23:44 UTC #4 lore raw

Re: [PATCH] SMP support for drain local pages v2.

Hi.

Quoted 7 lines · Andrew Morton

I think we only need a single entry point. Make it a new "drain_percpu_pages()" or such to break unconverted callers, switch callers of drain_local_pages()
over to the new function. It needs no SMP ifdefs in it - on_each_cpu() will
do the right thing on UP.
But until something which needs this change is merged into the tree I'd say that this patch should live with the patch which requires it.

CPU hotplugging uses drain_local_pages, and shouldn't drain the pcp lists for all cpus. That's why I left the original version alone.

I'm submitting it now in preparation for merging, but Pavel's work on SMP support for swsusp should be using this too. (It's not, but it should be).

Nigel

Signature · 7 lines

Nigel & Michelle Cunningham
C/- Westminster Presbyterian Church Belconnen
61 Templeton Street, Cook, ACT 2614.
+61 (2) 6251 7727(wk); +61 (2) 6254 0216 (home)

Evolution (n): A hypothetical process whereby infinitely improbable events occur
with alarming frequency, order arises from chaos, and no one is given credit.

Andrew Morton

2004-05-26 23:54 UTC #5 lore raw

Re: [PATCH] SMP support for drain local pages v2.

Quoted 9 lines · Nigel Cunningham

Hi.
Quoted 5 lines · Andrew Morton
I think we only need a single entry point. Make it a new "drain_percpu_pages()" or such to break unconverted callers, switch callers of drain_local_pages()
over to the new function. It needs no SMP ifdefs in it - on_each_cpu() will
do the right thing on UP.

Quoted 4 lines

Quoted 2 lines
But until something which needs this change is merged into the tree I'd say that this patch should live with the patch which requires it.
CPU hotplugging uses drain_local_pages,

Early versions did, but not now.

Nigel Cunningham

author

2004-05-27 00:02 UTC #6 lore raw

Re: [PATCH] SMP support for drain local pages v2.

Quoted 3 lines · Andrew Morton

Quoted 1 line · Nigel Cunningham
CPU hotplugging uses drain_local_pages,
Early versions did, but not now.

Oh. Okay. :>

Nigel

Signature · 7 lines

Nigel & Michelle Cunningham
C/- Westminster Presbyterian Church Belconnen
61 Templeton Street, Cook, ACT 2614.
+61 (2) 6251 7727(wk); +61 (2) 6254 0216 (home)

Evolution (n): A hypothetical process whereby infinitely improbable events occur
with alarming frequency, order arises from chaos, and no one is given credit.

Pavel Machek

2004-05-27 19:30 UTC #7 lore raw

Re: [PATCH] SMP support for drain local pages v2.

In reply to Nigel Cunningham

Hi!

Quoted 10 lines

Quoted 3 lines
But until something which needs this change is merged into the tree I'd say
that this patch should live with the patch which requires it.
CPU hotplugging uses drain_local_pages, and shouldn't drain the pcp lists for all cpus. That's why I left the original version alone.
I'm submitting it now in preparation for merging, but Pavel's work on SMP support for swsusp should be using this too. (It's not, but it should be).

drain_local_pages was there so that accounting is not screwed by local pages. That should be non-issue for stopped cpus.

OTOH you are right that my swsusp with smp does not work on my smp machine, and I do not yet know why.

Signature · 1 line

64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms

Nigel Cunningham

author

2004-05-27 21:29 UTC #8 lore raw

Re: [PATCH] SMP support for drain local pages v2.

Hi.

Andrew, Dave: do you want to be cc'd still?

Quoted 2 lines · Pavel Machek

drain_local_pages was there so that accounting is not screwed by local pages. That should be non-issue for stopped cpus.

You're probably right that it's not absolutely necessary. That said, I have three reasons for wanting to do it: each cpu has say 200 pages in its pcp structs (this varies with zone size). If we don't drain them, we're saving 200 extra pages (because they're not marked as free). That's fine for a dual CPU system, but it's not very scalable. (I admit I wouldn't want to suspend to disk a 4GB machine now, but in the future?).

More than that, though, my real motivation is to be able to account for every page in the system and ensure that suspend is doing the right thing with all of them. If I drain the PCP page lists, I can be more certain that allocations do come from the right place.

Thirdly, after suspending hot pages aren't really hot anymore. We may as well start with clean lists.

Quoted 3 lines

OTOH you are right that my swsusp with smp does not work on my smp machine, and I do not yet know why.

I found that I needed to:
- save context for other CPUs.
- ensure they're completely idle during copyback even running the idle thread messed things up (use a smp function)
- ensure suspend is always running on CPU0 (at the start of suspending and resuming)

You might also consider how you're restoring highmem pages and how that interacts with SMP esp in flushing tlbs.

Nigel

Signature · 7 lines

Nigel & Michelle Cunningham
C/- Westminster Presbyterian Church Belconnen
61 Templeton Street, Cook, ACT 2614.
+61 (2) 6251 7727(wk); +61 (2) 6254 0216 (home)

Evolution (n): A hypothetical process whereby infinitely improbable events occur
with alarming frequency, order arises from chaos, and no one is given credit.

Pavel Machek

2004-05-27 19:30 UTC #9 lore raw

In reply to Nigel Cunningham

Hi!

Quoted 4 lines

This patch adds SMP support for drain_local_pages, so that suspend implementations can drain pcp structures on all CPUs and thus accurately
determine which pages are free.

Why do you need it, btw?

1st it might be easier to just on_each_cpu(drain_local_pages) in suspend2

2nd all but one cpus are stopped, right? That mrans that their local pages can't mess up suspnd's accounting => no need to drain them.
Pavel

2.6.6-current-bk/mm/page_alloc.c

Quoted 40 lines

smp-drain-local-pages/mm/page_alloc.c
--- 2.6.6-current-bk/mm/page_alloc.c 2004-05-26 19:47:15.000000000
+1000
+++ smp-drain-local-pages/mm/page_alloc.c 2004-05-26
19:56:19.000000000 +1000
mm/page_alloc.c 2004-05-26 19:56:19.000000000 +1000 renamed from mm/page_alloc.c 2004-05-26 19:47:15.000000000 +1000 +18 -0
@@ -459,6 +459,24 @@
        __drain_pages(smp_processor_id());
        local_irq_restore(flags);
 }
+
+#ifdef CONFIG_SMP
+static void __smp_drain_local_pages(void * data)
+{
+       drain_local_pages();
+}
+
+void smp_drain_local_pages(void)
+{
+       smp_call_function(__smp_drain_local_pages, NULL, 0, 1);
+       drain_local_pages();
+}
+#else
+void smp_drain_local_pages(void)
+{
+       drain_local_pages();
+}
+#endif
 #endif /* CONFIG_PM */
 
 static void zone_statistics(struct zonelist *zonelist, struct zone 
*z)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Signature · 1 line

64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms