Hi Alan,
Following six patches for swsusp that were used by various people
through swsusp list. They should apply in order to 2.4.19-pre10-ac2.
Most of those is already in 2.5.22 suspend part managed by Pavel.
1/6 Documentation and comments cleanings
2/6 Fixing some compilation configuration problem, cleaning code
and making suspension stuff printk on a pretty console
3/6 Fix possible endless loop in ide-suspend stuff when using
removable devices
4/6 Fix swap signature in case of noresume option
5/6 Use memeat to make suspension of *big* sessions possible
6/6 Clean SysRq stuff.
Clean obsolete PF_KERNTHREAD flag.
Manage kernel threads: bdflush, khubd, nfs shares (lockd, rpciod), kjournald, kreiserfsd.
I've tested the patches sequentially and made one compilation of each.
If you agree to apply them, this should achieve partial merge between
your tree and what I call swsusp-beta8
Sincerely,
--
Florent Chabaud
Hi!
> Following six patches for swsusp that were used by various people
> through swsusp list. They should apply in order to 2.4.19-pre10-ac2.
> Most of those is already in 2.5.22 suspend part managed by Pavel.
Heh, so now I'm going patch by patch to figure what to merge :-(.
> 1/6 Documentation and comments cleanings
@@ -1715,9 +1715,9 @@
S: Germany
N: Gabor Kuti
-E: [email protected]
-E: [email protected]
-D: Author and Maintainer for Software Suspend
+M: [email protected]
+M: [email protected]
+D: Software suspend
N: Jaroslav Kysela
E: [email protected]
Why do you change E to M here?
Pavel
--
(about SSSCA) "I don't say this lightly. However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa
Hi!
> 1/6 Documentation and comments cleanings
> 2/6 Fixing some compilation configuration problem, cleaning code
> and making suspension stuff printk on a pretty console
@@ -141,31 +141,23 @@
*/
#define PAGES_FOR_IO 512
-static const char *name_suspend = "Suspend Machine: ";
-static const char *name_resume = "Resume Machine: ";
+#define name_suspend "Suspend Machine: "
+#define name_resume "Resume Machine: "
+#define name_swsusp "Software suspend: "
Florent originally did it in separate string to save space by not
copying it hundred times.
-#ifdef DEBUG_DEFAULT
...
+#if defined( DEBUG_DEFAULT )
#ifdef seems way more readable to me.
+ name_resume, cur->swh.magic.magic); /* even with a noresume option, it is better
+ to panic here, because that means that the
+ resume device is not a proper swap one. It
+ is perhaps a linux or dos partition and we
+ don't want to risk damaging it. */
Swapon seems to do its own checks, so this is red herring.
> 3/6 Fix possible endless loop in ide-suspend stuff when using
> removable devices
> 4/6 Fix swap signature in case of noresume option
> 5/6 Use memeat to make suspension of *big* sessions possible
> 6/6 Clean SysRq stuff.
> Clean obsolete PF_KERNTHREAD flag.
> Manage kernel threads: bdflush, khubd, nfs shares (lockd,
> rpciod), kjournald, kreiserfsd.
sprintf(current->comm, "lockd");
-
+ current->flags |= PF_IOTHREAD;
Lockd does not seem any io needed for suspend-to-disk. I guess it
would be better to freeze it.
strcpy(current->comm, "rpciod");
-
+ current->flags |= PF_IOTHREAD;
+
No, this is incorrect. I believe rpciod could submit packet for io in
time we are freezing devices. If it does that... bye bye to your data.
Fixing swap signatures should really be done in separate function.
Pavel
PS: This is what I did in response to your patch (it compiles,
otherwise untested). I'll try to fix noresume fixing signatures
somehow.
--- linux-swsusp.test/CREDITS Mon Jun 10 17:17:36 2002
+++ linux-swsusp/CREDITS Sun Jun 30 23:48:20 2002
@@ -502,6 +502,14 @@
S: Fremont, California 94539
S: USA
+N: Florent Chabaud
+E: [email protected]
+D: software suspend
+S: SGDN/DCSSI/SDS/LTI
+S: 58, Bd Latour-Maubourg
+S: 75700 Paris 07 SP
+S: France
+
N: Gordon Chaffee
E: [email protected]
W: http://bmrc.berkeley.edu/people/chaffee/
@@ -1708,7 +1716,7 @@
N: Gabor Kuti
M: [email protected]
M: [email protected]
-D: Software suspend
+D: Original author of software suspend
N: Jaroslav Kysela
E: [email protected]
--- linux-swsusp.test/kernel/suspend.c Thu Jun 27 23:53:13 2002
+++ linux-swsusp/kernel/suspend.c Mon Jul 1 00:36:32 2002
@@ -97,7 +97,7 @@
/* Variables to be preserved over suspend */
static int new_loglevel = 7;
static int orig_loglevel = 0;
-static int orig_fgconsole;
+static int orig_fgconsole, orig_kmsg;
static int pagedir_order_check;
static int nr_copy_pages_check;
@@ -145,26 +145,17 @@
/*
* Debug
*/
-#define DEBUG_DEFAULT 1
+#undef DEBUG_DEFAULT
#undef DEBUG_PROCESS
#undef DEBUG_SLOW
#define TEST_SWSUSP 1 /* Set to 1 to reboot instead of halt machine after suspension */
#ifdef DEBUG_DEFAULT
-#define PRINTD(func, f, a...) \
- do { \
- printk("%s", func); \
- printk(f, ## a); \
- } while(0)
-#define PRINTS(f, a...) PRINTD(name_suspend, f, ## a)
-#define PRINTR(f, a...) PRINTD(name_resume, f, ## a)
-#define PRINTK(f, a...) printk(f, ## a)
+# define PRINTK(f, a...) printk(f, ## a)
#else
-#define PRINTD(func, f, a...)
-#define PRINTS(f, a...)
-#define PRINTR(f, a...)
-#define PRINTK(f, a...)
+# define PRINTK(f, a...)
#endif
+
#ifdef DEBUG_SLOW
#define MDELAY(a) mdelay(a)
#else
@@ -195,8 +186,8 @@
long save;
save = current->state;
current->state = TASK_STOPPED;
-// PRINTK("%s entered refrigerator\n", current->comm);
- printk(":");
+ PRINTK("%s entered refrigerator\n", current->comm);
+ printk("=");
current->flags &= ~PF_FREEZE;
if (flag)
flush_signals(current); /* We have signaled a kernel thread, which isn't normal behaviour
@@ -205,8 +196,7 @@
current->flags |= PF_FROZEN;
while (current->flags & PF_FROZEN)
schedule();
-// PRINTK("%s left refrigerator\n", current->comm);
- printk(":");
+ PRINTK("%s left refrigerator\n", current->comm);
current->state = save;
}
@@ -216,8 +206,7 @@
int todo, start_time;
struct task_struct *p;
- PRINTS( "Waiting for tasks to stop... " );
-
+ printk( "Stopping tasks:\n" );
start_time = jiffies;
do {
todo = 0;
@@ -240,13 +229,13 @@
sys_sched_yield();
schedule();
if (time_after(jiffies, start_time + TIMEOUT)) {
- PRINTK( "\n" );
+ printk( "\n" );
printk(KERN_ERR " stopping tasks failed (%d tasks remaining)\n", todo );
return todo;
}
} while(todo);
- PRINTK( " ok\n" );
+ printk( "|\n" );
return 0;
}
@@ -254,7 +243,7 @@
{
struct task_struct *p;
- PRINTR( "Restarting tasks..." );
+ printk( "Restarting tasks..." );
read_lock(&tasklist_lock);
for_each_task(p) {
INTERESTING(p);
@@ -265,7 +254,7 @@
wake_up_process(p);
}
read_unlock(&tasklist_lock);
- PRINTK( " done\n" );
+ printk( " done\n" );
MDELAY(500);
}
@@ -376,7 +365,7 @@
root_swap = i;
} else {
#if 0
- PRINTS( "device %s (%x != %x) ignored\n", swap_info[i].swap_file->d_name.name, swap_info[i].swap_device, resume_device );
+ printk( "Resume: device %s (%x != %x) ignored\n", swap_info[i].swap_file->d_name.name, swap_info[i].swap_device, resume_device );
#endif
swapfile_used[i] = SWAPFILE_IGNORED;
}
@@ -409,10 +398,10 @@
unsigned long address;
struct page *page;
- PRINTS( "Writing data to swap (%d pages): ", nr_copy_pages );
+ printk( "Writing data to swap (%d pages): ", nr_copy_pages );
for (i=0; i<nr_copy_pages; i++) {
if (!(i%100))
- PRINTK( "." );
+ printk( "." );
if (!(entry = get_swap_page()).val)
panic("\nNot enough swapspace when writing data" );
@@ -424,12 +413,12 @@
rw_swap_page_sync(WRITE, entry, page);
(pagedir_nosave+i)->swap_address = entry;
}
- PRINTK(" done\n");
- PRINTS( "Writing pagedir (%d pages): ", nr_pgdir_pages);
+ printk( "|\n" );
+ printk( "Writing pagedir (%d pages): ", nr_pgdir_pages);
for (i=0; i<nr_pgdir_pages; i++) {
cur = (union diskpage *)((char *) pagedir_nosave)+i;
BUG_ON ((char *) cur != (((char *) pagedir_nosave) + i*PAGE_SIZE));
- PRINTK( "." );
+ printk( "." );
if (!(entry = get_swap_page()).val) {
printk(KERN_CRIT "Not enough swapspace when writing pgdir\n" );
panic("Don't know how to recover");
@@ -448,7 +437,7 @@
rw_swap_page_sync(WRITE, entry, page);
prev = entry;
}
- PRINTK(", header");
+ printk("H");
BUG_ON (sizeof(struct suspend_header) > PAGE_SIZE-sizeof(swp_entry_t));
BUG_ON (sizeof(union diskpage) != PAGE_SIZE);
if (!(entry = get_swap_page()).val)
@@ -466,9 +455,9 @@
rw_swap_page_sync(WRITE, entry, page);
prev = entry;
- PRINTK( ", signature" );
+ printk( "S" );
mark_swapfiles(prev, MARK_SWAP_SUSPEND);
- PRINTK( ", done\n" );
+ printk( "|\n" );
MDELAY(1000);
free_page((unsigned long) buffer);
@@ -504,7 +493,7 @@
if (ADDRESS(loop) >= (unsigned long)
&__nosave_begin && ADDRESS(loop) <
(unsigned long)&__nosave_end) {
- printk("[nosave]");
+ PRINTK("[nosave %x]", ADDRESS(loop));
continue;
}
/* Hmm, perhaps copying all reserved pages is not too healthy as they may contain
@@ -585,9 +574,11 @@
set_console (SUSPEND_CONSOLE);
if(vt_waitactive(SUSPEND_CONSOLE)) {
- PRINTS("Bummer. Can't switch VCs.");
+ PRINTK("Bummer. Can't switch VCs.");
return 1;
}
+ orig_kmsg = kmsg_redirect;
+ kmsg_redirect = SUSPEND_CONSOLE;
#endif
#endif
return 0;
@@ -604,13 +595,12 @@
static int prepare_suspend_processes(void)
{
- PRINTS( "Stopping processes\n" );
- MDELAY(1000);
if (freeze_processes()) {
- PRINTS( "Not all processes stopped!\n" );
+ printk( KERN_ERR "Suspend failed: Not all processes stopped!\n" );
thaw_processes();
return 1;
}
+ MDELAY(1000);
do_suspend_sync();
return 0;
}
@@ -622,10 +612,10 @@
*/
static void free_some_memory(void)
{
- PRINTS("Freeing memory: ");
+ printk("Freeing memory: ");
while (try_to_free_pages(&contig_page_data.node_zones[ZONE_HIGHMEM], GFP_KSWAPD, 0))
printk(".");
- printk("\n");
+ printk("|\n");
}
/* Make disk drivers accept operations, again */
@@ -682,11 +672,11 @@
unsigned int nr_needed_pages = 0;
pagedir_nosave = NULL;
- PRINTS( "/critical section: Counting pages to copy" );
+ printk( "/critical section: Counting pages to copy" );
nr_copy_pages = count_and_copy_data_pages(NULL);
nr_needed_pages = nr_copy_pages + PAGES_FOR_IO;
- PRINTK(" (pages needed: %d+%d=%d free: %d)\n",nr_copy_pages,PAGES_FOR_IO,nr_needed_pages,nr_free_pages());
+ printk(" (pages needed: %d+%d=%d free: %d)\n",nr_copy_pages,PAGES_FOR_IO,nr_needed_pages,nr_free_pages());
if(nr_free_pages() < nr_needed_pages) {
printk(KERN_CRIT "%sCouldn't get enough free pages, on %d pages short\n",
name_suspend, nr_needed_pages-nr_free_pages());
@@ -726,14 +716,12 @@
*/
drivers_unsuspend();
spin_unlock_irq(&suspend_pagedir_lock);
- PRINTS( "critical section/: done (%d pages copied)\n", nr_copy_pages );
+ printk( "critical section/: done (%d pages copied)\n", nr_copy_pages );
lock_swapdevices();
write_suspend_image();
lock_swapdevices(); /* This will unlock ignored swap devices since writing is finished */
- /* Image is saved, call sync & restart machine */
- PRINTS( "Syncing disks\n" );
/* It is important _NOT_ to umount filesystems at this point. We want
* them synced (in case something goes wrong) but we DO not want to mark
* filesystem clean: it is not. (And it does not matter, if we resume
@@ -745,9 +733,9 @@
void suspend_power_down(void)
{
C_A_D = 0;
- printk(KERN_EMERG "%sTrying to power down.\n", name_suspend);
+ printk(KERN_EMERG "%s%s Trying to power down.\n", name_suspend, TEST_SWSUSP ? "Disable TEST_SWSUSP. NOT ": "");
#ifdef CONFIG_VT
- printk(KERN_EMERG "shift_state: %04x\n", shift_state);
+ PRINTK(KERN_EMERG "shift_state: %04x\n", shift_state);
mdelay(1000);
if (TEST_SWSUSP ^ (!!(shift_state & (1 << KG_CTRL))))
machine_restart(NULL);
@@ -774,7 +762,7 @@
mb();
spin_lock_irq(&suspend_pagedir_lock); /* Done to disable interrupts */
- printk( "Waiting for DMAs to settle down...\n");
+ 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
driver scheduled DMA, we have good chance for DMA to finish ;-). */
@@ -785,7 +773,7 @@
BUG_ON (nr_copy_pages_check != nr_copy_pages);
BUG_ON (pagedir_order_check != pagedir_order);
- PRINTR( "Freeing prev allocated pagedir\n" );
+ PRINTK( "Freeing prev allocated pagedir\n" );
free_suspend_pagedir((unsigned long) pagedir_save);
__flush_tlb_global(); /* Even mappings of "global" things (vmalloc) need to be fixed */
drivers_resume(RESUME_ALL_PHASES);
@@ -813,7 +801,7 @@
if (!suspend_save_image())
suspend_power_down(); /* FIXME: if suspend_power_down is commented out, console is lost after few suspends ?! */
- printk(KERN_WARNING "%sSuspend failed, trying to recover...\n", name_suspend);
+ printk(KERN_EMERG "%sSuspend failed, trying to recover...\n", name_suspend);
MDELAY(1000); /* So user can wait and report us messages if armageddon comes :-) */
barrier();
@@ -826,7 +814,7 @@
drivers_resume(RESUME_PHASE1);
spin_unlock_irq(&suspend_pagedir_lock);
mark_swapfiles(((swp_entry_t) {0}), MARK_SWAP_RESUME);
- printk(KERN_WARNING "%sLeaving do_magic_suspend_2...\n", name_suspend);
+ PRINTK(KERN_WARNING "%sLeaving do_magic_suspend_2...\n", name_suspend);
}
/*
@@ -838,7 +826,7 @@
{
arch_prepare_suspend();
if (prepare_suspend_console())
- printk( "Can't allocate a console... proceeding\n");
+ printk( "%sCan't allocate a console... proceeding\n", name_suspend);
if (!prepare_suspend_processes()) {
free_some_memory();
@@ -847,11 +835,11 @@
* We sync here -- so you have consistent filesystem state when things go wrong.
* -- so that noone writes to disk after we do atomic copy of data.
*/
- PRINTS("Syncing disks before copy\n");
+ PRINTK("Syncing disks before copy\n");
do_suspend_sync();
if(drivers_suspend()==0)
do_magic(0); /* This function returns after machine woken up from resume */
- PRINTR("Restarting processes...\n");
+ PRINTK("Restarting processes...\n");
thaw_processes();
}
software_suspend_enabled = 1;
@@ -939,6 +927,8 @@
void **eaten_memory = NULL;
void **c = eaten_memory, *m, *f;
+ printk("Relocating pagedir");
+
if(!does_collide_order(old_pagedir, (unsigned long)old_pagedir, pagedir_order)) {
printk("not neccessary\n");
return 0;
@@ -968,7 +958,7 @@
if (f)
free_pages((unsigned long)f, pagedir_order);
}
- printk("okay\n");
+ printk("|\n");
return 0;
}
@@ -1029,11 +1019,13 @@
static int resume_try_to_read(const char * specialfile, int noresume)
{
union diskpage *cur;
+ unsigned long scratch_page = 0;
swp_entry_t next;
int i, nr_pgdir_pages, error;
resume_device = name_to_kdev_t(specialfile);
- cur = (void *) get_free_page(GFP_ATOMIC);
+ scratch_page = get_free_page(GFP_ATOMIC);
+ cur = (void *) scratch_page;
if (!cur) {
printk( "%sNot enough memory?\n", name_resume );
error = -ENOMEM;
@@ -1069,6 +1061,41 @@
panic("%sUnable to find suspended-data signature (%.10s - misspelled?\n",
name_resume, cur->swh.magic.magic);
}
+ if(noresume) {
+#if 0
+ struct buffer_head *bh;
+ /* We don't do a sanity check here: we want to restore the swap
+ whatever version of kernel made the suspend image */
+ printk("%s: Fixing swap signatures %s...\n", name_resume, resume_file);
+ /* We need to write swap, but swap is *not* enabled so
+ we must write the device directly */
+ bh = bread(resume_device, 0, PAGE_SIZE);
+ if (!bh || (!bh->b_data)) {
+ error = -EIO;
+ free_page((unsigned long)cur);
+ goto resume_read_error;
+ }
+ if (is_read_only(bh->b_dev)) {
+ printk(name_resume "Can't write to read-only device %s\n",
+ kdevname(bh->b_dev));
+ } else {
+ memcpy(bh->b_data, cur, PAGE_SIZE);
+ generic_make_request(WRITE, bh);
+ wait_on_buffer(bh);
+ if (buffer_uptodate(bh)) {
+ error = 0;
+ brelse(bh);
+ } else {
+ printk(name_resume "Warning %s: Fixing swap signatures unsuccessful...\n", resume_file);
+ error = -EIO;
+ bforget(bh);
+ }
+ }
+#endif
+ }
+
+ if (prepare_suspend_console())
+ printk("%sCan't allocate a console... proceeding\n", name_resume);
printk( "%sSignature found, resuming\n", name_resume );
MDELAY(1000);
@@ -1093,7 +1120,7 @@
if(!pagedir_nosave)
goto resume_read_error;
- PRINTR( "%sReading pagedir, ", name_resume );
+ PRINTK( "%sReading pagedir, ", name_resume );
/* We get pages in reverse order of saving! */
error=-EIO;
@@ -1105,30 +1132,30 @@
}
BUG_ON (next.val);
- printk("Relocating pagedir");
if((error=relocate_pagedir())!=0)
goto resume_read_error;
if((error=check_pagedir())!=0)
goto resume_read_error;
- PRINTK( "image data (%d pages): ", nr_copy_pages );
+ printk( "Reading image data (%d pages): ", nr_copy_pages );
error = -EIO;
for(i=0; i < nr_copy_pages; i++) {
swp_entry_t swap_address = (pagedir_nosave+i)->swap_address;
if (!(i%100))
- PRINTK( "." );
+ printk( "." );
next.val = swp_offset(swap_address) * PAGE_SIZE;
/* You do not need to check for overlaps...
... check_pagedir already did this work */
READTO(next.val, (char *)((pagedir_nosave+i)->address));
}
- PRINTK( " done\n" );
+ printk( "|\n" );
error = 0;
resume_read_error:
+ free_page(scratch_page);
switch (error) {
case 0:
- PRINTR("Reading resume file was successful\n");
+ PRINTK("Reading resume file was successful\n");
break;
case -EINVAL:
break;
@@ -1163,7 +1190,8 @@
printk( "%s", name_resume );
if(resume_status == NORESUME) {
- /* FIXME: Signature should be restored here */
+ if(resume_file[0])
+ resume_try_to_read(resume_file,1);
printk( "disabled\n" );
return;
}
@@ -1173,7 +1201,7 @@
console_loglevel = new_loglevel;
if(!resume_file[0] && resume_status == RESUME_SPECIFIED) {
- printk( "nowhere to resume from\n" );
+ printk( "suspension device unspecified\n" );
return;
}
--
(about SSSCA) "I don't say this lightly. However, I really think that the U.S.
no longer is classifiable as a democracy, but rather as a plutocracy." --hpa
Hi!
Le 1 Jul, Pavel Machek a ?crit :
> + name_resume, cur->swh.magic.magic); /* even with a noresume option, it is better
> + to panic here, because that means that the
> + resume device is not a proper swap one. It
> + is perhaps a linux or dos partition and we
> + don't want to risk damaging it. */
>
> Swapon seems to do its own checks, so this is red herring.
I think we need it anyway, because if we're not on a swap partition and
we have a noresume option, we may force a swap signature just after. The
point is thta this is a misconfiguration of kernel. So I think panic is
a good warning :-)
>
>
>
>
>> 3/6 Fix possible endless loop in ide-suspend stuff when using
>> removable devices
>> 4/6 Fix swap signature in case of noresume option
>> 5/6 Use memeat to make suspension of *big* sessions possible
>> 6/6 Clean SysRq stuff.
>> Clean obsolete PF_KERNTHREAD flag.
>> Manage kernel threads: bdflush, khubd, nfs shares (lockd,
>> rpciod), kjournald, kreiserfsd.
>
> sprintf(current->comm, "lockd");
> -
> + current->flags |= PF_IOTHREAD;
>
> Lockd does not seem any io needed for suspend-to-disk. I guess it
> would be better to freeze it.
>
> strcpy(current->comm, "rpciod");
> -
> + current->flags |= PF_IOTHREAD;
> +
>
> No, this is incorrect. I believe rpciod could submit packet for io in
> time we are freezing devices. If it does that... bye bye to your data.
I think so. At first I did freeze those two tasks but someone post a much simpler patch and... I think you're right. I'll fix that.
>
> Fixing swap signatures should really be done in separate function.
>
> Pavel
> PS: This is what I did in response to your patch (it compiles,
> otherwise untested). I'll try to fix noresume fixing signatures
> somehow.
For 2.5 tree ?
> @@ -604,13 +595,12 @@
>
> static int prepare_suspend_processes(void)
> {
> - PRINTS( "Stopping processes\n" );
> - MDELAY(1000);
> if (freeze_processes()) {
> - PRINTS( "Not all processes stopped!\n" );
> + printk( KERN_ERR "Suspend failed: Not all processes
stopped!\n"
> );
> thaw_processes();
> return 1;
> }
>
> + MDELAY(1000);
> do_suspend_sync();
> return 0;
> }
Where does this MDELAY come from ?
> @@ -1029,11 +1019,13 @@
> static int resume_try_to_read(const char * specialfile, int noresume)
> {
> union diskpage *cur;
> + unsigned long scratch_page = 0;
> swp_entry_t next;
> int i, nr_pgdir_pages, error;
>
> resume_device = name_to_kdev_t(specialfile);
> - cur = (void *) get_free_page(GFP_ATOMIC);
> + scratch_page = get_free_page(GFP_ATOMIC);
> + cur = (void *) scratch_page;
Why doing that in two steps ?
> + if(noresume) {
> +#if 0
I believe this is for 2.5 reasons ;-)
> + struct buffer_head *bh;
> + /* We don't do a sanity check here: we want to restore the swap
> + whatever version of kernel made the suspend image */
> + printk("%s: Fixing swap signatures %s...\n", name_resume, resume_file);
> + /* We need to write swap, but swap is *not* enabled so
> + we must write the device directly */
> + bh = bread(resume_device, 0, PAGE_SIZE);
> + if (!bh || (!bh->b_data)) {
--
Florent Chabaud
Le 30 Jui, Pavel Machek a ?crit :
> Hi!
>
>> Following six patches for swsusp that were used by various people
>> through swsusp list. They should apply in order to 2.4.19-pre10-ac2.
>> Most of those is already in 2.5.22 suspend part managed by Pavel.
>
> Heh, so now I'm going patch by patch to figure what to merge :-(.
Actually, that was intended for Alan to have one patch for one function
(well at least that was the idea ;-)
>
>> 1/6 Documentation and comments cleanings
>
> @@ -1715,9 +1715,9 @@
> S: Germany
>
> N: Gabor Kuti
> -E: [email protected]
> -E: [email protected]
> -D: Author and Maintainer for Software Suspend
> +M: [email protected]
> +M: [email protected]
> +D: Software suspend
>
> N: Jaroslav Kysela
> E: [email protected]
>
> Why do you change E to M here?
That's a mistake. The fact was that Gabor was inserted twice.
--
Florent
Hi!
> > + name_resume, cur->swh.magic.magic); /* even with a noresume option, it is better
> > + to panic here, because that means that the
> > + resume device is not a proper swap one. It
> > + is perhaps a linux or dos partition and we
> > + don't want to risk damaging it. */
> >
> > Swapon seems to do its own checks, so this is red herring.
>
> I think we need it anyway, because if we're not on a swap partition and
> we have a noresume option, we may force a swap signature just after. The
> point is thta this is a misconfiguration of kernel. So I think panic is
> a good warning :-)
Okay.
> >> 3/6 Fix possible endless loop in ide-suspend stuff when using
> >> removable devices
> >> 4/6 Fix swap signature in case of noresume option
> >> 5/6 Use memeat to make suspension of *big* sessions possible
> >> 6/6 Clean SysRq stuff.
> >> Clean obsolete PF_KERNTHREAD flag.
> >> Manage kernel threads: bdflush, khubd, nfs shares (lockd,
> >> rpciod), kjournald, kreiserfsd.
> >
> > sprintf(current->comm, "lockd");
> > -
> > + current->flags |= PF_IOTHREAD;
> >
> > Lockd does not seem any io needed for suspend-to-disk. I guess it
> > would be better to freeze it.
> >
> > strcpy(current->comm, "rpciod");
> > -
> > + current->flags |= PF_IOTHREAD;
> > +
> >
> > No, this is incorrect. I believe rpciod could submit packet for io in
> > time we are freezing devices. If it does that... bye bye to your data.
>
>
> I think so. At first I did freeze those two tasks but someone post a much simpler patch and... I think you're right. I'll fix that.
>
Thanks.
> > PS: This is what I did in response to your patch (it compiles,
> > otherwise untested). I'll try to fix noresume fixing signatures
> > somehow.
>
> For 2.5 tree ?
Yep.
> > @@ -604,13 +595,12 @@
> >
> > static int prepare_suspend_processes(void)
> > {
> > - PRINTS( "Stopping processes\n" );
> > - MDELAY(1000);
> > if (freeze_processes()) {
> > - PRINTS( "Not all processes stopped!\n" );
> > + printk( KERN_ERR "Suspend failed: Not all processes
> stopped!\n"
> > );
> > thaw_processes();
> > return 1;
> > }
> >
> > + MDELAY(1000);
> > do_suspend_sync();
> > return 0;
> > }
>
>
> Where does this MDELAY come from ?
>From top of the function.
> > @@ -1029,11 +1019,13 @@
> > static int resume_try_to_read(const char * specialfile, int noresume)
> > {
> > union diskpage *cur;
> > + unsigned long scratch_page = 0;
> > swp_entry_t next;
> > int i, nr_pgdir_pages, error;
> >
> > resume_device = name_to_kdev_t(specialfile);
> > - cur = (void *) get_free_page(GFP_ATOMIC);
> > + scratch_page = get_free_page(GFP_ATOMIC);
> > + cur = (void *) scratch_page;
>
> Why doing that in two steps ?
I do not think I can write that into one expression...
>
> > + if(noresume) {
> > +#if 0
>
> I believe this is for 2.5 reasons ;-)
It will not compile; small reshuffling is needed to address this.
Pavel
--
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.
Hi!
> > No, this is incorrect. I believe rpciod could submit packet for io in
> > time we are freezing devices. If it does that... bye bye to your data.
>
>
> I think so. At first I did freeze those two tasks but someone post a much simpler patch and... I think you're right. I'll fix that.
>
Mail me a patch when you have that.
> > Fixing swap signatures should really be done in separate function.
> >
> > Pavel
> > PS: This is what I did in response to your patch (it compiles,
> > otherwise untested). I'll try to fix noresume fixing signatures
> > somehow.
>
> For 2.5 tree ?
Yep. [Actually noresume fixing signatures is harder than I expected.]
> > @@ -604,13 +595,12 @@
> >
> > static int prepare_suspend_processes(void)
> > {
> > - PRINTS( "Stopping processes\n" );
> > - MDELAY(1000);
> > if (freeze_processes()) {
> > - PRINTS( "Not all processes stopped!\n" );
> > + printk( KERN_ERR "Suspend failed: Not all processes
> stopped!\n"
> > );
> > thaw_processes();
> > return 1;
> > }
> >
> > + MDELAY(1000);
> > do_suspend_sync();
> > return 0;
> > }
>
>
> Where does this MDELAY come from ?
>From right above.
> > @@ -1029,11 +1019,13 @@
> > static int resume_try_to_read(const char * specialfile, int noresume)
> > {
> > union diskpage *cur;
> > + unsigned long scratch_page = 0;
> > swp_entry_t next;
> > int i, nr_pgdir_pages, error;
> >
> > resume_device = name_to_kdev_t(specialfile);
> > - cur = (void *) get_free_page(GFP_ATOMIC);
> > + scratch_page = get_free_page(GFP_ATOMIC);
> > + cur = (void *) scratch_page;
>
> Why doing that in two steps ?
So we can free scratch_page without ugly casts.
> > + if(noresume) {
> > +#if 0
>
> I believe this is for 2.5 reasons ;-)
Yes.
Pavel
PS: Killed Alan from Cc, he reads lists anyway and I guess he's not
so much interested.
--
Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?
Le 23 Jul, Pavel Machek a ?crit :
> Hi!
>
>> > No, this is incorrect. I believe rpciod could submit packet for io in
>> > time we are freezing devices. If it does that... bye bye to your data.
>>
>>
>> I think so. At first I did freeze those two tasks but someone post a much simpler patch and... I think you're right. I'll fix that.
>>
>
> Mail me a patch when you have that.
That's in beta9. Have a look at the incremental patch.
>
>> > Fixing swap signatures should really be done in separate function.
>> >
>> > Pavel
>> > PS: This is what I did in response to your patch (it compiles,
>> > otherwise untested). I'll try to fix noresume fixing signatures
>> > somehow.
>>
>> For 2.5 tree ?
>
> Yep. [Actually noresume fixing signatures is harder than I expected.]
Astonishing. Actually with 2.4 I found it easier than I thought.
> PS: Killed Alan from Cc, he reads lists anyway and I guess he's not
> so much interested.
OK.
--
Florent