[PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[Arnaldo Carvalho de Melo]

i686 -Iarch/i386/mach-generic -nostdinc -iwithprefix include    -DKBUILD_BASENAME=suspend -DKBUILD_MODNAME=suspend -DEXPORT_SYMTAB  -c -o kernel/suspend.o kernel/suspend.c
kernel/suspend.c:295: warning: #warning This might be broken. We need to somehow wait for data to reach the disk
kernel/suspend.c: In function `free_some_memory':
kernel/suspend.c:627: `contig_page_data' undeclared (first use in this function)
kernel/suspend.c:627: (Each undeclared identifier is reported only once
kernel/suspend.c:627: for each function it appears in.)
make[1]: ** [kernel/suspend.o] Erro 1
make: ** [kernel] Erro 2
[acme@oops hell_header-2.5]$ grep CONFIG_DISCONTIGMEM .config
CONFIG_DISCONTIGMEM=y

and in ./mm/page_alloc.c

#ifndef CONFIG_DISCONTIGMEM
static bootmem_data_t contig_bootmem_data;
struct pglist_data contig_page_data = { .bdata = &contig_bootmem_data };

void __init free_area_init(unsigned long *zones_size)
{
        free_area_init_node(0, &contig_page_data, NULL, zones_size, 0, NULL);
        mem_map = contig_page_data.node_mem_map;
}
#endif

So perhaps the following patch is in order? Its kind of brute force, disabling it
altogether, but it at least fixes it for now.

You can import this changeset into BK by piping this whole message to:
'| bk receive [path to repository]' or apply the patch as usual.

===================================================================


ChangeSet@1.851, 2002-11-15 06:07:56-02:00, acme@conectiva.com.br
  o swsusp: depends on CONFIG_DISCONTIGMEM=n


 Kconfig |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)


diff -Nru a/arch/i386/Kconfig b/arch/i386/Kconfig
--- a/arch/i386/Kconfig	Fri Nov 15 06:09:18 2002
+++ b/arch/i386/Kconfig	Fri Nov 15 06:09:18 2002
@@ -1518,7 +1518,7 @@
 
 config SOFTWARE_SUSPEND
 	bool "Software Suspend (EXPERIMENTAL)"
-	depends on EXPERIMENTAL && PM
+	depends on EXPERIMENTAL && PM && !DISCONTIGMEM
 	---help---
 	  Enable the possibilty of suspendig machine. It doesn't need APM.
 	  You may suspend your machine by 'swsusp' or 'shutdown -z <time>' 

===================================================================


This BitKeeper patch contains the following changesets:
1.851
## Wrapped with gzip_uu ##


begin 664 bkpatch30899
M'XL(`*ZKU#T``\V4T6K;,!2&KZ.GT"CT9M@^1[84Q>"1-LG:D*8-:0N#,8:J
M:'%H;`7+2=?AAY^=;$VW;BL;&TP2DM`11[\^_>B`7CM3Q"VE,T,.Z*EU9=S2
M-C>Z7&R4KVWFWQ1U8&IM'0A2FYG@>!2X.[=V*Y///.9S4L<GJM0IW9C"Q2WT
MPX>5\GYEXM9T<')]=C0E)$EH+U7YW%R:DB8)*6VQ4<N9ZZHR7=K<+PN5N\R4
MVY.KAZT5`V!UY=@.@8L*!43M2N,,445H9L`B*:)]MD;F+W,A(@>!(?(*H(,1
MZ5/T)4<*+$`,D%,0,;1C+CQ@,0!M^'2_YT)?(O6`'-._>XT>T=32'>*8SDS#
MV5&;T][%^>OAR?O^\+*>70U/QH-QDI,1%9P))),]6N+]9B$$%)!7]--BM3++
M[G*1KS]ZF9"WOBWF;[]>[UVE"IT&BU"*8%3#^+"8[VA""!"%#&7%(B%E)9"%
M*$'==)"W!>@?X_M9MN9M)(B052`Z`%O7/-G:N.<?J27%VI7WW:;7MEAMU:KU
M\WF;TJZ51Q5PR>3.4]\Z2L0A>]91^!\XJN%^0;WB;MMJ@TR>/L$?N*R/G"%%
M,OPRMAYI&;R9#*;#\>#\ZNB,'A[2R;CI7SS6MO]I=&KTK5MG"<=(1EPQ\AD,
'L(Q2R@0`````
`
end

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[William Lee Irwin III]

On Fri, Nov 15, 2002 at 06:10:44AM -0200, Arnaldo Carvalho de Melo wrote:
> So perhaps the following patch is in order? Its kind of brute force,
> disabling it altogether, but it at least fixes it for now.
> You can import this changeset into BK by piping this whole message to:
> '| bk receive [path to repository]' or apply the patch as usual.
> ===================================================================
> ChangeSet@1.851, 2002-11-15 06:07:56-02:00, acme@conectiva.com.br
>   o swsusp: depends on CONFIG_DISCONTIGMEM=n

The following dropped hunk from Pavel should repair it:


--- clean/kernel/suspend.c	2002-11-01 00:37:42.000000000 +0100
+++ linux-swsusp/kernel/suspend.c	2002-11-08 11:37:06.000000000 +0100
@@ -623,7 +623,7 @@
 static void free_some_memory(void)
 {
 	printk("Freeing memory: ");
-	while (try_to_free_pages(&contig_page_data.node_zones[ZONE_HIGHMEM], GFP_KSWAPD, 0))
+	while (shrink_all_memory(10000))
 		printk(".");
 	printk("|\n");
 }

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[William Lee Irwin III]

On Fri, Nov 15, 2002 at 12:49:15AM -0800, William Lee Irwin III wrote:
> The following dropped hunk from Pavel should repair it:

[cc: list trimmed to spare the uninterested]

Hmm, there are some oddities here in count_and_copy_data_pages(). It
looks like the CONFIG_HIGHMEM panic() is there because copy_page() is
done without kmapping, and the CONFIG_DISCONTIGMEM panic() is there
because the pgdat list etc. are not walked according to VM conventions.

So the traversal looks like it should go something like:

	for_each_zone(zone) {
		for (k = 0; k < zone->present_pages; ++k) {
			struct page *page = &zone->zone_mem_map[k];

			if (!PageReserved) {
				if (PageNosave(page))
					continue;

				chunk_size = is_head_of_free_region(page);

				/* c.f. k++ above */
				if (chunk_size) {
					k += chunk_size - 1;
					continue;
				}
			} else if (PageReserved(page)) {
				BUG_ON(PageNosave(page));

				if (page_to_pfn(page) >= nosave_begin_pfn
					&& page_to_pfn(page) < nosave_end_pfn)
					continue;
			}

			nr_copy_pages++;

			/*
			 * The general usage of page backup entries
			 * is unclear; this is probably incorrect in
			 * some cases, and needs some idea of the size
			 * and layout of the page backup entry array(s)
			 * if they cannot be contiguously allocated or
			 * simultaneously mapped by kernel pagetables.
			 */
			if (pagedir_p) {
				char *src, *dst;
				src = kmap_atomic(page, KM_SWSUSP0);
				dst = kmap_atomic(pagedir_p->page, KM_SWSUSP1);
				copy_page(dst, src);
				kunmap_atomic(dst, KM_SWSUSP0);
				kunmap_atomic(src, KM_SWSUSP1);
				++pagedir_p;
			}
		}
		return nr_copy_pages;
	}

I don't know what to make of highmem on laptops etc., but the VM's
conventions should not be that hard to follow; also, there are uses for
the swsusp functionality on other kinds of machines (e.g. checkpointing).
Pure computationally-oriented systems such as would make use of this
are somewhat different from my primary userbase to support, but I think
it would be valuable to generalize swsusp in this way, and so provide
rudimentary support for such users in addition to some small measure of
cleanup (i.e. the cleanup adds functionality).

Pavel, what do you think?


Bill

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[Pavel Machek]

Hi!

> > The following dropped hunk from Pavel should repair it:
> 
> [cc: list trimmed to spare the uninterested]
> 
> Hmm, there are some oddities here in count_and_copy_data_pages(). It
> looks like the CONFIG_HIGHMEM panic() is there because copy_page() is
> done without kmapping, and the CONFIG_DISCONTIGMEM panic() is there
> because the pgdat list etc. are not walked according to VM
> conventions.

How much memory is needed for HIGHMEM to be neccessary? Is it 1GB? If
so, I can well imagine 1GB laptop....

> So the traversal looks like it should go something like:
> 
> 	for_each_zone(zone) {
> 		for (k = 0; k < zone->present_pages; ++k) {
> 			struct page *page = &zone->zone_mem_map[k];
> 
> 			if (!PageReserved) {
> 				if (PageNosave(page))
> 					continue;
> 
> 				chunk_size = is_head_of_free_region(page);
> 
> 				/* c.f. k++ above */
> 				if (chunk_size) {
> 					k += chunk_size - 1;
> 					continue;
> 				}
> 			} else if (PageReserved(page)) {
> 				BUG_ON(PageNosave(page));
> 
> 				if (page_to_pfn(page) >= nosave_begin_pfn
> 					&& page_to_pfn(page) < nosave_end_pfn)
> 					continue;
> 			}
> 
> 			nr_copy_pages++;
> 
> 			/*
> 			 * The general usage of page backup entries
> 			 * is unclear; this is probably incorrect in
> 			 * some cases, and needs some idea of the size
> 			 * and layout of the page backup entry array(s)
> 			 * if they cannot be contiguously allocated or
> 			 * simultaneously mapped by kernel pagetables.
> 			 */
> 			if (pagedir_p) {
> 				char *src, *dst;
> 				src = kmap_atomic(page, KM_SWSUSP0);
> 				dst = kmap_atomic(pagedir_p->page, KM_SWSUSP1);
> 				copy_page(dst, src);
> 				kunmap_atomic(dst, KM_SWSUSP0);
> 				kunmap_atomic(src, KM_SWSUSP1);
> 				++pagedir_p;

This certainly does not work. We'd need to do some deep magic in
suspend_asm.S to copy pages back. [Well, deep magic... Same
kmap_atomic.] But suspend_asm.S has to guarantee not touching any
memory so the change is not quite trivial.

> I don't know what to make of highmem on laptops etc., but the VM's
> conventions should not be that hard to follow; also, there are uses for
> the swsusp functionality on other kinds of machines (e.g. checkpointing).
> Pure computationally-oriented systems such as would make use of this
> are somewhat different from my primary userbase to support, but I think
> it would be valuable to generalize swsusp in this way, and so provide
> rudimentary support for such users in addition to some small measure of
> cleanup (i.e. the cleanup adds functionality).
> 
> Pavel, what do you think?

I definitely want to support swsusp for server boxes, but I'm not 100%
sure how to do that easily.
								Pavel
-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[William Lee Irwin III]

[code snippet snipped]

On Fri, Nov 15, 2002 at 01:02:33PM +0100, Pavel Machek wrote:
> This certainly does not work. We'd need to do some deep magic in
> suspend_asm.S to copy pages back. [Well, deep magic... Same
> kmap_atomic.] But suspend_asm.S has to guarantee not touching any
> memory so the change is not quite trivial.

I wasn't entirely expecting it to work out of the box. I was really
trying to get the VM structure traversal and access in line with
the general case, and then worry (later) about deeper issues like
that once they arise. kmap_atomic() etc. should be no-ops without
highmem, so otherwise it'd be safe (at least on the save path), aside
from perhaps suggesting that highmem should work when it wouldn't.
The restore path, OTOH...

I should take a look at suspend_asm.S to get an idea of what you're
talking about with respect to its expectations being violated by
the code snippet I posted.


At some point in the past, I wrote:
>> I don't know what to make of highmem on laptops etc., but the VM's
>> conventions should not be that hard to follow; also, there are uses for
>> the swsusp functionality on other kinds of machines (e.g. checkpointing).
>> Pure computationally-oriented systems such as would make use of this
>> are somewhat different from my primary userbase to support, but I think
>> it would be valuable to generalize swsusp in this way, and so provide
>> rudimentary support for such users in addition to some small measure of
>> cleanup (i.e. the cleanup adds functionality).
>> Pavel, what do you think?

On Fri, Nov 15, 2002 at 01:02:33PM +0100, Pavel Machek wrote:
> I definitely want to support swsusp for server boxes, but I'm not 100%
> sure how to do that easily.
> 								Pavel

I'm not entirely sure either. Mostly I suspect that the deep arch
issues will be the tough ones, but things like this I can handle. =)


Thanks,
Bill

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[Pavel Machek]

Hi!

> >> I don't know what to make of highmem on laptops etc., but the VM's
> >> conventions should not be that hard to follow; also, there are uses for
> >> the swsusp functionality on other kinds of machines (e.g. checkpointing).
> >> Pure computationally-oriented systems such as would make use of this
> >> are somewhat different from my primary userbase to support, but I think
> >> it would be valuable to generalize swsusp in this way, and so provide
> >> rudimentary support for such users in addition to some small measure of
> >> cleanup (i.e. the cleanup adds functionality).
> >> Pavel, what do you think?
> 
> On Fri, Nov 15, 2002 at 01:02:33PM +0100, Pavel Machek wrote:
> > I definitely want to support swsusp for server boxes, but I'm not 100%
> > sure how to do that easily.
> 
> I'm not entirely sure either. Mostly I suspect that the deep arch
> issues will be the tough ones, but things like this I can handle. =)

Well, I'd really hate to do 64GB support for swsusp for i386. It would
mean wider pointers in on-disk format and probably would not be
exactly nice.

								Pavel

-- 
Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[William Lee Irwin III]

At some point in the past, I wrote:
>> I'm not entirely sure either. Mostly I suspect that the deep arch
>> issues will be the tough ones, but things like this I can handle. =)

On Fri, Nov 15, 2002 at 07:09:25PM +0100, Pavel Machek wrote:
> Well, I'd really hate to do 64GB support for swsusp for i386. It would
> mean wider pointers in on-disk format and probably would not be
> exactly nice.
> 								Pavel

i386 is actually a poor cpu for numerical workloads due to the
design of its FPU, so the interest will likely be low there, but...

It looks like the struct pbe is storing a physical address in an
unsigned long; using page frame numbers instead of raw physical
addresses should at least catch 36-bit i386 and a substantial fraction
of PPC highmem (40-bit, so not all) with very little effort. A good
chunk of the VM's arch support address calculation API (if not all)
has been converted to pfn-based calculations already, so we're already
in very good shape, aside from a one-shot format change.


Bill

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[Benjamin Herrenschmidt]

On Fri, 2002-11-15 at 13:02, Pavel Machek wrote:
> Hi!
> 
> > > The following dropped hunk from Pavel should repair it:
> > 
> > [cc: list trimmed to spare the uninterested]
> > 
> > Hmm, there are some oddities here in count_and_copy_data_pages(). It
> > looks like the CONFIG_HIGHMEM panic() is there because copy_page() is
> > done without kmapping, and the CONFIG_DISCONTIGMEM panic() is there
> > because the pgdat list etc. are not walked according to VM
> > conventions.
> 
> How much memory is needed for HIGHMEM to be neccessary? Is it 1GB? If
> so, I can well imagine 1GB laptop....

Depends on the arch & other matters. 768Mb on PPC at least, and
it starting to be common within laptops as well.

> This certainly does not work. We'd need to do some deep magic in
> suspend_asm.S to copy pages back. [Well, deep magic... Same
> kmap_atomic.] But suspend_asm.S has to guarantee not touching any
> memory so the change is not quite trivial.

At worst, that could be an arch provided routine. On most PPC32's
I can then just disable data translation on the MMU and access
all pages without kmap'ing them. But that's not terribly portable
and each arch would need different kind of hacking.

Ben.

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[Pavel Machek]

Hi!

> > > > The following dropped hunk from Pavel should repair it:
> > > 
> > > [cc: list trimmed to spare the uninterested]
> > > 
> > > Hmm, there are some oddities here in count_and_copy_data_pages(). It
> > > looks like the CONFIG_HIGHMEM panic() is there because copy_page() is
> > > done without kmapping, and the CONFIG_DISCONTIGMEM panic() is there
> > > because the pgdat list etc. are not walked according to VM
> > > conventions.
> > 
> > How much memory is needed for HIGHMEM to be neccessary? Is it 1GB? If
> > so, I can well imagine 1GB laptop....
> 
> Depends on the arch & other matters. 768Mb on PPC at least, and
> it starting to be common within laptops as well.

I just hope we'll never ever see 64GB i386 laptop...

... We'll probably have to write simpler equivalent of kmap_atomic for
use in suspend_asm.S. It is not really *so* deep magic.

> > This certainly does not work. We'd need to do some deep magic in
> > suspend_asm.S to copy pages back. [Well, deep magic... Same
> > kmap_atomic.] But suspend_asm.S has to guarantee not touching any
> > memory so the change is not quite trivial.
> 
> At worst, that could be an arch provided routine. On most PPC32's
> I can then just disable data translation on the MMU and access
> all pages without kmap'ing them. But that's not terribly portable
> and each arch would need different kind of hacking.

It is arch-specific already. What's worse, resume has to be written in
assembly, so you can't simply write kmap_atomic() there. See
arch/i386/kernel/suspend_asm.S.

								Pavel
-- 
Worst form of spam? Adding advertisment signatures ala sourceforge.net.
What goes next? Inserting advertisment *into* email?

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[William Lee Irwin III]

On Fri, Nov 15, 2002 at 07:12:47PM +0100, Pavel Machek wrote:
> I just hope we'll never ever see 64GB i386 laptop...
> ... We'll probably have to write simpler equivalent of kmap_atomic for
> use in suspend_asm.S. It is not really *so* deep magic.

64GB raises deadly scalability (read as: box won't boot) issues with
boot-time memory reservations and other things. It's an open question
as to whether it will ever be made to work with mainline Linux, but one
I'd like to see answered "yes", if only because it is in some senses
the ultimate test of leanness: "If you are bloated, you will die."

But like I said, it's very unlikely any strong interest will ever
arise specifically in large-scale i386 checkpointing. Computational
workloads are very attached to having clean and efficient FPU's, which
i386 lacks. RISC etc. boxen with clean FPU's are more important for
that. OTOH if highmem works, why wouldn't bigger highmem boxen work? NFI

The general framework is all I have a direct interest in accommodating.
Aside from that I don't see a real need to make the low-level guts of
large-scale highmem systems support swsusp, though 1GB laptops make sense.

Thanks,
Bill

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[Alan Cox]

On Fri, 2002-11-15 at 18:56, William Lee Irwin III wrote:
> But like I said, it's very unlikely any strong interest will ever
> arise specifically in large-scale i386 checkpointing. Computational
> workloads are very attached to having clean and efficient FPU's, which
> i386 lacks. RISC etc. boxen with clean FPU's are more important for
> that. OTOH if highmem works, why wouldn't bigger highmem boxen work? NFI

Most large scale FP computation jobs are done on x86. RISC stuff isnt
that much faster if at all. It may be elegant but the PIV and Athlon go
at 2.5Ghz so make up for elegance by being stupid very fast (note that
by the PIV and athlon the FPU's are not actually very veryy smart)

Alan

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[William Lee Irwin III]

On Sat, Nov 16, 2002 at 02:41:24AM +0000, Alan Cox wrote:
> Most large scale FP computation jobs are done on x86. RISC stuff isnt
> that much faster if at all. It may be elegant but the PIV and Athlon go
> at 2.5Ghz so make up for elegance by being stupid very fast (note that
> by the PIV and athlon the FPU's are not actually very veryy smart)

The clock speeds of i386 may very well now be up to the point where its
clock speeds dominate all other considerations. But I've never seen
large-scale i386 machines used in what numerics I've been involved in,
which, granted, was not much and several years ago (SGI dominated).

I'm still somewhat skeptical given the cpu counts, process address
space, and I/O architecture limitations on i386, but I don't honestly
care. Numerics is not my game anymore if it ever was, and whatever
additional functionality might come out of cleanups is only ammunition
to push the cleanups to me.


Bill

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[Oliver Xymoron]

On Sat, Nov 16, 2002 at 02:41:24AM +0000, Alan Cox wrote:
> On Fri, 2002-11-15 at 18:56, William Lee Irwin III wrote:
> > But like I said, it's very unlikely any strong interest will ever
> > arise specifically in large-scale i386 checkpointing. Computational
> > workloads are very attached to having clean and efficient FPU's, which
> > i386 lacks. RISC etc. boxen with clean FPU's are more important for
> > that. OTOH if highmem works, why wouldn't bigger highmem boxen work? NFI
> 
> Most large scale FP computation jobs are done on x86. RISC stuff isnt
> that much faster if at all. It may be elegant but the PIV and Athlon go
> at 2.5Ghz so make up for elegance by being stupid very fast (note that
> by the PIV and athlon the FPU's are not actually very veryy smart)

Their built-in trancendental ops still beat the pants off RISC clock
for clock, so even at the same clock rate, some number crunching stuff
still wins big on x86. Throw in the cost/FLOPS difference and its
little wonder that very few big machines are being built with anything
but.

-- 
 "Love the dolphins," she advised him. "Write by W.A.S.T.E.." 

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[Pavel Machek]

Hi!

> i686 -Iarch/i386/mach-generic -nostdinc -iwithprefix include    -DKBUILD_BASENAME=suspend -DKBUILD_MODNAME=suspend -DEXPORT_SYMTAB  -c -o kernel/suspend.o kernel/suspend.c
> kernel/suspend.c:295: warning: #warning This might be broken. We need to somehow wait for data to reach the disk
> kernel/suspend.c: In function `free_some_memory':
> kernel/suspend.c:627: `contig_page_data' undeclared (first use in this function)
> kernel/suspend.c:627: (Each undeclared identifier is reported only once
> kernel/suspend.c:627: for each function it appears in.)
> make[1]: ** [kernel/suspend.o] Erro 1
> make: ** [kernel] Erro 2
> [acme@oops hell_header-2.5]$ grep CONFIG_DISCONTIGMEM .config
> CONFIG_DISCONTIGMEM=y
> 
> and in ./mm/page_alloc.c
> 
> #ifndef CONFIG_DISCONTIGMEM
> static bootmem_data_t contig_bootmem_data;
> struct pglist_data contig_page_data = { .bdata = &contig_bootmem_data };
> 
> void __init free_area_init(unsigned long *zones_size)
> {
>         free_area_init_node(0, &contig_page_data, NULL, zones_size, 0, NULL);
>         mem_map = contig_page_data.node_mem_map;
> }
> #endif
> 
> So perhaps the following patch is in order? Its kind of brute force, disabling it
> altogether, but it at least fixes it for now.

Please don't, better patch is pending to fix that.
									Pavel

-- 
Casualities in World Trade Center: ~3k dead inside the building,
cryptography in U.S.A. and free speech in Czech Republic.

Re: [PATCH] swsuspend and CONFIG_DISCONTIGMEM=y

[Arnaldo Carvalho de Melo]

Em Fri, Nov 15, 2002 at 12:54:02PM +0100, Pavel Machek escreveu:
> > So perhaps the following patch is in order? Its kind of brute force, disabling it
> > altogether, but it at least fixes it for now.
> 
> Please don't, better patch is pending to fix that.

wli told me, going to use shrink_all_memory(), etc

- Arnaldo