Next step of smp support & fix device suspending
Subsystems (2)
Hi!
This introduces functions for stopping all-but-boot-cpus, which will be needed for smp suspend, and fixes level for calling driver model:
there's no D4 power level, only D3 (means device off), and tg3 driver actually cares. Ugh and one useless mdelay killed, and freeze_processes() now BUGS() if its not compiled in. [We can probably just remove it for non-CONFIG_PM case in future]. It is bad idea to pretend success, and nobody should ever call it in !CONFIG_PM case anyway. Please apply,
Pavel
kernel/power/swsusp.c +11 -7
@@ -716,7 +721,7 @@mb ();spin_lock_irq (&suspend_pagedir_lock);/* Done to disable interrupts */ -device_power_down (4 ); +device_power_down (3 );PRINTK ("Waiting for DMAs to settle down...\n" );mdelay (1000 );/* We do not want some readahead with DMA to corrupt our memory, right? Do it with disabled interrupts for best effect. That way, if some
@@ -785,7 +795,7 @@ {int is_problem;read_swapfiles (); -device_power_down (4 ); +device_power_down (3 ); is_problem =suspend_prepare_image ();device_power_up ();spin_unlock_irq (&suspend_pagedir_lock);
@@ -802,7 +812,6 @@barrier ();mb ();spin_lock_irq (&suspend_pagedir_lock);/* Done to disable interrupts */ -mdelay (1000 );free_pages ((unsigned long ) pagedir_nosave, pagedir_order);spin_unlock_irq (&suspend_pagedir_lock);
@@ -839,9 +848,10 @@need half of memory free. */ free_some_memory (); - -/* Save state of all device drivers, and stop them. */ -if ((res =device_suspend (4 ))==0 ) +disable_nonboot_cpus (); +/* Save state of all device drivers, and stop them. */ +printk ("Suspending devices... " ); +if ((res =device_suspend (3 ))==0 ) {/* If stopping device drivers worked, we proceed basically into * suspend_save_image. *
@@ -852,7 +862,9 @@* using normal kernel mechanism. */ do_magic (0 ); + }thaw_processes (); +enable_nonboot_cpus (); }else res = -EBUSY ; software_suspend_enabled =1 ;
@@ -1192,7 +1204,9 @@printk ("resuming from %s\n" , resume_file);if (read_suspend_image (resume_file,0 )) goto read_failure; -device_suspend (4 ); +/* FIXME: Should we stop processes here, just to be safer? */ +disable_nonboot_cpus (); +device_suspend (3 );do_magic (1 );panic ("This never returns" );
include/linux/suspend.h +11 -12
@@ -67,20 +67,19 @@extern void pm_restore_console (void );#else -static inline void refrigerator (unsigned long flag) -{ - -} -static inline int freeze_processes (void ) -{ -return 0 ; -} -static inline void thaw_processes (void ) -{ - -} +static inline void refrigerator (unsigned long flag) {} +static inline int freeze_processes (void ) {BUG (); } +static inline void thaw_processes (void ) {}#endif /* CONFIG_PM */ +#ifdef CONFIG_SMP +extern void disable_nonboot_cpus (void ); +extern void enable_nonboot_cpus (void ); +#else +static inline void disable_nonboot_cpus (void ) {} +static inline void enable_nonboot_cpus (void ) {} +#endif + asmlinkage void do_magic(int is_resume); asmlinkage void do_magic_resume_1(void); asmlinkage void do_magic_resume_2(void);
Signature · 2 lines
People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
A message that is not in the archive (sent off-list or not yet received): [email protected]
Hi!
Quoted 10 lines
Quoted 2 lines
This introduces functions for stopping all-but-boot-cpus, which will be needed for smp suspend, and fixes level for calling driver model:
hm, this breaks the build.
kernel/built-in.o(.text+0x21731): In function `software_suspend':
/usr/src/25/kernel/power/swsusp.c:841: undefined reference todisable_nonboot_cpus' kernel/built-in.o(.text+0x2175f):/usr/src/25/kernel/power/swsusp.c:857: undefined reference toenable_nonboot_cpus'I'll drop all the swpsup patches, as they seem to be in some flux.
That's smp, right? Sorry about that.
Can you take this one, instead? [cc-ed lkml]
It fixes levels for calling driver model, puts devices into sleep before powering down (so that emergency parking does not happen), and actually introduces SMP support, but its disabled for now. Plus noone should try to freeze_processes() when thats not implemented, we now BUG()s -- we do not want Heisenbugs.
Thanks,
Pavel
include/linux/suspend.h +11 -12
@@ -67,20 +67,19 @@extern void pm_restore_console (void );#else -static inline void refrigerator (unsigned long flag) -{ - -} -static inline int freeze_processes (void ) -{ -return 0 ; -} -static inline void thaw_processes (void ) -{ - -} +static inline void refrigerator (unsigned long flag) {} +static inline int freeze_processes (void ) {BUG (); } +static inline void thaw_processes (void ) {}#endif /* CONFIG_PM */ +#ifdef CONFIG_SMP +extern void disable_nonboot_cpus (void ); +extern void enable_nonboot_cpus (void ); +#else +static inline void disable_nonboot_cpus (void ) {} +static inline void enable_nonboot_cpus (void ) {} +#endif + asmlinkage void do_magic(int is_resume); asmlinkage void do_magic_resume_1(void); asmlinkage void do_magic_resume_2(void);
kernel/power/swsusp.c +12 -7
@@ -693,6 +701,7 @@ else#endif { +device_suspend (3 );device_shutdown ();machine_power_off (); }
@@ -713,7 +722,7 @@mb ();spin_lock_irq (&suspend_pagedir_lock);/* Done to disable interrupts */ -device_power_down (4 ); +device_power_down (3 );PRINTK ("Waiting for DMAs to settle down...\n" );mdelay (1000 );/* We do not want some readahead with DMA to corrupt our memory, right? Do it with disabled interrupts for best effect. That way, if some
@@ -782,7 +796,7 @@ {int is_problem;read_swapfiles (); -device_power_down (4 ); +device_power_down (3 ); is_problem =suspend_prepare_image ();device_power_up ();spin_unlock_irq (&suspend_pagedir_lock);
@@ -799,7 +813,6 @@barrier ();mb ();spin_lock_irq (&suspend_pagedir_lock);/* Done to disable interrupts */ -mdelay (1000 );free_pages ((unsigned long ) pagedir_nosave, pagedir_order);spin_unlock_irq (&suspend_pagedir_lock);
@@ -836,9 +849,10 @@need half of memory free. */ free_some_memory (); - -/* Save state of all device drivers, and stop them. */ -if ((res =device_suspend (4 ))==0 ) +disable_nonboot_cpus (); +/* Save state of all device drivers, and stop them. */ +printk ("Suspending devices... " ); +if ((res =device_suspend (3 ))==0 ) {/* If stopping device drivers worked, we proceed basically into * suspend_save_image. *
@@ -849,7 +863,9 @@* using normal kernel mechanism. */ do_magic (0 ); + }thaw_processes (); +enable_nonboot_cpus (); }else res = -EBUSY ; software_suspend_enabled =1 ;
@@ -1201,7 +1205,9 @@printk ("resuming from %s\n" , resume_file);if (read_suspend_image (resume_file,0 )) goto read_failure; -device_suspend (4 ); +/* FIXME: Should we stop processes here, just to be safer? */ +disable_nonboot_cpus (); +device_suspend (3 );do_magic (1 );panic ("This never returns" );
kernel/power/smp.c +85 -0
85 lines not on this page · raw message
kernel/power/Makefile +1 -0
@@ -1,4 +1,5 @@ obj-y := main.o process.o console.o pm.o +obj-$(CONFIG_SMP) += smp.o obj-$(CONFIG_SOFTWARE_SUSPEND) += swsusp.o obj-$(CONFIG_PM_DISK) += disk.o pmdisk.o
Signature · 2 lines
People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
Quoted 9 lines
This introduces functions for stopping all-but-boot-cpus, which will be needed for smp suspend, and fixes level for calling driver model:
there's no D4 power level, only D3 (means device off), and tg3 driver actually cares. Ugh and one useless mdelay killed, andfreeze_processes()nowBUGS()if its not compiled in. [We can probably just remove it for non-CONFIG_PM case in future]. It is bad idea to pretend success, and nobody should ever call it in !CONFIG_PM case anyway. Please apply,
Nice, just a couple of questions...
Review
Quoted 2 lines
(quoted diff) +1
-1- device_power_down(4); + device_power_down(3);
There are defined values in include/linux/device.h. You should be using those, instead of the magic constants (even if the magic constants actually make sense as the power states :).
kernel/power/swsusp.c line 727
Quoted 3 lines
kernel/power/swsusp.c +0
-0PRINTK ("Waiting for DMAs to settle down...\n" );mdelay (1000 );/* We do not want some readahead with DMA to corrupt our memory, right? Do it with disabled interrupts for best effect. That way, if some
On a related note, can we kill this piece of code? It's not clear that it's necessary. If it is, it begs for a more systematic way of achieving the goal.
Pat
Quoted 11 lines
Quoted 9 lines
This introduces functions for stopping all-but-boot-cpus, which will be needed for smp suspend, and fixes level for calling driver model:
there's no D4 power level, only D3 (means device off), and tg3 driver actually cares. Ugh and one useless mdelay killed, andfreeze_processes()nowBUGS()if its not compiled in. [We can probably just remove it for non-CONFIG_PM case in future]. It is bad idea to pretend success, and nobody should ever call it in !CONFIG_PM case anyway. Please apply,Nice, just a couple of questions...
Thanks.
Quoted 6 lines
Review
Quoted 2 lines
(quoted diff) +1
-1- device_power_down(4); + device_power_down(3);There are defined values in
include/linux/device.h. You should be using those, instead of the magic constants (even if the magic constants actually make sense as the power states :).
Ugh, I have not figured which magic constants should I use. Should that be SUSPEND_POWER_DOWN?
Quoted 7 lines
kernel/power/swsusp.c line 727
Quoted 3 lines
kernel/power/swsusp.c +0
-0PRINTK ("Waiting for DMAs to settle down...\n" );mdelay (1000 );/* We do not want some readahead with DMA to corrupt our memory, right? Do it with disabled interrupts for best effect. That way, if some On a related note, can we kill this piece of code? It's not clear that it's necessary. If it is, it begs for a more systematic way of achieving the goal.
Ok, killed, I'll propagate it if there are no problems.
Pavel
Signature · 2 lines
People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!