swsusp critical code rewritten to assembly
Hi!
do_magic was really too fragile to be written in C. This patch rewrites it into assembly, to make sure C compiler does not generate stack access and corrupt memory that way. Plus it cleans up suspend.c a bit, makes it really free memory it needs, an no longer calls drivers from atomic context.
Please apply,
Pavel
arch/i386/kernel/Makefile +1 -1
@@ -25,7 +25,7 @@ obj-$(CONFIG_X86_MPPARSE) += mpparse.o obj-$(CONFIG_X86_LOCAL_APIC) += apic.o nmi.o obj-$(CONFIG_X86_IO_APIC) += io_apic.o -obj-$(CONFIG_SOFTWARE_SUSPEND) += suspend.o +obj-$(CONFIG_SOFTWARE_SUSPEND) += suspend.o suspend_asm.o obj-$(CONFIG_X86_NUMAQ) += numaq.o obj-$(CONFIG_PROFILING) += profile.o obj-$(CONFIG_EDD) += edd.o
arch/i386/kernel/suspend.c +123 -106
287 lines not on this page · raw message
arch/i386/kernel/suspend_asm.S +87 -0
87 lines not on this page · raw message
include/asm-i386/suspend.h +6 -3
@@ -15,8 +15,6 @@/* image of the saved processor state */ struct saved_context { -unsigned long eax, ebx, ecx, edx; -unsigned long esp, ebp, esi, edi;u16 es, fs, gs, ss;unsigned long cr0, cr2, cr3, cr4;u16 gdt_pad;
@@ -30,9 +28,14 @@unsigned long tr;unsigned long safety;unsigned long return_address; -unsigned long eflags; } __attribute__((packed)); +/* We'll access these from assembly, so we'd better have them outside struct */ +extern unsigned long saved_context_eax, saved_context_ebx, saved_context_ecx, saved_context_edx; +extern unsigned long saved_context_esp, saved_context_ebp, saved_context_esi, saved_context_edi; +extern unsigned long saved_context_eflags; + +#define loaddebug (thread,register) \ __asm__("movl %0,%%db" #register \ :/* no output */ \
kernel/suspend.c +18 -11
@@ -235,6 +235,7 @@ }while (todo);printk ("|\n" ); +BUG_ON (in_atomic ());return 0 ; }
@@ -489,7 +490,6 @@if (PageNosave (page))continue ; -if ((chunk_size=is_head_of_free_region (page))!=0 ) { pfn += chunk_size -1 ;continue ;
@@ -500,9 +500,8 @@/* * Just copy whole code segment. Hopefully it is not that big. */ -if (ADDRESS (pfn) >= (unsigned long ) - &__nosave_begin &&ADDRESS (pfn) < - (unsigned long )&__nosave_end) { +if ((ADDRESS (pfn) >= (unsigned long ) &__nosave_begin) && + (ADDRESS (pfn) < (unsigned long ) &__nosave_end)) {PRINTK ("[nosave %x]" ,ADDRESS (pfn));continue ; }
@@ -623,7 +623,7 @@static void free_some_memory (void ) {printk ("Freeing memory: " ); -while (try_to_free_pages (&contig_page_data.node_zones[ZONE_HIGHMEM ],GFP_KSWAPD ,0 )) +while (shrink_all_memory (10000 ))printk ("." );printk ("|\n" ); }
@@ -676,7 +676,7 @@ } } -static int suspend_save_image (void ) +static int suspend_prepare_image (void ) {struct sysinfo i;unsigned int nr_needed_pages =0 ;
@@ -725,9 +725,15 @@* * Following line enforces not writing to disk until we choose. */ -drivers_unsuspend (); -spin_unlock_irq (&suspend_pagedir_lock); +printk ("critical section/: done (%d pages copied)\n" , nr_copy_pages ); +spin_unlock_irq (&suspend_pagedir_lock); +return 0 ; +} + +void suspend_save_image (void ) +{ +drivers_unsuspend ();lock_swapdevices ();write_suspend_image ();
@@ -738,7 +744,6 @@* filesystem clean: it is not. (And it does not matter, if we resume * correctly, we'll mark system clean, anyway.) */ -return 0 ; }void suspend_power_down (void )
@@ -788,8 +793,8 @@PRINTK ("Freeing prev allocated pagedir\n" );free_suspend_pagedir ((unsigned long ) pagedir_save); -drivers_resume (RESUME_ALL_PHASES );spin_unlock_irq (&suspend_pagedir_lock); +drivers_resume (RESUME_ALL_PHASES );PRINTK ("Fixing swap signatures... " );mark_swapfiles (((swp_entry_t ) {0 }),MARK_SWAP_RESUME );
@@ -804,14 +809,17 @@ {mb ();barrier (); +BUG_ON (in_atomic ());spin_lock_irq (&suspend_pagedir_lock); }void do_magic_suspend_2 (void ) {read_swapfiles (); -if (!suspend_save_image ()) +if (!suspend_prepare_image ()) {/* suspend_save_image realeses suspend_pagedir_lock */ +suspend_save_image ();suspend_power_down ();/* FIXME: if suspend_power_down is commented out, console is lost after few suspends ?! */ + }printk (KERN_EMERG "%sSuspend failed, trying to recover...\n" , name_suspend);MDELAY (1000 );/* So user can wait and report us messages if armageddon comes :-) */
Signature · 2 lines
Worst form of spam? Adding advertisment signatures ala sourceforge.net. What goes next? Inserting advertisment *into* email?
Quoted 6 lines · Pavel Machek
do_magicwas really too fragile to be written in C. This patch rewrites it into assembly, to make sure C compiler does not generate stack access and corrupt memory that way. Plus it cleans upsuspend.ca bit, makes it really free memory it needs, an no longer calls drivers from atomic context.
But this still has stuff in C:
arch/i386/kernel/suspend.c line 213
Quoted 11 lines
arch/i386/kernel/suspend.c +9
-1+ asmvolatile ("movl %0, %%esp" ::"m" (saved_context_esp)); + asmvolatile ("movl %0, %%ebp" ::"m" (saved_context_ebp)); + asmvolatile ("movl %0, %%eax" ::"m" (saved_context_eax)); + asmvolatile ("movl %0, %%ebx" ::"m" (saved_context_ebx)); + asmvolatile ("movl %0, %%ecx" ::"m" (saved_context_ecx)); + asmvolatile ("movl %0, %%edx" ::"m" (saved_context_edx)); + asmvolatile ("movl %0, %%esi" ::"m" (saved_context_esi)); + asmvolatile ("movl %0, %%edi" ::"m" (saved_context_edi)); -fix_processor_context (); +restore_processor_state ();
What's up with that? There's no way you can safely restore regular registers and _especially_ %%esp from C code, since the compiler may be using them for other things.
Linus
Hi!
Quoted 17 lines
Quoted 5 lines
do_magicwas really too fragile to be written in C. This patch rewrites it into assembly, to make sure C compiler does not generate stack access and corrupt memory that way. Plus it cleans upsuspend.ca bit, makes it really free memory it needs, an no longer calls drivers from atomic context.But this still has stuff in C:
Quoted 9 lines
arch/i386/kernel/suspend.c +8
-0+ asmvolatile ("movl %0, %%esp" ::"m" (saved_context_esp)); + asmvolatile ("movl %0, %%ebp" ::"m" (saved_context_ebp)); + asmvolatile ("movl %0, %%eax" ::"m" (saved_context_eax)); + asmvolatile ("movl %0, %%ebx" ::"m" (saved_context_ebx)); + asmvolatile ("movl %0, %%ecx" ::"m" (saved_context_ecx)); + asmvolatile ("movl %0, %%edx" ::"m" (saved_context_edx)); + asmvolatile ("movl %0, %%esi" ::"m" (saved_context_esi)); + asmvolatile ("movl %0, %%edi" ::"m" (saved_context_edi));
Quoted 6 lines
arch/i386/kernel/suspend.c line 213
Quoted 2 lines
arch/i386/kernel/suspend.c +1
-1-fix_processor_context (); +restore_processor_state ();What's up with that? There's no way you can safely restore regular registers and _especially_ %%esp from C code, since the compiler may be using them for other things.
After applying that patch, this stuff is no longer used for suspend-to-disk. Its still there for suspend-to-ram; I'll fix that.
Pavel
Signature · 2 lines
Casualities in World Trade Center: ~3k dead inside the building, cryptography in U.S.A. and free speech in Czech Republic.