* [PATCH] Don't refill pcp lists during SWSUSP.
@ 2003-03-17 23:56 Nigel Cunningham
2003-03-18 0:05 ` Andrew Morton
2003-03-18 8:18 ` Pavel Machek
0 siblings, 2 replies; 8+ messages in thread
From: Nigel Cunningham @ 2003-03-17 23:56 UTC (permalink / raw)
To: Pavel Machek, Linux Kernel Mailing List
Hi.
Here's another patch (the last for a little while, I promise!). It stops
the pcp lists from being refilled while SWSUSP is running. Despite the
comment in the page, drain_local_pages does only need to get called once
right now, but I have patches coming that will (DV) change that. This
patch is thus groundwork for them.
I see Linus has just done 2.5.65 - I'll rediff if need be.
Regards,
Nigel
--
Nigel Cunningham
495 St Georges Road South, Hastings 4201, New Zealand
Be diligent to present yourself approved to God as a workman who does
not need to be ashamed, handling accurately the word of truth.
-- 2 Timothy 2:14, NASB.
diff -ruN linux-2.5.64-02-free-page-map/include/linux/suspend.h linux-2.5.64-03-do-not-refill-hot-pages/include/linux/suspend.h
--- linux-2.5.64-02-free-page-map/include/linux/suspend.h 2003-03-10 12:36:16.000000000 +1300
+++ linux-2.5.64-03-do-not-refill-hot-pages/include/linux/suspend.h 2003-03-18 11:21:53.000000000 +1200
@@ -63,6 +63,8 @@
extern unsigned int nr_copy_pages __nosavedata;
extern suspend_pagedir_t *pagedir_nosave __nosavedata;
+extern unsigned int suspend_task;
+
/* Communication between kernel/suspend.c and arch/i386/suspend.c */
extern void do_magic_resume_1(void);
diff -ruN linux-2.5.64-02-free-page-map/kernel/suspend.c linux-2.5.64-03-do-not-refill-hot-pages/kernel/suspend.c
--- linux-2.5.64-02-free-page-map/kernel/suspend.c 2003-03-18 10:54:31.000000000 +1200
+++ linux-2.5.64-03-do-not-refill-hot-pages/kernel/suspend.c 2003-03-18 11:21:53.000000000 +1200
@@ -68,6 +68,7 @@
extern int sys_sync(void);
unsigned char software_suspend_enabled = 0;
+unsigned int suspend_task = 0;
#define SUSPEND_CONSOLE (MAX_NR_CONSOLES-1)
/* With SUSPEND_CONSOLE defined, it suspend looks *really* cool, but
@@ -232,6 +233,8 @@
}
} while(todo);
+ suspend_task = current->pid;
+
printk( "|\n" );
BUG_ON(in_atomic());
return 0;
@@ -253,6 +256,7 @@
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
+ suspend_task = 0;
printk( " done\n" );
MDELAY(500);
}
diff -ruN linux-2.5.64-02-free-page-map/mm/page_alloc.c linux-2.5.64-03-do-not-refill-hot-pages/mm/page_alloc.c
--- linux-2.5.64-02-free-page-map/mm/page_alloc.c 2003-03-18 10:53:22.000000000 +1200
+++ linux-2.5.64-03-do-not-refill-hot-pages/mm/page_alloc.c 2003-03-18 11:21:53.000000000 +1200
@@ -486,6 +486,10 @@
* Really, prep_compound_page() should be called from __rmqueue_bulk(). But
* we cheat by calling it from here, in the order > 0 path. Saves a branch
* or two.
+ *
+ * While suspending, we don't use the pcp structure. It mucks up our
+ * accounting for all the pages and necessitates calling drain_local_pages
+ * multiple times.
*/
static struct page *buffered_rmqueue(struct zone *zone, int order, int cold)
@@ -493,7 +497,7 @@
unsigned long flags;
struct page *page = NULL;
- if (order == 0) {
+ if ((order == 0) && (!suspend_task)) {
struct per_cpu_pages *pcp;
pcp = &zone->pageset[get_cpu()].pcp[cold];
@@ -700,7 +704,7 @@
void __free_pages(struct page *page, unsigned int order)
{
if (!PageReserved(page) && put_page_testzero(page)) {
- if (order == 0)
+ if ((order == 0) && (!suspend_task))
free_hot_page(page);
else
__free_pages_ok(page, order);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't refill pcp lists during SWSUSP.
2003-03-17 23:56 [PATCH] Don't refill pcp lists during SWSUSP Nigel Cunningham
@ 2003-03-18 0:05 ` Andrew Morton
2003-03-18 5:27 ` Nigel Cunningham
2003-03-18 6:40 ` Nigel Cunningham
2003-03-18 8:18 ` Pavel Machek
1 sibling, 2 replies; 8+ messages in thread
From: Andrew Morton @ 2003-03-18 0:05 UTC (permalink / raw)
To: Nigel Cunningham; +Cc: pavel, linux-kernel
Nigel Cunningham <ncunningham@clear.net.nz> wrote:
>
> +extern unsigned int suspend_task;
Please do:
#ifdef CONFIG_SOFTWARE_SUSPEND
unsigned int suspend_task;
#else
#define suspend_task 0
#endif
so the compiler can remove the few fast-path instructions which you have
added.
>
> + suspend_task = current->pid;
> +
Zero is a valid PID (the idle task...). It might be clearer to make
suspend_task a task_struct*.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't refill pcp lists during SWSUSP.
2003-03-18 0:05 ` Andrew Morton
@ 2003-03-18 5:27 ` Nigel Cunningham
[not found] ` <20030317222018.5c7f7a56.akpm@digeo.com>
2003-03-18 6:40 ` Nigel Cunningham
1 sibling, 1 reply; 8+ messages in thread
From: Nigel Cunningham @ 2003-03-18 5:27 UTC (permalink / raw)
To: Andrew Morton; +Cc: Linux Kernel Mailing List
On Tue, 2003-03-18 at 12:05, Andrew Morton wrote:
> Nigel Cunningham <ncunningham@clear.net.nz> wrote:
> >
> > +extern unsigned int suspend_task;
>
>
> Please do:
>
> #ifdef CONFIG_SOFTWARE_SUSPEND
> unsigned int suspend_task;
> #else
> #define suspend_task 0
> #endif
>
> so the compiler can remove the few fast-path instructions which you have
> added.
Okee doke. Will do.
I said I was going to use dynamically allocated bitmaps instead of page
flags. Do you mind if I do use pageflags after all (at least for the
moment)? I've used one in the generate_free_page_map patch, and need one
more to mark pages which will be saved in another pageset.
> >
> > + suspend_task = current->pid;
> > +
>
> Zero is a valid PID (the idle task...). It might be clearer to make
> suspend_task a task_struct*.
Mmm, but I will use suspend_task elsewhere and wanted to make the test
simple (zero = not suspending at the moment; since init will never be
the suspend task, it's safe usage).
Regards,
Nigel
--
Nigel Cunningham
495 St Georges Road South, Hastings 4201, New Zealand
Be diligent to present yourself approved to God as a workman who does
not need to be ashamed, handling accurately the word of truth.
-- 2 Timothy 2:14, NASB.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't refill pcp lists during SWSUSP.
[not found] ` <20030317222018.5c7f7a56.akpm@digeo.com>
@ 2003-03-18 6:35 ` Nigel Cunningham
0 siblings, 0 replies; 8+ messages in thread
From: Nigel Cunningham @ 2003-03-18 6:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: Pavel Machek, Linux Kernel Mailing List
On Tue, 2003-03-18 at 18:20, Andrew Morton wrote:
> Nigel Cunningham <ncunningham@clear.net.nz> wrote:
> > I said I was going to use dynamically allocated bitmaps instead of page
> > flags. Do you mind if I do use pageflags after all (at least for the
> > moment)? I've used one in the generate_free_page_map patch, and need one
> > more to mark pages which will be saved in another pageset.
> >
>
> I think it'd be best to avoid using more flags if poss. We are getting
> short, and designing for this now is probably less trauma than going
> through and redoing your stuff later.
Okay. I'll add the dynamic bitmap code and redo the previous patch.
Regards,
Nigel
--
Nigel Cunningham
495 St Georges Road South, Hastings 4201, New Zealand
Be diligent to present yourself approved to God as a workman who does
not need to be ashamed, handling accurately the word of truth.
-- 2 Timothy 2:14, NASB.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't refill pcp lists during SWSUSP.
2003-03-18 0:05 ` Andrew Morton
2003-03-18 5:27 ` Nigel Cunningham
@ 2003-03-18 6:40 ` Nigel Cunningham
1 sibling, 0 replies; 8+ messages in thread
From: Nigel Cunningham @ 2003-03-18 6:40 UTC (permalink / raw)
To: Andrew Morton; +Cc: Pavel Machek, Linux Kernel Mailing List
Here's the updated version.
Regards,
Nigel
diff -ruN linux-2.5.65-02/include/linux/suspend.h linux-2.5.65-03/include/linux/suspend.h
--- linux-2.5.65-02/include/linux/suspend.h 2003-03-10 12:36:16.000000000 +1300
+++ linux-2.5.65-03/include/linux/suspend.h 2003-03-18 17:56:37.000000000 +1200
@@ -63,6 +63,8 @@
extern unsigned int nr_copy_pages __nosavedata;
extern suspend_pagedir_t *pagedir_nosave __nosavedata;
+extern unsigned int suspend_task;
+
/* Communication between kernel/suspend.c and arch/i386/suspend.c */
extern void do_magic_resume_1(void);
@@ -79,6 +81,7 @@
static inline void software_suspend(void)
{
}
+#define suspend_task 0
#define software_resume() do { } while(0)
#define register_suspend_notifier(a) do { } while(0)
#define unregister_suspend_notifier(a) do { } while(0)
diff -ruN linux-2.5.65-02/kernel/suspend.c linux-2.5.65-03/kernel/suspend.c
--- linux-2.5.65-02/kernel/suspend.c 2003-03-18 17:53:44.000000000 +1200
+++ linux-2.5.65-03/kernel/suspend.c 2003-03-18 17:55:55.000000000 +1200
@@ -68,6 +68,7 @@
extern int sys_sync(void);
unsigned char software_suspend_enabled = 0;
+unsigned int suspend_task = 0;
#define SUSPEND_CONSOLE (MAX_NR_CONSOLES-1)
/* With SUSPEND_CONSOLE defined, it suspend looks *really* cool, but
@@ -232,6 +233,8 @@
}
} while(todo);
+ suspend_task = current->pid;
+
printk( "|\n" );
BUG_ON(in_atomic());
return 0;
@@ -253,6 +256,7 @@
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
+ suspend_task = 0;
printk( " done\n" );
MDELAY(500);
}
diff -ruN linux-2.5.65-02/mm/page_alloc.c linux-2.5.65-03/mm/page_alloc.c
--- linux-2.5.65-02/mm/page_alloc.c 2003-03-18 17:53:44.000000000 +1200
+++ linux-2.5.65-03/mm/page_alloc.c 2003-03-18 17:55:55.000000000 +1200
@@ -486,6 +486,10 @@
* Really, prep_compound_page() should be called from __rmqueue_bulk(). But
* we cheat by calling it from here, in the order > 0 path. Saves a branch
* or two.
+ *
+ * While suspending, we don't use the pcp structure. It mucks up our
+ * accounting for all the pages and necessitates calling drain_local_pages
+ * multiple times.
*/
static struct page *buffered_rmqueue(struct zone *zone, int order, int cold)
@@ -493,7 +497,7 @@
unsigned long flags;
struct page *page = NULL;
- if (order == 0) {
+ if ((order == 0) && (!suspend_task)) {
struct per_cpu_pages *pcp;
pcp = &zone->pageset[get_cpu()].pcp[cold];
@@ -700,7 +704,7 @@
void __free_pages(struct page *page, unsigned int order)
{
if (!PageReserved(page) && put_page_testzero(page)) {
- if (order == 0)
+ if ((order == 0) && (!suspend_task))
free_hot_page(page);
else
__free_pages_ok(page, order);
--
Nigel Cunningham
495 St Georges Road South, Hastings 4201, New Zealand
Be diligent to present yourself approved to God as a workman who does
not need to be ashamed, handling accurately the word of truth.
-- 2 Timothy 2:14, NASB.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't refill pcp lists during SWSUSP.
2003-03-17 23:56 [PATCH] Don't refill pcp lists during SWSUSP Nigel Cunningham
2003-03-18 0:05 ` Andrew Morton
@ 2003-03-18 8:18 ` Pavel Machek
2003-03-18 10:06 ` Nigel Cunningham
1 sibling, 1 reply; 8+ messages in thread
From: Pavel Machek @ 2003-03-18 8:18 UTC (permalink / raw)
To: Nigel Cunningham, akpm; +Cc: Linux Kernel Mailing List
Hi!
> Here's another patch (the last for a little while, I promise!). It stops
> the pcp lists from being refilled while SWSUSP is running. Despite the
> comment in the page, drain_local_pages does only need to get called once
> right now, but I have patches coming that will (DV) change that. This
> patch is thus groundwork for them.
This adds external (and pretty ugly) dependency of swsusp on the
outside. And as it still needs to drain_local_pages(), nothing is
gained. I believe it is better to just call drain_local_pages few
times. Magic hooks "if suspending, don't do this" seem like wrong
approach to me.
Pavel
--
Horseback riding is like software...
...vgf orggre jura vgf serr.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't refill pcp lists during SWSUSP.
2003-03-18 8:18 ` Pavel Machek
@ 2003-03-18 10:06 ` Nigel Cunningham
2003-03-18 16:58 ` Pavel Machek
0 siblings, 1 reply; 8+ messages in thread
From: Nigel Cunningham @ 2003-03-18 10:06 UTC (permalink / raw)
To: Pavel Machek; +Cc: akpm, Linux Kernel Mailing List
Hi Pavel.
You need to remember that this is infrastructure for later. I tried it
other ways and would have needed a number of calls to drain local pages.
Perhaps I'm taking the wrong approach, trying to feed a patch at a time
when you can't see how it all fits together. If you like, I'll send a
'whole kit and caboodle' patch as soon as I get the last bugs ironed
out. I have it suspending and resuming at the moment, but have one last
bug to iron out that's stopping me getting back from
do_suspend_lowlevel. It's just a matter of time, and then that will be
fixed. I could send you a monster patch then :> (I'll be posting it
somewhere anyway - I'll want others to test it, of course).
Regards,
Nigel
On Tue, 2003-03-18 at 20:18, Pavel Machek wrote:
> Hi!
>
> > Here's another patch (the last for a little while, I promise!). It stops
> > the pcp lists from being refilled while SWSUSP is running. Despite the
> > comment in the page, drain_local_pages does only need to get called once
> > right now, but I have patches coming that will (DV) change that. This
> > patch is thus groundwork for them.
>
> This adds external (and pretty ugly) dependency of swsusp on the
> outside. And as it still needs to drain_local_pages(), nothing is
> gained. I believe it is better to just call drain_local_pages few
> times. Magic hooks "if suspending, don't do this" seem like wrong
> approach to me.
>
> Pavel
--
Nigel Cunningham
495 St Georges Road South, Hastings 4201, New Zealand
Be diligent to present yourself approved to God as a workman who does
not need to be ashamed, handling accurately the word of truth.
-- 2 Timothy 2:14, NASB.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Don't refill pcp lists during SWSUSP.
2003-03-18 10:06 ` Nigel Cunningham
@ 2003-03-18 16:58 ` Pavel Machek
0 siblings, 0 replies; 8+ messages in thread
From: Pavel Machek @ 2003-03-18 16:58 UTC (permalink / raw)
To: Nigel Cunningham; +Cc: Pavel Machek, akpm, Linux Kernel Mailing List
Hi!
> You need to remember that this is infrastructure for later. I tried it
> other ways and would have needed a number of calls to drain local
> pages.
If you need 10x drain_local_pages(), that's okay -- because it does
not slow down non-suspend paths. If you need more of them... well then
I understand this is the way to go (but you should be able to hide
drain_local_pages in some local funcion or something...).
Pavel
--
Horseback riding is like software...
...vgf orggre jura vgf serr.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2003-03-18 16:47 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-03-17 23:56 [PATCH] Don't refill pcp lists during SWSUSP Nigel Cunningham
2003-03-18 0:05 ` Andrew Morton
2003-03-18 5:27 ` Nigel Cunningham
[not found] ` <20030317222018.5c7f7a56.akpm@digeo.com>
2003-03-18 6:35 ` Nigel Cunningham
2003-03-18 6:40 ` Nigel Cunningham
2003-03-18 8:18 ` Pavel Machek
2003-03-18 10:06 ` Nigel Cunningham
2003-03-18 16:58 ` Pavel Machek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).