Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
On Sun, Feb 13, 2005 at 01:04:56PM +0706, [EMAIL PROTECTED] wrote:
> On Wed, Feb 09, 2005 at 12:22:52AM +0100, Rafael J. Wysocki wrote:
> > On Tuesday, 8 of February 2005 23:42, Pavel Machek wrote:
> > > Hi!
> > >
> > > > +static inline void eat_page(void *page) {
> > >
> > > Please put { on new line.
> >
> > Oh, I still tend to forget about this. Corrected in the patch that is
> > available on the web
> > (http://www.sisk.pl/kernel/patches/2.6.11-rc3-mm1/swsusp-use-list-resume-v4.patch).
> >
> >
> > > Okay, as you can see, I could find very little wrong with this
> > > patch. That hopefully means it is okay ;-). I should still check error
> > > handling, but I guess I'll do it when it is applied because it is hard
> > > to do on a diff. I guess it should go into -mm just after 2.6.11 is
> > > released...
> >
> > That would be great!
> >
> > Greets,
> > Rafael
>
> Here is powerpc port support for that. Thanks for greate patch.
>
Sorry I forgot this one.
--- 2.6.11-rc3-mm2-use-list-resume-v4/arch/ppc/kernel/asm-offsets.c
2004-12-30 14:55:39.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/kernel/asm-offsets.c
2005-02-13 12:30:59.0 +0800
@@ -16,6 +16,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -136,6 +137,10 @@ main(void)
DEFINE(TI_CPU, offsetof(struct thread_info, cpu));
DEFINE(TI_PREEMPT, offsetof(struct thread_info, preempt_count));
+ DEFINE(pbe_address, offsetof(struct pbe, address));
+ DEFINE(pbe_orig_address, offsetof(struct pbe, orig_address));
+ DEFINE(pbe_next, offsetof(struct pbe, next));
+
DEFINE(NUM_USER_SEGMENTS, TASK_SIZE>>28);
return 0;
}
--
Hu Gang .-.
/v\
// \\
Linux User /( )\ [204016]
GPG Key ID ^^-^^ http://soulinfo.com/~hugang/hugang.asc
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
On Wed, Feb 09, 2005 at 12:22:52AM +0100, Rafael J. Wysocki wrote:
> On Tuesday, 8 of February 2005 23:42, Pavel Machek wrote:
> > Hi!
> >
> > > +static inline void eat_page(void *page) {
> >
> > Please put { on new line.
>
> Oh, I still tend to forget about this. Corrected in the patch that is
> available on the web
> (http://www.sisk.pl/kernel/patches/2.6.11-rc3-mm1/swsusp-use-list-resume-v4.patch).
>
>
> > Okay, as you can see, I could find very little wrong with this
> > patch. That hopefully means it is okay ;-). I should still check error
> > handling, but I guess I'll do it when it is applied because it is hard
> > to do on a diff. I guess it should go into -mm just after 2.6.11 is
> > released...
>
> That would be great!
>
> Greets,
> Rafael
Here is powerpc port support for that. Thanks for greate patch.
--- 2.6.11-rc3-mm2-use-list-resume-v4/arch/ppc/Kconfig 2005-02-13
12:13:31.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/Kconfig 2005-02-13
12:22:32.0 +0800
@@ -1068,6 +1068,8 @@ config PROC_HARDWARE
source "drivers/zorro/Kconfig"
+source kernel/power/Kconfig
+
endmenu
menu "Bus options"
--- 2.6.11-rc3-mm2-use-list-resume-v4/arch/ppc/kernel/Makefile 2005-02-13
12:13:31.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/kernel/Makefile
2005-02-13 12:22:06.0 +0800
@@ -16,6 +16,7 @@ obj-y := entry.o traps.o irq.o idle.o
semaphore.o syscalls.o setup.o \
cputable.o ppc_htab.o perfmon.o
obj-$(CONFIG_6xx) += l2cr.o cpu_setup_6xx.o
+obj-$(CONFIG_SOFTWARE_SUSPEND) += swsusp.o
obj-$(CONFIG_POWER4) += cpu_setup_power4.o
obj-$(CONFIG_MODULES) += module.o ppc_ksyms.o
obj-$(CONFIG_NOT_COHERENT_CACHE) += dma-mapping.o
--- 2.6.11-rc3-mm2-use-list-resume-v4/arch/ppc/kernel/signal.c 2005-02-13
12:11:50.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/kernel/signal.c
2005-02-13 12:22:06.0 +0800
@@ -28,6 +28,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -704,6 +705,14 @@ int do_signal(sigset_t *oldset, struct p
unsigned long frame, newsp;
int signr, ret;
+ if (current->flags & PF_FREEZE) {
+ refrigerator(PF_FREEZE);
+ signr = 0;
+ ret = regs->gpr[3];
+ if (!signal_pending(current))
+ goto no_signal;
+ }
+
if (!oldset)
oldset = >blocked;
@@ -726,6 +735,7 @@ int do_signal(sigset_t *oldset, struct p
regs->gpr[3] = EINTR;
/* note that the cr0.SO bit is already set */
} else {
+no_signal:
regs->nip -= 4; /* Back up & retry system call */
regs->result = 0;
regs->trap = 0;
--- /dev/null 2004-06-07 18:45:47.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/kernel/swsusp.S
1904-01-01 00:05:22.0 +0706
@@ -0,0 +1,349 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+
+/*
+ * Structure for storing CPU registers on the save area.
+ */
+#define SL_SP 0
+#define SL_PC 4
+#define SL_MSR 8
+#define SL_SDR10xc
+#define SL_SPRG0 0x10/* 4 sprg's */
+#define SL_DBAT0 0x20
+#define SL_IBAT0 0x28
+#define SL_DBAT1 0x30
+#define SL_IBAT1 0x38
+#define SL_DBAT2 0x40
+#define SL_IBAT2 0x48
+#define SL_DBAT3 0x50
+#define SL_IBAT3 0x58
+#define SL_TB 0x60
+#define SL_R2 0x68
+#define SL_CR 0x6c
+#define SL_LR 0x70
+#define SL_R12 0x74/* r12 to r31 */
+#define SL_SIZE(SL_R12 + 80)
+
+ .section .data
+ .align 5
+
+_GLOBAL(swsusp_save_area)
+ .space SL_SIZE
+
+
+ .section .text
+ .align 5
+
+_GLOBAL(swsusp_arch_suspend)
+
+ lis r11,[EMAIL PROTECTED]
+ ori r11,r11,[EMAIL PROTECTED]
+
+ mflrr0
+ stw r0,SL_LR(r11)
+ mfcrr0
+ stw r0,SL_CR(r11)
+ stw r1,SL_SP(r11)
+ stw r2,SL_R2(r11)
+ stmwr12,SL_R12(r11)
+
+ /* Save MSR & SDR1 */
+ mfmsr r4
+ stw r4,SL_MSR(r11)
+ mfsdr1 r4
+ stw r4,SL_SDR1(r11)
+
+ /* Get a stable timebase and save it */
+1: mftbu r4
+ stw r4,SL_TB(r11)
+ mftbr5
+ stw r5,SL_TB+4(r11)
+ mftbu r3
+ cmpwr3,r4
+ bne 1b
+
+ /* Save SPRGs */
+ mfsprg r4,0
+ stw r4,SL_SPRG0(r11)
+ mfsprg r4,1
+ stw r4,SL_SPRG0+4(r11)
+ mfsprg r4,2
+ stw r4,SL_SPRG0+8(r11)
+ mfsprg r4,3
+ stw r4,SL_SPRG0+12(r11)
+
+ /* Save BATs */
+ mfdbatu
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
On Wed, Feb 09, 2005 at 12:22:52AM +0100, Rafael J. Wysocki wrote:
On Tuesday, 8 of February 2005 23:42, Pavel Machek wrote:
Hi!
+static inline void eat_page(void *page) {
Please put { on new line.
Oh, I still tend to forget about this. Corrected in the patch that is
available on the web
(http://www.sisk.pl/kernel/patches/2.6.11-rc3-mm1/swsusp-use-list-resume-v4.patch).
Okay, as you can see, I could find very little wrong with this
patch. That hopefully means it is okay ;-). I should still check error
handling, but I guess I'll do it when it is applied because it is hard
to do on a diff. I guess it should go into -mm just after 2.6.11 is
released...
That would be great!
Greets,
Rafael
Here is powerpc port support for that. Thanks for greate patch.
--- 2.6.11-rc3-mm2-use-list-resume-v4/arch/ppc/Kconfig 2005-02-13
12:13:31.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/Kconfig 2005-02-13
12:22:32.0 +0800
@@ -1068,6 +1068,8 @@ config PROC_HARDWARE
source drivers/zorro/Kconfig
+source kernel/power/Kconfig
+
endmenu
menu Bus options
--- 2.6.11-rc3-mm2-use-list-resume-v4/arch/ppc/kernel/Makefile 2005-02-13
12:13:31.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/kernel/Makefile
2005-02-13 12:22:06.0 +0800
@@ -16,6 +16,7 @@ obj-y := entry.o traps.o irq.o idle.o
semaphore.o syscalls.o setup.o \
cputable.o ppc_htab.o perfmon.o
obj-$(CONFIG_6xx) += l2cr.o cpu_setup_6xx.o
+obj-$(CONFIG_SOFTWARE_SUSPEND) += swsusp.o
obj-$(CONFIG_POWER4) += cpu_setup_power4.o
obj-$(CONFIG_MODULES) += module.o ppc_ksyms.o
obj-$(CONFIG_NOT_COHERENT_CACHE) += dma-mapping.o
--- 2.6.11-rc3-mm2-use-list-resume-v4/arch/ppc/kernel/signal.c 2005-02-13
12:11:50.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/kernel/signal.c
2005-02-13 12:22:06.0 +0800
@@ -28,6 +28,7 @@
#include linux/elf.h
#include linux/tty.h
#include linux/binfmts.h
+#include linux/suspend.h
#include asm/ucontext.h
#include asm/uaccess.h
#include asm/pgtable.h
@@ -704,6 +705,14 @@ int do_signal(sigset_t *oldset, struct p
unsigned long frame, newsp;
int signr, ret;
+ if (current-flags PF_FREEZE) {
+ refrigerator(PF_FREEZE);
+ signr = 0;
+ ret = regs-gpr[3];
+ if (!signal_pending(current))
+ goto no_signal;
+ }
+
if (!oldset)
oldset = current-blocked;
@@ -726,6 +735,7 @@ int do_signal(sigset_t *oldset, struct p
regs-gpr[3] = EINTR;
/* note that the cr0.SO bit is already set */
} else {
+no_signal:
regs-nip -= 4; /* Back up retry system call */
regs-result = 0;
regs-trap = 0;
--- /dev/null 2004-06-07 18:45:47.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/kernel/swsusp.S
1904-01-01 00:05:22.0 +0706
@@ -0,0 +1,349 @@
+#include linux/config.h
+#include linux/threads.h
+#include asm/processor.h
+#include asm/page.h
+#include asm/cputable.h
+#include asm/thread_info.h
+#include asm/ppc_asm.h
+#include asm/offsets.h
+
+
+/*
+ * Structure for storing CPU registers on the save area.
+ */
+#define SL_SP 0
+#define SL_PC 4
+#define SL_MSR 8
+#define SL_SDR10xc
+#define SL_SPRG0 0x10/* 4 sprg's */
+#define SL_DBAT0 0x20
+#define SL_IBAT0 0x28
+#define SL_DBAT1 0x30
+#define SL_IBAT1 0x38
+#define SL_DBAT2 0x40
+#define SL_IBAT2 0x48
+#define SL_DBAT3 0x50
+#define SL_IBAT3 0x58
+#define SL_TB 0x60
+#define SL_R2 0x68
+#define SL_CR 0x6c
+#define SL_LR 0x70
+#define SL_R12 0x74/* r12 to r31 */
+#define SL_SIZE(SL_R12 + 80)
+
+ .section .data
+ .align 5
+
+_GLOBAL(swsusp_save_area)
+ .space SL_SIZE
+
+
+ .section .text
+ .align 5
+
+_GLOBAL(swsusp_arch_suspend)
+
+ lis r11,[EMAIL PROTECTED]
+ ori r11,r11,[EMAIL PROTECTED]
+
+ mflrr0
+ stw r0,SL_LR(r11)
+ mfcrr0
+ stw r0,SL_CR(r11)
+ stw r1,SL_SP(r11)
+ stw r2,SL_R2(r11)
+ stmwr12,SL_R12(r11)
+
+ /* Save MSR SDR1 */
+ mfmsr r4
+ stw r4,SL_MSR(r11)
+ mfsdr1 r4
+ stw r4,SL_SDR1(r11)
+
+ /* Get a stable timebase and save it */
+1: mftbu r4
+ stw r4,SL_TB(r11)
+ mftbr5
+ stw r5,SL_TB+4(r11)
+ mftbu r3
+ cmpwr3,r4
+ bne 1b
+
+ /* Save SPRGs */
+ mfsprg r4,0
+ stw r4,SL_SPRG0(r11)
+ mfsprg r4,1
+ stw
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
On Sun, Feb 13, 2005 at 01:04:56PM +0706, [EMAIL PROTECTED] wrote:
On Wed, Feb 09, 2005 at 12:22:52AM +0100, Rafael J. Wysocki wrote:
On Tuesday, 8 of February 2005 23:42, Pavel Machek wrote:
Hi!
+static inline void eat_page(void *page) {
Please put { on new line.
Oh, I still tend to forget about this. Corrected in the patch that is
available on the web
(http://www.sisk.pl/kernel/patches/2.6.11-rc3-mm1/swsusp-use-list-resume-v4.patch).
Okay, as you can see, I could find very little wrong with this
patch. That hopefully means it is okay ;-). I should still check error
handling, but I guess I'll do it when it is applied because it is hard
to do on a diff. I guess it should go into -mm just after 2.6.11 is
released...
That would be great!
Greets,
Rafael
Here is powerpc port support for that. Thanks for greate patch.
Sorry I forgot this one.
--- 2.6.11-rc3-mm2-use-list-resume-v4/arch/ppc/kernel/asm-offsets.c
2004-12-30 14:55:39.0 +0800
+++ 2.6.11-rc3-mm2-use-list-resume-v4-ppc/arch/ppc/kernel/asm-offsets.c
2005-02-13 12:30:59.0 +0800
@@ -16,6 +16,7 @@
#include linux/string.h
#include linux/types.h
#include linux/ptrace.h
+#include linux/suspend.h
#include linux/mman.h
#include linux/mm.h
#include asm/io.h
@@ -136,6 +137,10 @@ main(void)
DEFINE(TI_CPU, offsetof(struct thread_info, cpu));
DEFINE(TI_PREEMPT, offsetof(struct thread_info, preempt_count));
+ DEFINE(pbe_address, offsetof(struct pbe, address));
+ DEFINE(pbe_orig_address, offsetof(struct pbe, orig_address));
+ DEFINE(pbe_next, offsetof(struct pbe, next));
+
DEFINE(NUM_USER_SEGMENTS, TASK_SIZE28);
return 0;
}
--
Hu Gang .-.
/v\
// \\
Linux User /( )\ [204016]
GPG Key ID ^^-^^ http://soulinfo.com/~hugang/hugang.asc
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
On Tuesday, 8 of February 2005 23:42, Pavel Machek wrote:
> Hi!
>
> > +static inline void eat_page(void *page) {
>
> Please put { on new line.
Oh, I still tend to forget about this. Corrected in the patch that is
available on the web
(http://www.sisk.pl/kernel/patches/2.6.11-rc3-mm1/swsusp-use-list-resume-v4.patch).
> Okay, as you can see, I could find very little wrong with this
> patch. That hopefully means it is okay ;-). I should still check error
> handling, but I guess I'll do it when it is applied because it is hard
> to do on a diff. I guess it should go into -mm just after 2.6.11 is
> released...
That would be great!
Greets,
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"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi!
> +static inline void eat_page(void *page) {
Please put { on new line.
Okay, as you can see, I could find very little wrong with this
patch. That hopefully means it is okay ;-). I should still check error
handling, but I guess I'll do it when it is applied because it is hard
to do on a diff. I guess it should go into -mm just after 2.6.11 is
released...
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
On Tuesday, 8 of February 2005 20:10, Pavel Machek wrote: > Hi! > > > > so it is okay, but... > > > > ... I could have done it more elegantly. You're right, I've now introduced > > a function eat_page() that adds a page to the list of unusable pages and > > used it instead of the free_page() here. > > Thanks. > > > > > + p = pbe; > > > > + pbe += PB_PAGE_SKIP; > > > > + do > > > > + p->next = p + 1; > > > > + while (p++ < pbe); > > > > > > I've already seen this code somewhere around in different > > > variant... Perhaps you want to make it inline function? > > > > I tried to avoid modifying the suspend part, but if it's not a problem, > > why don't we go farther and reuse alloc_pagedir() in the resume code? > > > > It has the advantage that read_pagedir() is then much simpler, and it > > returns an integer. However, for this purpose, it's better to split > > alloc_pagedir() into two functions, one of which allocates memory pages, > > and the second puts the list structure on them. > > I guess that modifying suspend part is okay. We do not want to have > two copies of similar code... > > > > > + if(!(pagedir_nosave = swsusp_pagedir_relocate(p))) > > > > + return -ENOMEM; > > > > > > Same here. > > > > The value is used in error reporting and the only reason why this function > > may fail is the lack of memory (the same applies to alloc_pagedir()). > > > > The revised (not as thoroughly tested as the previous one, but hopefully > > nicer) patch follows. > > I guess I'll wait for "reuse alloc_pagedir" version. It's this one. :-) Greets, 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" - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi! > > so it is okay, but... > > ... I could have done it more elegantly. You're right, I've now introduced > a function eat_page() that adds a page to the list of unusable pages and > used it instead of the free_page() here. Thanks. > > > + p = pbe; > > > + pbe += PB_PAGE_SKIP; > > > + do > > > + p->next = p + 1; > > > + while (p++ < pbe); > > > > I've already seen this code somewhere around in different > > variant... Perhaps you want to make it inline function? > > I tried to avoid modifying the suspend part, but if it's not a problem, > why don't we go farther and reuse alloc_pagedir() in the resume code? > > It has the advantage that read_pagedir() is then much simpler, and it > returns an integer. However, for this purpose, it's better to split > alloc_pagedir() into two functions, one of which allocates memory pages, > and the second puts the list structure on them. I guess that modifying suspend part is okay. We do not want to have two copies of similar code... > > > + if(!(pagedir_nosave = swsusp_pagedir_relocate(p))) > > > + return -ENOMEM; > > > > Same here. > > The value is used in error reporting and the only reason why this function > may fail is the lack of memory (the same applies to alloc_pagedir()). > > The revised (not as thoroughly tested as the previous one, but hopefully > nicer) patch follows. I guess I'll wait for "reuse alloc_pagedir" version. Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi,
On Monday, 7 of February 2005 17:23, Pavel Machek wrote:
> Hi!
>
> > The (updated) patch follows.
>
> Okay, few comments...
>
>
> > Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
> >
> > diff -Nru linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S
> > linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S
> > --- linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S 2004-12-24
> > 22:34:31.0 +0100
> > +++ linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S 2005-02-05
> > 20:57:03.0 +0100
> > @@ -28,28 +28,28 @@
> > ret
> >
> > ENTRY(swsusp_arch_resume)
> > - movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx
> > - movl %ecx,%cr3
> > + movl$swsusp_pg_dir-__PAGE_OFFSET,%ecx
> > + movl%ecx,%cr3
> >
> > - movlpagedir_nosave, %ebx
> > - xorl%eax, %eax
> > - xorl%edx, %edx
> > + movlpagedir_nosave, %edx
>
> move copy_loop: here
OK
> > + testl %edx, %edx
> > + jz done
> > .p2align 4,,7
> >
> > copy_loop:
> > - movl4(%ebx,%edx),%edi
> > - movl(%ebx,%edx),%esi
> > + movl(%edx), %esi
> > + movl4(%edx), %edi
> >
> > movl$1024, %ecx
> > rep
> > movsl
> >
> > - incl%eax
> > - addl$16, %edx
> > - cmplnr_copy_pages,%eax
> > - jb copy_loop
> > + movl12(%edx), %edx
> > + testl %edx, %edx
> > + jnz copy_loop
>
> And do unconditional jump here?
OK (I did the same for x86-64).
> Also, 12(%edx)... Could it be handled using asm-offsets, like on x86-64?
Yes, and so I did.
> > +static void __init free_eaten_memory(void) {
>
> Please put { at new line.
>
> > + for_each_pbe(p, pblist)
> > + p->address = 0UL;
> > +
> > + for_each_pbe(p, pblist) {
> > + p->address = get_usable_page(GFP_ATOMIC);
> > + if(!p->address)
>
> I'd put space between if and (. And probably do the same for
> for_each_pbe... it behaves like a while.
OK
[-- snip --]
> > + for_each_pb_page(pbpage, pblist) {
> > + if (does_collide_order((unsigned long)pbpage, 0)) {
> > + m = (void *)get_usable_page(GFP_ATOMIC | __GFP_COLD);
> > + if (!m) {
> > + error = -ENOMEM;
> > + break;
> > + }
> > + memcpy(m, (void *)pbpage, PAGE_SIZE);
> > + if (pbpage == pblist)
> > + pblist = (struct pbe *)m;
> > + else
> > + tail->next = (struct pbe *)m;
> > +
> > + free_page((unsigned long)pbpage);
>
> Uh, you free it so that you can allocate it again, and again find out
> that it is unusable? It will probably end up on list of unusable
> pages,
That's because I wanted the page to end up on this list.
> so it is okay, but...
... I could have done it more elegantly. You're right, I've now introduced
a function eat_page() that adds a page to the list of unusable pages and
used it instead of the free_page() here.
> > + pbpage = (struct pbe *)m;
> > +
> > + /* We have to link the PBEs again */
> > +
> > + for (p = pbpage ; p < pbpage + PB_PAGE_SKIP ; p++)
>
> I'd avoid " " before ;.
OK
> > + p = pbe;
> > + pbe += PB_PAGE_SKIP;
> > + do
> > + p->next = p + 1;
> > + while (p++ < pbe);
>
> I've already seen this code somewhere around in different
> variant... Perhaps you want to make it inline function?
I tried to avoid modifying the suspend part, but if it's not a problem,
why don't we go farther and reuse alloc_pagedir() in the resume code?
It has the advantage that read_pagedir() is then much simpler, and it
returns an integer. However, for this purpose, it's better to split
alloc_pagedir() into two functions, one of which allocates memory pages,
and the second puts the list structure on them.
> > + p->next = NULL;
> > + pr_debug("swsusp: Read %d pages, allocated %d PBEs\n", i, num);
> > + error = (i != swsusp_info.pagedir_pages); /* a sanity check */
>
> If it is sanity check, do BUG_ON().
OK
> > + if(!(p = read_pagedir()))
> > + return -EFAULT;
>
> Is the value used? By using pointers instead of normal ints, you kill
> possibility of meaningfull error reporting...
Yes, but it is fixed easily if alloc_pagedir() is reused in the resume code.
> > + if(!(pagedir_nosave = swsusp_pagedir_relocate(p)))
> > + return -ENOMEM;
>
> Same here.
The value is used in error reporting and the only reason why this function
may fail is the lack of memory (the same applies to alloc_pagedir()).
The revised (not as thoroughly tested as the previous one, but hopefully
nicer) patch follows.
Greets,
Rafael
Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
diff -Nru linux-2.6.11-rc3-mm1-orig/arch/i386/kernel/asm-offsets.c
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi,
On Monday, 7 of February 2005 17:23, Pavel Machek wrote:
Hi!
The (updated) patch follows.
Okay, few comments...
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
diff -Nru linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S
linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S
--- linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S 2004-12-24
22:34:31.0 +0100
+++ linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S 2005-02-05
20:57:03.0 +0100
@@ -28,28 +28,28 @@
ret
ENTRY(swsusp_arch_resume)
- movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx
- movl %ecx,%cr3
+ movl$swsusp_pg_dir-__PAGE_OFFSET,%ecx
+ movl%ecx,%cr3
- movlpagedir_nosave, %ebx
- xorl%eax, %eax
- xorl%edx, %edx
+ movlpagedir_nosave, %edx
move copy_loop: here
OK
+ testl %edx, %edx
+ jz done
.p2align 4,,7
copy_loop:
- movl4(%ebx,%edx),%edi
- movl(%ebx,%edx),%esi
+ movl(%edx), %esi
+ movl4(%edx), %edi
movl$1024, %ecx
rep
movsl
- incl%eax
- addl$16, %edx
- cmplnr_copy_pages,%eax
- jb copy_loop
+ movl12(%edx), %edx
+ testl %edx, %edx
+ jnz copy_loop
And do unconditional jump here?
OK (I did the same for x86-64).
Also, 12(%edx)... Could it be handled using asm-offsets, like on x86-64?
Yes, and so I did.
+static void __init free_eaten_memory(void) {
Please put { at new line.
+ for_each_pbe(p, pblist)
+ p-address = 0UL;
+
+ for_each_pbe(p, pblist) {
+ p-address = get_usable_page(GFP_ATOMIC);
+ if(!p-address)
I'd put space between if and (. And probably do the same for
for_each_pbe... it behaves like a while.
OK
[-- snip --]
+ for_each_pb_page(pbpage, pblist) {
+ if (does_collide_order((unsigned long)pbpage, 0)) {
+ m = (void *)get_usable_page(GFP_ATOMIC | __GFP_COLD);
+ if (!m) {
+ error = -ENOMEM;
+ break;
+ }
+ memcpy(m, (void *)pbpage, PAGE_SIZE);
+ if (pbpage == pblist)
+ pblist = (struct pbe *)m;
+ else
+ tail-next = (struct pbe *)m;
+
+ free_page((unsigned long)pbpage);
Uh, you free it so that you can allocate it again, and again find out
that it is unusable? It will probably end up on list of unusable
pages,
That's because I wanted the page to end up on this list.
so it is okay, but...
... I could have done it more elegantly. You're right, I've now introduced
a function eat_page() that adds a page to the list of unusable pages and
used it instead of the free_page() here.
+ pbpage = (struct pbe *)m;
+
+ /* We have to link the PBEs again */
+
+ for (p = pbpage ; p pbpage + PB_PAGE_SKIP ; p++)
I'd avoid before ;.
OK
+ p = pbe;
+ pbe += PB_PAGE_SKIP;
+ do
+ p-next = p + 1;
+ while (p++ pbe);
I've already seen this code somewhere around in different
variant... Perhaps you want to make it inline function?
I tried to avoid modifying the suspend part, but if it's not a problem,
why don't we go farther and reuse alloc_pagedir() in the resume code?
It has the advantage that read_pagedir() is then much simpler, and it
returns an integer. However, for this purpose, it's better to split
alloc_pagedir() into two functions, one of which allocates memory pages,
and the second puts the list structure on them.
+ p-next = NULL;
+ pr_debug(swsusp: Read %d pages, allocated %d PBEs\n, i, num);
+ error = (i != swsusp_info.pagedir_pages); /* a sanity check */
If it is sanity check, do BUG_ON().
OK
+ if(!(p = read_pagedir()))
+ return -EFAULT;
Is the value used? By using pointers instead of normal ints, you kill
possibility of meaningfull error reporting...
Yes, but it is fixed easily if alloc_pagedir() is reused in the resume code.
+ if(!(pagedir_nosave = swsusp_pagedir_relocate(p)))
+ return -ENOMEM;
Same here.
The value is used in error reporting and the only reason why this function
may fail is the lack of memory (the same applies to alloc_pagedir()).
The revised (not as thoroughly tested as the previous one, but hopefully
nicer) patch follows.
Greets,
Rafael
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
diff -Nru linux-2.6.11-rc3-mm1-orig/arch/i386/kernel/asm-offsets.c
linux-2.6.11-rc3-mm1/arch/i386/kernel/asm-offsets.c
--- linux-2.6.11-rc3-mm1-orig/arch/i386/kernel/asm-offsets.c2004-12-24
22:34:31.0 +0100
+++ linux-2.6.11-rc3-mm1/arch/i386/kernel/asm-offsets.c 2005-02-08
18:14:27.0 +0100
@@ -7,6 +7,7
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi! so it is okay, but... ... I could have done it more elegantly. You're right, I've now introduced a function eat_page() that adds a page to the list of unusable pages and used it instead of the free_page() here. Thanks. + p = pbe; + pbe += PB_PAGE_SKIP; + do + p-next = p + 1; + while (p++ pbe); I've already seen this code somewhere around in different variant... Perhaps you want to make it inline function? I tried to avoid modifying the suspend part, but if it's not a problem, why don't we go farther and reuse alloc_pagedir() in the resume code? It has the advantage that read_pagedir() is then much simpler, and it returns an integer. However, for this purpose, it's better to split alloc_pagedir() into two functions, one of which allocates memory pages, and the second puts the list structure on them. I guess that modifying suspend part is okay. We do not want to have two copies of similar code... + if(!(pagedir_nosave = swsusp_pagedir_relocate(p))) + return -ENOMEM; Same here. The value is used in error reporting and the only reason why this function may fail is the lack of memory (the same applies to alloc_pagedir()). The revised (not as thoroughly tested as the previous one, but hopefully nicer) patch follows. I guess I'll wait for reuse alloc_pagedir version. Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
On Tuesday, 8 of February 2005 20:10, Pavel Machek wrote: Hi! so it is okay, but... ... I could have done it more elegantly. You're right, I've now introduced a function eat_page() that adds a page to the list of unusable pages and used it instead of the free_page() here. Thanks. + p = pbe; + pbe += PB_PAGE_SKIP; + do + p-next = p + 1; + while (p++ pbe); I've already seen this code somewhere around in different variant... Perhaps you want to make it inline function? I tried to avoid modifying the suspend part, but if it's not a problem, why don't we go farther and reuse alloc_pagedir() in the resume code? It has the advantage that read_pagedir() is then much simpler, and it returns an integer. However, for this purpose, it's better to split alloc_pagedir() into two functions, one of which allocates memory pages, and the second puts the list structure on them. I guess that modifying suspend part is okay. We do not want to have two copies of similar code... + if(!(pagedir_nosave = swsusp_pagedir_relocate(p))) + return -ENOMEM; Same here. The value is used in error reporting and the only reason why this function may fail is the lack of memory (the same applies to alloc_pagedir()). The revised (not as thoroughly tested as the previous one, but hopefully nicer) patch follows. I guess I'll wait for reuse alloc_pagedir version. It's this one. :-) Greets, 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi!
+static inline void eat_page(void *page) {
Please put { on new line.
Okay, as you can see, I could find very little wrong with this
patch. That hopefully means it is okay ;-). I should still check error
handling, but I guess I'll do it when it is applied because it is hard
to do on a diff. I guess it should go into -mm just after 2.6.11 is
released...
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
On Tuesday, 8 of February 2005 23:42, Pavel Machek wrote:
Hi!
+static inline void eat_page(void *page) {
Please put { on new line.
Oh, I still tend to forget about this. Corrected in the patch that is
available on the web
(http://www.sisk.pl/kernel/patches/2.6.11-rc3-mm1/swsusp-use-list-resume-v4.patch).
Okay, as you can see, I could find very little wrong with this
patch. That hopefully means it is okay ;-). I should still check error
handling, but I guess I'll do it when it is applied because it is hard
to do on a diff. I guess it should go into -mm just after 2.6.11 is
released...
That would be great!
Greets,
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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi!
> The (updated) patch follows.
Okay, few comments...
> Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
>
> diff -Nru linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S
> linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S
> --- linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S2004-12-24
> 22:34:31.0 +0100
> +++ linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S 2005-02-05
> 20:57:03.0 +0100
> @@ -28,28 +28,28 @@
> ret
>
> ENTRY(swsusp_arch_resume)
> - movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx
> - movl %ecx,%cr3
> + movl$swsusp_pg_dir-__PAGE_OFFSET,%ecx
> + movl%ecx,%cr3
>
> - movlpagedir_nosave, %ebx
> - xorl%eax, %eax
> - xorl%edx, %edx
> + movlpagedir_nosave, %edx
move copy_loop: here
> + testl %edx, %edx
> + jz done
> .p2align 4,,7
>
> copy_loop:
> - movl4(%ebx,%edx),%edi
> - movl(%ebx,%edx),%esi
> + movl(%edx), %esi
> + movl4(%edx), %edi
>
> movl$1024, %ecx
> rep
> movsl
>
> - incl%eax
> - addl$16, %edx
> - cmplnr_copy_pages,%eax
> - jb copy_loop
> + movl12(%edx), %edx
> + testl %edx, %edx
> + jnz copy_loop
And do unconditional jump here? Also, 12(%edx)... Could it be handled
using asm-offsets, like on x86-64?
> +static void __init free_eaten_memory(void) {
Please put { at new line.
> + for_each_pbe(p, pblist)
> + p->address = 0UL;
> +
> + for_each_pbe(p, pblist) {
> + p->address = get_usable_page(GFP_ATOMIC);
> + if(!p->address)
I'd put space between if and (. And probably do the same for
for_each_pbe... it behaves like a while.
> @@ -966,45 +1018,52 @@
> zone->zone_start_pfn));
> }
>
> - /* Clear orig address */
> + /* Clear orig addresses */
>
> - for(i = 0, p = pagedir_nosave; i < nr_copy_pages; i++, p++) {
> + for_each_pbe(p, pblist)
> ClearPageNosaveFree(virt_to_page(p->orig_address));
> - }
>
> - if (!does_collide_order((unsigned long)old_pagedir, pagedir_order)) {
> - printk("not necessary\n");
> - return check_pagedir();
> - }
> + tail = pblist + PB_PAGE_SKIP;
>
> - while ((m = (void *) __get_free_pages(GFP_ATOMIC, pagedir_order)) !=
> NULL) {
> - if (!does_collide_order((unsigned long)m, pagedir_order))
> - break;
> - eaten_memory = m;
> - printk( "." );
> - *eaten_memory = c;
> - c = eaten_memory;
> - }
> + /* Relocate colliding pages */
>
> - if (!m) {
> - printk("out of memory\n");
> - ret = -ENOMEM;
> - } else {
> - pagedir_nosave =
> - memcpy(m, old_pagedir, PAGE_SIZE << pagedir_order);
> + for_each_pb_page(pbpage, pblist) {
> + if (does_collide_order((unsigned long)pbpage, 0)) {
> + m = (void *)get_usable_page(GFP_ATOMIC | __GFP_COLD);
> + if (!m) {
> + error = -ENOMEM;
> + break;
> + }
> + memcpy(m, (void *)pbpage, PAGE_SIZE);
> + if (pbpage == pblist)
> + pblist = (struct pbe *)m;
> + else
> + tail->next = (struct pbe *)m;
> +
> + free_page((unsigned long)pbpage);
Uh, you free it so that you can allocate it again, and again find out
that it is unusable? It will probably end up on list of unusable
pages, so it is okay, but...
> + pbpage = (struct pbe *)m;
> +
> + /* We have to link the PBEs again */
> +
> + for (p = pbpage ; p < pbpage + PB_PAGE_SKIP ; p++)
I'd avoid " " before ;.
> + p = pbe;
> + pbe += PB_PAGE_SKIP;
> + do
> + p->next = p + 1;
> + while (p++ < pbe);
I've already seen this code somewhere around in different
variant... Perhaps you want to make it inline function?
> + p->next = NULL;
> + pr_debug("swsusp: Read %d pages, allocated %d PBEs\n", i, num);
> + error = (i != swsusp_info.pagedir_pages); /* a sanity check */
If it is sanity check, do BUG_ON().
> + if(!(p = read_pagedir()))
> + return -EFAULT;
Is the value used? By using pointers instead of normal ints, you kill
possibility of meaningfull error reporting...
> + if(!(pagedir_nosave = swsusp_pagedir_relocate(p)))
> + return -ENOMEM;
Same here.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi,
On Monday, 7 of February 2005 15:27, Pavel Machek wrote:
> Hi!
>
> > > The following patch is (yet) an(other) attempt to eliminate the need for
> > > using higher
> > > order memory allocations on resume. It accomplishes this by replacing
> > > the array
> > > of page backup entries with a list, so it is only necessary to allocate
> > > individual
> > > memory pages. This approach makes it possible to avoid relocating many
> > > memory
> > > pages on resume (as a result, much less memory is used) and to simplify
> > > the assembly code that restores the image.
> >
> > I have updated the resume patch to apply to the 2.6.11-rc3-mm1 kernel that
> > contains the suspend part and the x86_64-Speed-up-suspend patch. The patch
> > is only for x86-64 and i386.
> >
> > [Note: without this patch the resume process fails on my box ("out of
> > memory")
> > during every 7th - 8th suspend/resume cycle, on the average.]
Well, this doesn't depend on the previous patch, apparently. ;-)
> Pssst. At this point, solution would be to revert the first part,
> too. 2.6.11 is too near to do anything else.
Oh, I didn't mean changing anything now (eg because of the missing ppc
assembly part). However, the patch is useful for me so I thought I'd post it
in case someone else (using the -mm kernels) needed it.
Greets,
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"
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi!
> > The following patch is (yet) an(other) attempt to eliminate the need for
> > using higher
> > order memory allocations on resume. It accomplishes this by replacing the
> > array
> > of page backup entries with a list, so it is only necessary to allocate
> > individual
> > memory pages. This approach makes it possible to avoid relocating many
> > memory
> > pages on resume (as a result, much less memory is used) and to simplify
> > the assembly code that restores the image.
>
> I have updated the resume patch to apply to the 2.6.11-rc3-mm1 kernel that
> contains the suspend part and the x86_64-Speed-up-suspend patch. The patch
> is only for x86-64 and i386.
>
> [Note: without this patch the resume process fails on my box ("out of memory")
> during every 7th - 8th suspend/resume cycle, on the average.]
Pssst. At this point, solution would be to revert the first part,
too. 2.6.11 is too near to do anything else.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi,
On Monday, 31 of January 2005 00:19, Rafael J. Wysocki wrote:
> Hi,
>
> The following patch is (yet) an(other) attempt to eliminate the need for
> using higher
> order memory allocations on resume. It accomplishes this by replacing the
> array
> of page backup entries with a list, so it is only necessary to allocate
> individual
> memory pages. This approach makes it possible to avoid relocating many memory
> pages on resume (as a result, much less memory is used) and to simplify
> the assembly code that restores the image.
I have updated the resume patch to apply to the 2.6.11-rc3-mm1 kernel that
contains the suspend part and the x86_64-Speed-up-suspend patch. The patch
is only for x86-64 and i386.
[Note: without this patch the resume process fails on my box ("out of memory")
during every 7th - 8th suspend/resume cycle, on the average.]
I'm going to maintain this patch so that it's available for the next -mm
kernels (it will be available under http://www.sisk.pl/kernel/patches/).
The (updated) patch follows.
Greets,
Rafael
Signed-off-by: Rafael J. Wysocki <[EMAIL PROTECTED]>
diff -Nru linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S
linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S
--- linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S 2004-12-24
22:34:31.0 +0100
+++ linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S 2005-02-05
20:57:03.0 +0100
@@ -28,28 +28,28 @@
ret
ENTRY(swsusp_arch_resume)
- movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx
- movl %ecx,%cr3
+ movl$swsusp_pg_dir-__PAGE_OFFSET,%ecx
+ movl%ecx,%cr3
- movlpagedir_nosave, %ebx
- xorl%eax, %eax
- xorl%edx, %edx
+ movlpagedir_nosave, %edx
+ testl %edx, %edx
+ jz done
.p2align 4,,7
copy_loop:
- movl4(%ebx,%edx),%edi
- movl(%ebx,%edx),%esi
+ movl(%edx), %esi
+ movl4(%edx), %edi
movl$1024, %ecx
rep
movsl
- incl%eax
- addl$16, %edx
- cmplnr_copy_pages,%eax
- jb copy_loop
+ movl12(%edx), %edx
+ testl %edx, %edx
+ jnz copy_loop
.p2align 4,,7
+done:
movl saved_context_esp, %esp
movl saved_context_ebp, %ebp
movl saved_context_ebx, %ebx
diff -Nru linux-2.6.11-rc3-mm1-orig/arch/x86_64/kernel/asm-offsets.c
linux-2.6.11-rc3-mm1/arch/x86_64/kernel/asm-offsets.c
--- linux-2.6.11-rc3-mm1-orig/arch/x86_64/kernel/asm-offsets.c 2005-02-05
20:49:04.0 +0100
+++ linux-2.6.11-rc3-mm1/arch/x86_64/kernel/asm-offsets.c 2005-02-05
20:57:03.0 +0100
@@ -62,8 +62,8 @@
offsetof (struct rt_sigframe32, uc.uc_mcontext));
BLANK();
#endif
- DEFINE(SIZEOF_PBE, sizeof(struct pbe));
DEFINE(pbe_address, offsetof(struct pbe, address));
DEFINE(pbe_orig_address, offsetof(struct pbe, orig_address));
+ DEFINE(pbe_next, offsetof(struct pbe, next));
return 0;
}
diff -Nru linux-2.6.11-rc3-mm1-orig/arch/x86_64/kernel/suspend_asm.S
linux-2.6.11-rc3-mm1/arch/x86_64/kernel/suspend_asm.S
--- linux-2.6.11-rc3-mm1-orig/arch/x86_64/kernel/suspend_asm.S 2005-02-05
20:49:04.0 +0100
+++ linux-2.6.11-rc3-mm1/arch/x86_64/kernel/suspend_asm.S 2005-02-05
20:57:03.0 +0100
@@ -54,15 +54,8 @@
movq%rax, %cr4; # turn PGE back on
movqpagedir_nosave(%rip), %rdx
- /* compute the limit */
- movlnr_copy_pages(%rip), %eax
- testl %eax, %eax
+ testq %rdx, %rdx
jz done
- movq%rdx,%r8
- movl$SIZEOF_PBE,%r9d
- mul %r9 # with rax, clobbers rdx
- movq%r8, %rdx
- addq%r8, %rax
loop:
/* get addresses from the pbe and copy the page */
movqpbe_address(%rdx), %rsi
@@ -72,9 +65,9 @@
movsq
/* progress to the next pbe */
- addq$SIZEOF_PBE, %rdx
- cmpq%rax, %rdx
- jb loop
+ movqpbe_next(%rdx), %rdx
+ testq %rdx, %rdx
+ jnz loop
done:
movl$24, %eax
movl%eax, %ds
diff -Nru linux-2.6.11-rc3-mm1-orig/kernel/power/swsusp.c
linux-2.6.11-rc3-mm1/kernel/power/swsusp.c
--- linux-2.6.11-rc3-mm1-orig/kernel/power/swsusp.c 2005-02-05
20:49:33.0 +0100
+++ linux-2.6.11-rc3-mm1/kernel/power/swsusp.c 2005-02-05 20:57:03.0
+0100
@@ -919,44 +919,96 @@
return 0;
}
-/*
- * We check here that pagedir & pages it points to won't collide with pages
- * where we're going to restore from the loaded pages later
+/**
+ * On resume, for storing the PBE list and the image,
+ * we can only use memory pages that do not conflict with the pages
+ * which had been used before suspend.
+ *
+ * We don't know which pages are usable until we allocate them.
+ *
+ * Allocated but unusable (ie eaten) memory pages are linked together
+ * to
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi, On Monday, 31 of January 2005 00:19, Rafael J. Wysocki wrote: Hi, The following patch is (yet) an(other) attempt to eliminate the need for using higher order memory allocations on resume. It accomplishes this by replacing the array of page backup entries with a list, so it is only necessary to allocate individual memory pages. This approach makes it possible to avoid relocating many memory pages on resume (as a result, much less memory is used) and to simplify the assembly code that restores the image. I have updated the resume patch to apply to the 2.6.11-rc3-mm1 kernel that contains the suspend part and the x86_64-Speed-up-suspend patch. The patch is only for x86-64 and i386. [Note: without this patch the resume process fails on my box (out of memory) during every 7th - 8th suspend/resume cycle, on the average.] I'm going to maintain this patch so that it's available for the next -mm kernels (it will be available under http://www.sisk.pl/kernel/patches/). The (updated) patch follows. Greets, Rafael Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED] diff -Nru linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S --- linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S 2004-12-24 22:34:31.0 +0100 +++ linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S 2005-02-05 20:57:03.0 +0100 @@ -28,28 +28,28 @@ ret ENTRY(swsusp_arch_resume) - movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx - movl %ecx,%cr3 + movl$swsusp_pg_dir-__PAGE_OFFSET,%ecx + movl%ecx,%cr3 - movlpagedir_nosave, %ebx - xorl%eax, %eax - xorl%edx, %edx + movlpagedir_nosave, %edx + testl %edx, %edx + jz done .p2align 4,,7 copy_loop: - movl4(%ebx,%edx),%edi - movl(%ebx,%edx),%esi + movl(%edx), %esi + movl4(%edx), %edi movl$1024, %ecx rep movsl - incl%eax - addl$16, %edx - cmplnr_copy_pages,%eax - jb copy_loop + movl12(%edx), %edx + testl %edx, %edx + jnz copy_loop .p2align 4,,7 +done: movl saved_context_esp, %esp movl saved_context_ebp, %ebp movl saved_context_ebx, %ebx diff -Nru linux-2.6.11-rc3-mm1-orig/arch/x86_64/kernel/asm-offsets.c linux-2.6.11-rc3-mm1/arch/x86_64/kernel/asm-offsets.c --- linux-2.6.11-rc3-mm1-orig/arch/x86_64/kernel/asm-offsets.c 2005-02-05 20:49:04.0 +0100 +++ linux-2.6.11-rc3-mm1/arch/x86_64/kernel/asm-offsets.c 2005-02-05 20:57:03.0 +0100 @@ -62,8 +62,8 @@ offsetof (struct rt_sigframe32, uc.uc_mcontext)); BLANK(); #endif - DEFINE(SIZEOF_PBE, sizeof(struct pbe)); DEFINE(pbe_address, offsetof(struct pbe, address)); DEFINE(pbe_orig_address, offsetof(struct pbe, orig_address)); + DEFINE(pbe_next, offsetof(struct pbe, next)); return 0; } diff -Nru linux-2.6.11-rc3-mm1-orig/arch/x86_64/kernel/suspend_asm.S linux-2.6.11-rc3-mm1/arch/x86_64/kernel/suspend_asm.S --- linux-2.6.11-rc3-mm1-orig/arch/x86_64/kernel/suspend_asm.S 2005-02-05 20:49:04.0 +0100 +++ linux-2.6.11-rc3-mm1/arch/x86_64/kernel/suspend_asm.S 2005-02-05 20:57:03.0 +0100 @@ -54,15 +54,8 @@ movq%rax, %cr4; # turn PGE back on movqpagedir_nosave(%rip), %rdx - /* compute the limit */ - movlnr_copy_pages(%rip), %eax - testl %eax, %eax + testq %rdx, %rdx jz done - movq%rdx,%r8 - movl$SIZEOF_PBE,%r9d - mul %r9 # with rax, clobbers rdx - movq%r8, %rdx - addq%r8, %rax loop: /* get addresses from the pbe and copy the page */ movqpbe_address(%rdx), %rsi @@ -72,9 +65,9 @@ movsq /* progress to the next pbe */ - addq$SIZEOF_PBE, %rdx - cmpq%rax, %rdx - jb loop + movqpbe_next(%rdx), %rdx + testq %rdx, %rdx + jnz loop done: movl$24, %eax movl%eax, %ds diff -Nru linux-2.6.11-rc3-mm1-orig/kernel/power/swsusp.c linux-2.6.11-rc3-mm1/kernel/power/swsusp.c --- linux-2.6.11-rc3-mm1-orig/kernel/power/swsusp.c 2005-02-05 20:49:33.0 +0100 +++ linux-2.6.11-rc3-mm1/kernel/power/swsusp.c 2005-02-05 20:57:03.0 +0100 @@ -919,44 +919,96 @@ return 0; } -/* - * We check here that pagedir pages it points to won't collide with pages - * where we're going to restore from the loaded pages later +/** + * On resume, for storing the PBE list and the image, + * we can only use memory pages that do not conflict with the pages + * which had been used before suspend. + * + * We don't know which pages are usable until we allocate them. + * + * Allocated but unusable (ie eaten) memory pages are linked together + * to create a list, so
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi! The following patch is (yet) an(other) attempt to eliminate the need for using higher order memory allocations on resume. It accomplishes this by replacing the array of page backup entries with a list, so it is only necessary to allocate individual memory pages. This approach makes it possible to avoid relocating many memory pages on resume (as a result, much less memory is used) and to simplify the assembly code that restores the image. I have updated the resume patch to apply to the 2.6.11-rc3-mm1 kernel that contains the suspend part and the x86_64-Speed-up-suspend patch. The patch is only for x86-64 and i386. [Note: without this patch the resume process fails on my box (out of memory) during every 7th - 8th suspend/resume cycle, on the average.] Pssst. At this point, solution would be to revert the first part, too. 2.6.11 is too near to do anything else. Pavel -- People were complaining that M$ turns users into beta-testers... ...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl! - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi, On Monday, 7 of February 2005 15:27, Pavel Machek wrote: Hi! The following patch is (yet) an(other) attempt to eliminate the need for using higher order memory allocations on resume. It accomplishes this by replacing the array of page backup entries with a list, so it is only necessary to allocate individual memory pages. This approach makes it possible to avoid relocating many memory pages on resume (as a result, much less memory is used) and to simplify the assembly code that restores the image. I have updated the resume patch to apply to the 2.6.11-rc3-mm1 kernel that contains the suspend part and the x86_64-Speed-up-suspend patch. The patch is only for x86-64 and i386. [Note: without this patch the resume process fails on my box (out of memory) during every 7th - 8th suspend/resume cycle, on the average.] Well, this doesn't depend on the previous patch, apparently. ;-) Pssst. At this point, solution would be to revert the first part, too. 2.6.11 is too near to do anything else. Oh, I didn't mean changing anything now (eg because of the missing ppc assembly part). However, the patch is useful for me so I thought I'd post it in case someone else (using the -mm kernels) needed it. Greets, 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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC][PATCH] swsusp: do not use higher order allocations on resume [update 2]
Hi!
The (updated) patch follows.
Okay, few comments...
Signed-off-by: Rafael J. Wysocki [EMAIL PROTECTED]
diff -Nru linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S
linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S
--- linux-2.6.11-rc3-mm1-orig/arch/i386/power/swsusp.S2004-12-24
22:34:31.0 +0100
+++ linux-2.6.11-rc3-mm1/arch/i386/power/swsusp.S 2005-02-05
20:57:03.0 +0100
@@ -28,28 +28,28 @@
ret
ENTRY(swsusp_arch_resume)
- movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx
- movl %ecx,%cr3
+ movl$swsusp_pg_dir-__PAGE_OFFSET,%ecx
+ movl%ecx,%cr3
- movlpagedir_nosave, %ebx
- xorl%eax, %eax
- xorl%edx, %edx
+ movlpagedir_nosave, %edx
move copy_loop: here
+ testl %edx, %edx
+ jz done
.p2align 4,,7
copy_loop:
- movl4(%ebx,%edx),%edi
- movl(%ebx,%edx),%esi
+ movl(%edx), %esi
+ movl4(%edx), %edi
movl$1024, %ecx
rep
movsl
- incl%eax
- addl$16, %edx
- cmplnr_copy_pages,%eax
- jb copy_loop
+ movl12(%edx), %edx
+ testl %edx, %edx
+ jnz copy_loop
And do unconditional jump here? Also, 12(%edx)... Could it be handled
using asm-offsets, like on x86-64?
+static void __init free_eaten_memory(void) {
Please put { at new line.
+ for_each_pbe(p, pblist)
+ p-address = 0UL;
+
+ for_each_pbe(p, pblist) {
+ p-address = get_usable_page(GFP_ATOMIC);
+ if(!p-address)
I'd put space between if and (. And probably do the same for
for_each_pbe... it behaves like a while.
@@ -966,45 +1018,52 @@
zone-zone_start_pfn));
}
- /* Clear orig address */
+ /* Clear orig addresses */
- for(i = 0, p = pagedir_nosave; i nr_copy_pages; i++, p++) {
+ for_each_pbe(p, pblist)
ClearPageNosaveFree(virt_to_page(p-orig_address));
- }
- if (!does_collide_order((unsigned long)old_pagedir, pagedir_order)) {
- printk(not necessary\n);
- return check_pagedir();
- }
+ tail = pblist + PB_PAGE_SKIP;
- while ((m = (void *) __get_free_pages(GFP_ATOMIC, pagedir_order)) !=
NULL) {
- if (!does_collide_order((unsigned long)m, pagedir_order))
- break;
- eaten_memory = m;
- printk( . );
- *eaten_memory = c;
- c = eaten_memory;
- }
+ /* Relocate colliding pages */
- if (!m) {
- printk(out of memory\n);
- ret = -ENOMEM;
- } else {
- pagedir_nosave =
- memcpy(m, old_pagedir, PAGE_SIZE pagedir_order);
+ for_each_pb_page(pbpage, pblist) {
+ if (does_collide_order((unsigned long)pbpage, 0)) {
+ m = (void *)get_usable_page(GFP_ATOMIC | __GFP_COLD);
+ if (!m) {
+ error = -ENOMEM;
+ break;
+ }
+ memcpy(m, (void *)pbpage, PAGE_SIZE);
+ if (pbpage == pblist)
+ pblist = (struct pbe *)m;
+ else
+ tail-next = (struct pbe *)m;
+
+ free_page((unsigned long)pbpage);
Uh, you free it so that you can allocate it again, and again find out
that it is unusable? It will probably end up on list of unusable
pages, so it is okay, but...
+ pbpage = (struct pbe *)m;
+
+ /* We have to link the PBEs again */
+
+ for (p = pbpage ; p pbpage + PB_PAGE_SKIP ; p++)
I'd avoid before ;.
+ p = pbe;
+ pbe += PB_PAGE_SKIP;
+ do
+ p-next = p + 1;
+ while (p++ pbe);
I've already seen this code somewhere around in different
variant... Perhaps you want to make it inline function?
+ p-next = NULL;
+ pr_debug(swsusp: Read %d pages, allocated %d PBEs\n, i, num);
+ error = (i != swsusp_info.pagedir_pages); /* a sanity check */
If it is sanity check, do BUG_ON().
+ if(!(p = read_pagedir()))
+ return -EFAULT;
Is the value used? By using pointers instead of normal ints, you kill
possibility of meaningfull error reporting...
+ if(!(pagedir_nosave = swsusp_pagedir_relocate(p)))
+ return -ENOMEM;
Same here.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL
