* [PATCH 0/3] swsusp: fix some obscure bugs
@ 2005-09-25 18:18 Rafael J. Wysocki
2005-09-25 18:24 ` [PATCH 1/3][Fix] swsusp: remove wrong code from data_free Rafael J. Wysocki
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Rafael J. Wysocki @ 2005-09-25 18:18 UTC (permalink / raw)
To: Pavel Machek; +Cc: LKML, Andrew Morton
Hi,
The following series of patches fixes some obscure bugs in swsusp.
Greetings,
Rafael
--
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
-- Lewis Carroll "Alice's Adventures in Wonderland"
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/3][Fix] swsusp: remove wrong code from data_free 2005-09-25 18:18 [PATCH 0/3] swsusp: fix some obscure bugs Rafael J. Wysocki @ 2005-09-25 18:24 ` Rafael J. Wysocki 2005-09-25 18:30 ` [PATCH 2/3][Fix] swsusp: prevent possible memory leak Rafael J. Wysocki ` (2 subsequent siblings) 3 siblings, 0 replies; 15+ messages in thread From: Rafael J. Wysocki @ 2005-09-25 18:24 UTC (permalink / raw) To: Pavel Machek; +Cc: LKML, Andrew Morton The following patch removes some wrong code from the data_free() function in swsusp. This function could only be called if there's an arror while writing the suspend image to swap, so it is not triggered easily. However, if triggered, it would probably corrupt some memory. Please apply, Rafael Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Index: linux-2.6.14-rc2-git3/kernel/power/swsusp.c =================================================================== --- linux-2.6.14-rc2-git3.orig/kernel/power/swsusp.c 2005-09-25 18:45:03.000000000 +0200 +++ linux-2.6.14-rc2-git3/kernel/power/swsusp.c 2005-09-25 18:49:00.000000000 +0200 @@ -402,15 +402,14 @@ static void data_free(void) { swp_entry_t entry; - int i; + struct pbe * p; - for (i = 0; i < nr_copy_pages; i++) { - entry = (pagedir_nosave + i)->swap_address; + for_each_pbe(p, pagedir_nosave) { + entry = p->swap_address; if (entry.val) swap_free(entry); else break; - (pagedir_nosave + i)->swap_address = (swp_entry_t){0}; } } ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/3][Fix] swsusp: prevent possible memory leak 2005-09-25 18:18 [PATCH 0/3] swsusp: fix some obscure bugs Rafael J. Wysocki 2005-09-25 18:24 ` [PATCH 1/3][Fix] swsusp: remove wrong code from data_free Rafael J. Wysocki @ 2005-09-25 18:30 ` Rafael J. Wysocki 2005-09-25 18:44 ` [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages Rafael J. Wysocki 2005-09-25 21:47 ` [PATCH 0/3] swsusp: fix some obscure bugs Pavel Machek 3 siblings, 0 replies; 15+ messages in thread From: Rafael J. Wysocki @ 2005-09-25 18:30 UTC (permalink / raw) To: Pavel Machek; +Cc: LKML, Andrew Morton The following patch prevents swsusp from leaking some memory in case of an error in read_pagedir(). It also prevents the BUG_ON() from triggering if there's an error while reading swap. Please apply, Rafael Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Index: linux-2.6.14-rc2-git3/kernel/power/swsusp.c =================================================================== --- linux-2.6.14-rc2-git3.orig/kernel/power/swsusp.c 2005-09-25 18:49:00.000000000 +0200 +++ linux-2.6.14-rc2-git3/kernel/power/swsusp.c 2005-09-25 18:52:48.000000000 +0200 @@ -1437,9 +1437,9 @@ } if (error) - free_page((unsigned long)pblist); - - BUG_ON(i != swsusp_info.pagedir_pages); + free_pagedir(pblist); + else + BUG_ON(i != swsusp_info.pagedir_pages); return error; } ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages 2005-09-25 18:18 [PATCH 0/3] swsusp: fix some obscure bugs Rafael J. Wysocki 2005-09-25 18:24 ` [PATCH 1/3][Fix] swsusp: remove wrong code from data_free Rafael J. Wysocki 2005-09-25 18:30 ` [PATCH 2/3][Fix] swsusp: prevent possible memory leak Rafael J. Wysocki @ 2005-09-25 18:44 ` Rafael J. Wysocki 2005-09-26 10:33 ` Pavel Machek 2005-09-25 21:47 ` [PATCH 0/3] swsusp: fix some obscure bugs Pavel Machek 3 siblings, 1 reply; 15+ messages in thread From: Rafael J. Wysocki @ 2005-09-25 18:44 UTC (permalink / raw) To: Pavel Machek; +Cc: LKML, Andrew Morton There's a silent assumption in swsusp that always sizeof(struct swsusp_info) <= PAGE_SIZE, which is wrong, because eg. on x86-64 sizeof(swp_entry_t) = 8. This causes swsusp to skip some pagedir pages while reading the image if there are too many of them (depending on the architecture, approx. 500 on x86-64). The following patch makes swsusp more flexible with this respect, by allocating separate swap page(s) (as many as needed) for storing the swap offsets of pagedir pages (without the patch they are all stored in a struct swsusp_info, and there may be not enough room for them in this structure). Please consider for applying, Rafael Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Index: linux-2.6.14-rc2-git3/kernel/power/power.h =================================================================== --- linux-2.6.14-rc2-git3.orig/kernel/power/power.h 2005-09-25 18:42:31.000000000 +0200 +++ linux-2.6.14-rc2-git3/kernel/power/power.h 2005-09-25 18:56:17.000000000 +0200 @@ -9,6 +9,7 @@ #define SUSPEND_CONSOLE (MAX_NR_CONSOLES-1) #endif +#define MAX_PB_SWP_PAGES 128 struct swsusp_info { struct new_utsname uts; @@ -18,10 +19,12 @@ unsigned long image_pages; unsigned long pagedir_pages; suspend_pagedir_t * suspend_pagedir; - swp_entry_t pagedir[768]; + unsigned pb_swp_pages; + swp_entry_t pb_swp[MAX_PB_SWP_PAGES]; + swp_entry_t * pb_swp_mem[MAX_PB_SWP_PAGES]; } __attribute__((aligned(PAGE_SIZE))); - +/* pb_swp_mem and pb_swp_pages are needed for error recovery */ #ifdef CONFIG_SOFTWARE_SUSPEND extern int pm_suspend_disk(void); Index: linux-2.6.14-rc2-git3/kernel/power/swsusp.c =================================================================== --- linux-2.6.14-rc2-git3.orig/kernel/power/swsusp.c 2005-09-25 18:52:48.000000000 +0200 +++ linux-2.6.14-rc2-git3/kernel/power/swsusp.c 2005-09-25 19:10:16.000000000 +0200 @@ -496,10 +496,17 @@ static void free_pagedir_entries(void) { - int i; + swp_entry_t **ptr, *entry; + int j, k; - for (i = 0; i < swsusp_info.pagedir_pages; i++) - swap_free(swsusp_info.pagedir[i]); + for (j = 0, ptr = swsusp_info.pb_swp_mem; + j < swsusp_info.pb_swp_pages; j++, ptr++) { + for (k = 0, entry = *ptr; + k < PAGE_SIZE/sizeof(swp_entry_t); k++, entry++) + if (entry->val) + swap_free(*entry); + free_page((unsigned long)*ptr); + } } @@ -510,18 +517,49 @@ static int write_pagedir(void) { - int error = 0; - unsigned n = 0; + int error = -EFAULT; + unsigned k, n, pages = 0; struct pbe * pbe; + swp_entry_t * buf; + + if (!(buf = (swp_entry_t *)get_zeroed_page(GFP_KERNEL))) + return -ENOMEM; printk( "Writing pagedir..."); + n = 0; + swsusp_info.pb_swp_mem[0] = buf; + swsusp_info.pb_swp_pages = 1; + k = 0; for_each_pb_page (pbe, pagedir_nosave) { - if ((error = write_page((unsigned long)pbe, &swsusp_info.pagedir[n++]))) + error = write_page((unsigned long)pbe, &buf[k++]); + if (!error) { + pages++; + if (k >= PAGE_SIZE/sizeof(swp_entry_t)) { + error = write_page((unsigned long)buf, &swsusp_info.pb_swp[n++]); + if (n >= MAX_PB_SWP_PAGES) + error = -ENOMEM; + if (!error) { + buf = (swp_entry_t *)get_zeroed_page(GFP_KERNEL); + if (buf) { + swsusp_info.pb_swp_mem[n] = buf; + swsusp_info.pb_swp_pages++; + k = 0; + } else + error = -ENOMEM; + } + } + } + if (error) return error; } + if (k > 0) + error = write_page((unsigned long)buf, &swsusp_info.pb_swp[n++]); + + if (!error) { + swsusp_info.pagedir_pages = pages; + printk("done (%u pages)\n", pages); + } - swsusp_info.pagedir_pages = n; - printk("done (%u pages)\n", n); return error; } @@ -1414,33 +1452,58 @@ static int read_pagedir(struct pbe *pblist) { struct pbe *pbpage, *p; - unsigned i = 0; - int error; + unsigned k, n = 0, pages = 0; + int error = -EFAULT; + swp_entry_t * buf; + unsigned long offset; if (!pblist) return -EFAULT; + if (!(buf = (swp_entry_t *)get_zeroed_page(GFP_KERNEL))) + return -ENOMEM; + printk("swsusp: Reading pagedir (%lu pages)\n", swsusp_info.pagedir_pages); - for_each_pb_page (pbpage, pblist) { - unsigned long offset = swp_offset(swsusp_info.pagedir[i++]); + if (!(offset = swp_offset(swsusp_info.pb_swp[n++]))) + goto free_buf; + if ((error = bio_read_page(offset, (swp_entry_t *)buf))) + goto free_buf; + + k = 0; + for_each_pb_page (pbpage, pblist) { error = -EFAULT; + offset = swp_offset(buf[k++]); if (offset) { p = (pbpage + PB_PAGE_SKIP)->next; error = bio_read_page(offset, (void *)pbpage); (pbpage + PB_PAGE_SKIP)->next = p; } - if (error) + if (!error) { + pages++; + if (k >= PAGE_SIZE/sizeof(swp_entry_t)) { + error = -EFAULT; + offset = swp_offset(swsusp_info.pb_swp[n++]); + if (offset) { + error = bio_read_page(offset, (swp_entry_t *)buf); + k = 0; + } + if (error) + break; + } + } else break; } +free_buf: if (error) free_pagedir(pblist); else - BUG_ON(i != swsusp_info.pagedir_pages); + BUG_ON(pages != swsusp_info.pagedir_pages); + free_page((unsigned long)buf); return error; } ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages 2005-09-25 18:44 ` [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages Rafael J. Wysocki @ 2005-09-26 10:33 ` Pavel Machek 2005-09-26 11:11 ` Rafael J. Wysocki 0 siblings, 1 reply; 15+ messages in thread From: Pavel Machek @ 2005-09-26 10:33 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: LKML, Andrew Morton Hi! > There's a silent assumption in swsusp that always > sizeof(struct swsusp_info) <= PAGE_SIZE, which is wrong, because > eg. on x86-64 sizeof(swp_entry_t) = 8. This causes swsusp to skip some pagedir > pages while reading the image if there are too many of them (depending on the > architecture, approx. 500 on x86-64). Last time I did the math, swsusp_info could cover a *lot* of memory. It was wrong not to check for overflow, but I do not think we want to introduce *yet another* linklist. Lets see... for i386, we have 768 pagedir entries. Each pagedir entry points to page with 1023 pointers to pages. That means that up-to 768*1023*4096 bytes image can be saved to swap ~= 768 * 1K * 4K ~= 3 GB. That's more than enough for i386. for x86-64, we can have 128 pagedir entries (could not we fit more there? 384 entries should fit, no?). Each pagedir entry has 511 pointers to pages (IIRC)... that is up-to 128*511*4K ~= 64*1K*4K = 256 MB image. Hmm, that should still be enough for any 512MB machine, and probably okay for much bigger machines, too... We can still get to 768 MB image (good enough for any 1.5GB machine, and probably for anything else, too). If that is not good enough for you, can you simply allocate more than 1 page for swsusp_info? No need for linklists yet. Andrew, please drop this one. It is too complex solution for quite a simple problem. Pavel -- if you have sharp zaurus hardware you don't need... you know my address ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages 2005-09-26 10:33 ` Pavel Machek @ 2005-09-26 11:11 ` Rafael J. Wysocki 2005-09-26 11:20 ` Pavel Machek 0 siblings, 1 reply; 15+ messages in thread From: Rafael J. Wysocki @ 2005-09-26 11:11 UTC (permalink / raw) To: Pavel Machek; +Cc: LKML, Andrew Morton Hi, On Monday, 26 of September 2005 12:33, Pavel Machek wrote: > Hi! > > > There's a silent assumption in swsusp that always > > sizeof(struct swsusp_info) <= PAGE_SIZE, which is wrong, because > > eg. on x86-64 sizeof(swp_entry_t) = 8. This causes swsusp to skip some pagedir > > pages while reading the image if there are too many of them (depending on the > > architecture, approx. 500 on x86-64). > > Last time I did the math, swsusp_info could cover a *lot* of > memory. It was wrong not to check for overflow, but I do not think we > want to introduce *yet another* linklist. Yes, I thought of another solution, but any of them would require more than one swap page and I'd have to track the swap offsets of them somehow. > Lets see... > > for i386, we have 768 pagedir entries. Each pagedir entry points to > page with 1023 pointers to pages. That means that up-to 768*1023*4096 > bytes image can be saved to swap ~= 768 * 1K * 4K ~= 3 GB. That's more > than enough for i386. > > for x86-64, we can have 128 pagedir entries (could not we fit more > there? 384 entries should fit, no?). Yes. To be exact, 460. > Each pagedir entry has 511 pointers to pages (IIRC)... 512, I think. > that is up-to 128*511*4K ~= 64*1K*4K = 256 MB image. > Hmm, that should still be enough for any 512MB machine, and > probably okay for much bigger machines, too... > > We can still get to 768 MB image (good enough for any 1.5GB machine, > and probably for anything else, too). > > If that is not good enough for you, can you simply allocate more than > 1 page for swsusp_info? No need for linklists yet. I can. The problem is I have to track the swap offsets of these pages which is necessary for resume. Is it guaranteed that the swap offsets of pages allocated in a row will be consecutive? > Andrew, please drop this one. It is too complex solution for quite a > simple problem. Perhaps it is. Anyway the problem hit me when I was playing with swsusp on a machine with 768 MB of RAM. Greetings, Rafael ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages 2005-09-26 11:11 ` Rafael J. Wysocki @ 2005-09-26 11:20 ` Pavel Machek 2005-09-26 12:54 ` Rafael J. Wysocki 0 siblings, 1 reply; 15+ messages in thread From: Pavel Machek @ 2005-09-26 11:20 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: LKML, Andrew Morton Hi! > > Lets see... > > > > for i386, we have 768 pagedir entries. Each pagedir entry points to > > page with 1023 pointers to pages. That means that up-to 768*1023*4096 > > bytes image can be saved to swap ~= 768 * 1K * 4K ~= 3 GB. That's more > > than enough for i386. > > > > for x86-64, we can have 128 pagedir entries (could not we fit more > > there? 384 entries should fit, no?). > > Yes. To be exact, 460. > > > Each pagedir entry has 511 pointers to pages (IIRC)... > > 512, I think. Okay, can we do simple solution where we put 460 there, plus a check if it overflows (printk, abort suspend), for now? That should fix 768MB machine... > > that is up-to 128*511*4K ~= 64*1K*4K = 256 MB image. > > Hmm, that should still be enough for any 512MB machine, and > > probably okay for much bigger machines, too... > > > > We can still get to 768 MB image (good enough for any 1.5GB machine, > > and probably for anything else, too). > > > > If that is not good enough for you, can you simply allocate more than > > 1 page for swsusp_info? No need for linklists yet. > > I can. The problem is I have to track the swap offsets of these pages > which is necessary for resume. Is it guaranteed that the swap offsets > of pages allocated in a row will be consecutive? No, they probably will not be consecutive. OTOH pagedirs are stored as a link list in memory already. Maybe we should be able to extend that link list for a disk, too, with minimal fuss? ...we'd have to write pagedir _backwards_ for that to work, probably not nice, and swap_free() would really like direct access. Pavel -- if you have sharp zaurus hardware you don't need... you know my address ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages 2005-09-26 11:20 ` Pavel Machek @ 2005-09-26 12:54 ` Rafael J. Wysocki 2005-09-26 14:26 ` Pavel Machek 0 siblings, 1 reply; 15+ messages in thread From: Rafael J. Wysocki @ 2005-09-26 12:54 UTC (permalink / raw) To: Pavel Machek; +Cc: LKML, Andrew Morton Hi, On Monday, 26 of September 2005 13:20, Pavel Machek wrote: > Hi! > > > > Lets see... > > > > > > for i386, we have 768 pagedir entries. Each pagedir entry points to > > > page with 1023 pointers to pages. That means that up-to 768*1023*4096 > > > bytes image can be saved to swap ~= 768 * 1K * 4K ~= 3 GB. That's more > > > than enough for i386. > > > > > > for x86-64, we can have 128 pagedir entries (could not we fit more > > > there? 384 entries should fit, no?). > > > > Yes. To be exact, 460. > > > > > Each pagedir entry has 511 pointers to pages (IIRC)... > > > > 512, I think. > > Okay, can we do simple solution where we put 460 there, plus a check > if it overflows (printk, abort suspend), for now? That should fix > 768MB machine... For now: a constant in power.h depending on sizeof(long) and PAGE_SIZE, the size of swsusp_info.pagedir[] depending on it and the overflow check in write_pagedir()? Unfortunately it's not enough for what I'm cooking (think of resuming in 35 sec. to a fully responsive system - well, that's on my box). A preliminary patch is at http://www.sisk.pl/kernel/patches/2.6.14-rc2-git3/swsusp-improve-freeing-memory.patch > > > that is up-to 128*511*4K ~= 64*1K*4K = 256 MB image. > > > Hmm, that should still be enough for any 512MB machine, and > > > probably okay for much bigger machines, too... > > > > > > We can still get to 768 MB image (good enough for any 1.5GB machine, > > > and probably for anything else, too). > > > > > > If that is not good enough for you, can you simply allocate more than > > > 1 page for swsusp_info? No need for linklists yet. > > > > I can. The problem is I have to track the swap offsets of these pages > > which is necessary for resume. Is it guaranteed that the swap offsets > > of pages allocated in a row will be consecutive? > > No, they probably will not be consecutive. > > OTOH pagedirs are stored as a link list in memory already. Maybe we > should be able to extend that link list for a disk, too, with minimal > fuss? ...we'd have to write pagedir _backwards_ for that to work, > probably not nice, and swap_free() would really like direct access. We write the pagedir after we have written the image, so the address field of each entry is not needed at that time, except for freeing the image memory in case of failure (with the "rework image freeing patch" they are not needed at all). Thus potentially we can use the address fields of pagedir entries to link the pages on the swap. Greetings, Rafael ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages 2005-09-26 12:54 ` Rafael J. Wysocki @ 2005-09-26 14:26 ` Pavel Machek 2005-09-26 19:19 ` Rafael J. Wysocki 2005-09-26 19:29 ` [PATCH][Fix] swsusp: avoid problems if there are too many pages to save Rafael J. Wysocki 0 siblings, 2 replies; 15+ messages in thread From: Pavel Machek @ 2005-09-26 14:26 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: LKML, Andrew Morton Hi! > > > > Lets see... > > > > > > > > for i386, we have 768 pagedir entries. Each pagedir entry points to > > > > page with 1023 pointers to pages. That means that up-to 768*1023*4096 > > > > bytes image can be saved to swap ~= 768 * 1K * 4K ~= 3 GB. That's more > > > > than enough for i386. > > > > > > > > for x86-64, we can have 128 pagedir entries (could not we fit more > > > > there? 384 entries should fit, no?). > > > > > > Yes. To be exact, 460. > > > > > > > Each pagedir entry has 511 pointers to pages (IIRC)... > > > > > > 512, I think. > > > > Okay, can we do simple solution where we put 460 there, plus a check > > if it overflows (printk, abort suspend), for now? That should fix > > 768MB machine... > > For now: a constant in power.h depending on sizeof(long) and PAGE_SIZE, > the size of swsusp_info.pagedir[] depending on it and the overflow check > in write_pagedir()? Yes, that would be very nice. > Unfortunately it's not enough for what I'm cooking (think of resuming in 35 sec. > to a fully responsive system - well, that's on my box). A preliminary patch > is at http://www.sisk.pl/kernel/patches/2.6.14-rc2-git3/swsusp-improve-freeing-memory.patch Okay, I see, nice... We want to support that in future. (Actually it is last piece of puzzle for swsusp3). > > No, they probably will not be consecutive. > > > > OTOH pagedirs are stored as a link list in memory already. Maybe we > > should be able to extend that link list for a disk, too, with minimal > > fuss? ...we'd have to write pagedir _backwards_ for that to work, > > probably not nice, and swap_free() would really like direct access. > > We write the pagedir after we have written the image, so the address > field of each entry is not needed at that time, except for freeing the > image memory in case of failure (with the "rework image freeing patch" > they are not needed at all). Thus potentially we can use the address > fields of pagedir entries to link the pages on the swap. I plan to push "rework image freeing patch" for other reasons, too. I'd like to run longer tests on it, but so far it looks okay. Pavel -- if you have sharp zaurus hardware you don't need... you know my address ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages 2005-09-26 14:26 ` Pavel Machek @ 2005-09-26 19:19 ` Rafael J. Wysocki 2005-09-26 23:22 ` Pavel Machek 2005-09-26 19:29 ` [PATCH][Fix] swsusp: avoid problems if there are too many pages to save Rafael J. Wysocki 1 sibling, 1 reply; 15+ messages in thread From: Rafael J. Wysocki @ 2005-09-26 19:19 UTC (permalink / raw) To: Pavel Machek; +Cc: LKML, Andrew Morton Hi, On Monday, 26 of September 2005 16:26, Pavel Machek wrote: > Hi! > ]-- snip --[ > > > Unfortunately it's not enough for what I'm cooking (think of resuming in 35 sec. > > to a fully responsive system - well, that's on my box). A preliminary patch > > is at http://www.sisk.pl/kernel/patches/2.6.14-rc2-git3/swsusp-improve-freeing-memory.patch > > Okay, I see, nice... We want to support that in future. (Actually it > is last piece of puzzle for swsusp3). Well, it can be implemented within the current swsusp, IMO. It actually works for me now. :-) ]-- snip --[ > > I plan to push "rework image freeing patch" for other reasons, > too. I'd like to run longer tests on it, but so far it looks okay. I think it's fine. Could you please point me to the current version? Greetings, Rafael ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages 2005-09-26 19:19 ` Rafael J. Wysocki @ 2005-09-26 23:22 ` Pavel Machek 0 siblings, 0 replies; 15+ messages in thread From: Pavel Machek @ 2005-09-26 23:22 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: LKML Hi! [Andrew killed from Cc, lets not flood his mailbox]. > > > Unfortunately it's not enough for what I'm cooking (think of resuming in 35 sec. > > > to a fully responsive system - well, that's on my box). A preliminary patch > > > is at http://www.sisk.pl/kernel/patches/2.6.14-rc2-git3/swsusp-improve-freeing-memory.patch > > > > Okay, I see, nice... We want to support that in future. (Actually it > > is last piece of puzzle for swsusp3). > > Well, it can be implemented within the current swsusp, IMO. It actually > works for me now. :-) Agreed, and swsusp3 provides graphical progress bar, esc-to-escape, and "are you sure you want to crash your machine" dialogs :-). > > I plan to push "rework image freeing patch" for other reasons, > > too. I'd like to run longer tests on it, but so far it looks okay. > > I think it's fine. Could you please point me to the current version? Well, its a bit complicated. I have 3 related trees on kernel.org: linux-good -- that are basically patches I either already sent upstream or that I plan to send very soon. Unfortunately I did not yet have time to apply all your stuff that you sent me for akpm; I'll need to pull them from -mm and apply here. linux -- that is my working tree. It contains some stuff you probably do not want (like special keymap). linux-sw3 -- and that's a tree to test suspending from userland, aka swsusp3 (aka uswsusp). Currently stuck at 2.6.13 ("first make it work, then port it to HEAD"), and it seems to work (but you should really know what you are doing, and use ext2). usr/swsusp.c is the userland program to do suspending. Pavel -- if you have sharp zaurus hardware you don't need... you know my address ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH][Fix] swsusp: avoid problems if there are too many pages to save 2005-09-26 14:26 ` Pavel Machek 2005-09-26 19:19 ` Rafael J. Wysocki @ 2005-09-26 19:29 ` Rafael J. Wysocki 2005-09-26 23:14 ` Pavel Machek 1 sibling, 1 reply; 15+ messages in thread From: Rafael J. Wysocki @ 2005-09-26 19:29 UTC (permalink / raw) To: Pavel Machek; +Cc: LKML, Andrew Morton Hi, The following patch makes swsusp avoid problems during resume if there are too many pages to save on suspend. It adds a constant that allows us to verify if we are going to save too many pages and implements the check (this is done as early as we can tell that the check will trigger, which is in swsusp_alloc()). This is to replace swsusp-prevent-swsusp-from-failing-if-theres-too-many-pagedir-pages.patch Please consider for applying. Greetings, Rafael Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> Index: linux-2.6.14-rc2-git6/kernel/power/power.h =================================================================== --- linux-2.6.14-rc2-git6.orig/kernel/power/power.h 2005-09-26 20:58:33.000000000 +0200 +++ linux-2.6.14-rc2-git6/kernel/power/power.h 2005-09-26 21:05:37.000000000 +0200 @@ -9,6 +9,9 @@ #define SUSPEND_CONSOLE (MAX_NR_CONSOLES-1) #endif +#define MAX_PBES ((PAGE_SIZE - sizeof(struct new_utsname) \ + - 4 - 3*sizeof(unsigned long) - sizeof(int) \ + - sizeof(void *)) / sizeof(swp_entry_t)) struct swsusp_info { struct new_utsname uts; @@ -18,7 +21,7 @@ unsigned long image_pages; unsigned long pagedir_pages; suspend_pagedir_t * suspend_pagedir; - swp_entry_t pagedir[768]; + swp_entry_t pagedir[MAX_PBES]; } __attribute__((aligned(PAGE_SIZE))); Index: linux-2.6.14-rc2-git6/kernel/power/swsusp.c =================================================================== --- linux-2.6.14-rc2-git6.orig/kernel/power/swsusp.c 2005-09-26 20:59:30.000000000 +0200 +++ linux-2.6.14-rc2-git6/kernel/power/swsusp.c 2005-09-26 21:05:13.000000000 +0200 @@ -931,6 +931,10 @@ if (!enough_swap()) return -ENOSPC; + if (MAX_PBES < nr_copy_pages / PBES_PER_PAGE + + !!(nr_copy_pages % PBES_PER_PAGE)) + return -ENOSPC; + if (!(pagedir_save = alloc_pagedir(nr_copy_pages))) { printk(KERN_ERR "suspend: Allocating pagedir failed.\n"); return -ENOMEM; ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH][Fix] swsusp: avoid problems if there are too many pages to save 2005-09-26 19:29 ` [PATCH][Fix] swsusp: avoid problems if there are too many pages to save Rafael J. Wysocki @ 2005-09-26 23:14 ` Pavel Machek 0 siblings, 0 replies; 15+ messages in thread From: Pavel Machek @ 2005-09-26 23:14 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: LKML, Andrew Morton Hi! > The following patch makes swsusp avoid problems during resume if there are too many > pages to save on suspend. It adds a constant that allows us to verify if we are going to > save too many pages and implements the check (this is done as early as we can tell that > the check will trigger, which is in swsusp_alloc()). > > This is to replace swsusp-prevent-swsusp-from-failing-if-theres-too-many-pagedir-pages.patch > > Please consider for applying. Andrew, if you could apply this one, it would be great. ACKed-by: Pavel Machek <pavel@suse.cz> Pavel -- if you have sharp zaurus hardware you don't need... you know my address ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] swsusp: fix some obscure bugs 2005-09-25 18:18 [PATCH 0/3] swsusp: fix some obscure bugs Rafael J. Wysocki ` (2 preceding siblings ...) 2005-09-25 18:44 ` [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages Rafael J. Wysocki @ 2005-09-25 21:47 ` Pavel Machek 2005-09-25 21:59 ` Andrew Morton 3 siblings, 1 reply; 15+ messages in thread From: Pavel Machek @ 2005-09-25 21:47 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: LKML, Andrew Morton Hi! > The following series of patches fixes some obscure bugs in swsusp. Andrew, could you apply parts #1 and #2? First is data-corruption in error path (nasty!), second is simple bugfix. Alternatively, I'll eat them into my tree and mail you proper patches with all signed-offs, but that will be tommorow. (Have to get some sleep now.) Pavel -- if you have sharp zaurus hardware you don't need... you know my address ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/3] swsusp: fix some obscure bugs 2005-09-25 21:47 ` [PATCH 0/3] swsusp: fix some obscure bugs Pavel Machek @ 2005-09-25 21:59 ` Andrew Morton 0 siblings, 0 replies; 15+ messages in thread From: Andrew Morton @ 2005-09-25 21:59 UTC (permalink / raw) To: Pavel Machek; +Cc: rjw, linux-kernel Pavel Machek <pavel@ucw.cz> wrote: > > > The following series of patches fixes some obscure bugs in swsusp. > > Andrew, could you apply parts #1 and #2? First is data-corruption in > error path (nasty!), second is simple bugfix. Sure. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2005-09-26 23:23 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2005-09-25 18:18 [PATCH 0/3] swsusp: fix some obscure bugs Rafael J. Wysocki 2005-09-25 18:24 ` [PATCH 1/3][Fix] swsusp: remove wrong code from data_free Rafael J. Wysocki 2005-09-25 18:30 ` [PATCH 2/3][Fix] swsusp: prevent possible memory leak Rafael J. Wysocki 2005-09-25 18:44 ` [PATCH 3/3][Fix] swsusp: prevent swsusp from failing if there's too many pagedir pages Rafael J. Wysocki 2005-09-26 10:33 ` Pavel Machek 2005-09-26 11:11 ` Rafael J. Wysocki 2005-09-26 11:20 ` Pavel Machek 2005-09-26 12:54 ` Rafael J. Wysocki 2005-09-26 14:26 ` Pavel Machek 2005-09-26 19:19 ` Rafael J. Wysocki 2005-09-26 23:22 ` Pavel Machek 2005-09-26 19:29 ` [PATCH][Fix] swsusp: avoid problems if there are too many pages to save Rafael J. Wysocki 2005-09-26 23:14 ` Pavel Machek 2005-09-25 21:47 ` [PATCH 0/3] swsusp: fix some obscure bugs Pavel Machek 2005-09-25 21:59 ` Andrew Morton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox