[PATCH] make swsusp produce nicer screen output

[PATCH] make swsusp produce nicer screen output

[Erik Rigtorp]

[-- Attachment #1: Type: text/plain, Size: 120 bytes --]

Hi!

I made a small patch that makes swsusp produce a bit nicer screen output,
it's still a little rough though.

/Erik

[-- Attachment #2: swsusp-nicer-output.patch --]
[-- Type: text/plain, Size: 1963 bytes --]

diff -Nru linux-2.6.8.1-mm2/kernel/power/disk.c linux-2.6.8.1-mm2-erkki/kernel/power/disk.c
--- linux-2.6.8.1-mm2/kernel/power/disk.c	2004-08-20 17:10:58.000000000 +0200
+++ linux-2.6.8.1-mm2-erkki/kernel/power/disk.c	2004-08-20 15:47:00.000000000 +0200
@@ -85,10 +85,17 @@
 
 static void free_some_memory(void)
 {
-	printk("Freeing memory: ");
-	while (shrink_all_memory(10000))
-		printk(".");
-	printk("|\n");
+	int i = 0;
+	char *p = "-\\|/";
+	
+	printk("Freeing memory:  ");
+	while (shrink_all_memory(10000)) {
+		printk("\b%c", p[i]);
+		i++;
+		if (i > 3)
+			i = 0;
+	}
+	printk("\bdone\n");
 }
 
 
diff -Nru linux-2.6.8.1-mm2/kernel/power/swsusp.c linux-2.6.8.1-mm2-erkki/kernel/power/swsusp.c
--- linux-2.6.8.1-mm2/kernel/power/swsusp.c	2004-08-20 17:10:58.000000000 +0200
+++ linux-2.6.8.1-mm2-erkki/kernel/power/swsusp.c	2004-08-20 16:13:29.000000000 +0200
@@ -296,15 +296,16 @@
 {
 	int error = 0;
 	int i;
-
-	printk( "Writing data to swap (%d pages): ", nr_copy_pages );
+	int mod = nr_copy_pages / 100;
+	
+	printk( "Writing data to swap (%d pages):     ", nr_copy_pages );
 	for (i = 0; i < nr_copy_pages && !error; i++) {
-		if (!(i%100))
-			printk( "." );
+		if (!(i%mod))
+			printk( "\b\b\b\b%3d%%", i / mod );
 		error = write_page((pagedir_nosave+i)->address,
 					  &((pagedir_nosave+i)->swap_address));
 	}
-	printk(" %d Pages done.\n",i);
+	printk("\b\b\b\bdone\n");
 	return error;
 }
 
@@ -1150,14 +1151,15 @@
 	struct pbe * p;
 	int error;
 	int i;
-
+	int mod = nr_copy_pages / 100;
+	
 	if ((error = swsusp_pagedir_relocate()))
 		return error;
 
-	printk( "Reading image data (%d pages): ", nr_copy_pages );
+	printk( "Reading image data (%d pages):     ", nr_copy_pages );
 	for(i = 0, p = pagedir_nosave; i < nr_copy_pages && !error; i++, p++) {
-		if (!(i%100))
-			printk( "." );
+		if (!(i%mod))
+			printk( "\b\b\b\b%3d%%", i / mod );
 		error = bio_read_page(swp_offset(p->swap_address),
 				  (void *)p->address);
 	}

Re: [PATCH] make swsusp produce nicer screen output

[Stefan Seyfried]

Hi,

Erik Rigtorp wrote:
> Hi!
> 
> I made a small patch that makes swsusp produce a bit nicer screen output,
> it's still a little rough though.

generally not a bad idea, but....

> diff -Nru linux-2.6.8.1-mm2/kernel/power/swsusp.c linux-2.6.8.1-mm2-erkki/kernel/power/swsusp.c
> --- linux-2.6.8.1-mm2/kernel/power/swsusp.c	2004-08-20 17:10:58.000000000 +0200
> +++ linux-2.6.8.1-mm2-erkki/kernel/power/swsusp.c	2004-08-20 16:13:29.000000000 +0200
> @@ -296,15 +296,16 @@
>  {
>  	int error = 0;
>  	int i;
> -
> -	printk( "Writing data to swap (%d pages): ", nr_copy_pages );
> +	int mod = nr_copy_pages / 100;
> +	
> +	printk( "Writing data to swap (%d pages):     ", nr_copy_pages );
>  	for (i = 0; i < nr_copy_pages && !error; i++) {
> -		if (!(i%100))
> -			printk( "." );
> +		if (!(i%mod))
> +			printk( "\b\b\b\b%3d%%", i / mod );

what will happen here if nr_copy_pages < 100?

>  		error = write_page((pagedir_nosave+i)->address,
>  					  &((pagedir_nosave+i)->swap_address));
>  	}
> -	printk(" %d Pages done.\n",i);
> +	printk("\b\b\b\bdone\n");
>  	return error;
>  }
>  
> @@ -1150,14 +1151,15 @@
>  	struct pbe * p;
>  	int error;
>  	int i;
> -
> +	int mod = nr_copy_pages / 100;
> +	
>  	if ((error = swsusp_pagedir_relocate()))
>  		return error;
>  
> -	printk( "Reading image data (%d pages): ", nr_copy_pages );
> +	printk( "Reading image data (%d pages):     ", nr_copy_pages );
>  	for(i = 0, p = pagedir_nosave; i < nr_copy_pages && !error; i++, p++) {
> -		if (!(i%100))
> -			printk( "." );
> +		if (!(i%mod))
> +			printk( "\b\b\b\b%3d%%", i / mod );

...and here...

>  		error = bio_read_page(swp_offset(p->swap_address),
>  				  (void *)p->address);
>  	}

  Stefan

Re: [PATCH] make swsusp produce nicer screen output

[Pavel Machek]

Hi!

> I made a small patch that makes swsusp produce a bit nicer screen output,
> it's still a little rough though.

Well, it looks nice, be sure to submit smooth version :-).

				Pavel

> @@ -85,10 +85,17 @@
>  
>  static void free_some_memory(void)
>  {
> -	printk("Freeing memory: ");
> -	while (shrink_all_memory(10000))
> -		printk(".");
> -	printk("|\n");
> +	int i = 0;
> +	char *p = "-\|/";
> +	
> +	printk("Freeing memory:  ");
> +	while (shrink_all_memory(10000)) {
> +		printk("\b%c", p[i]);
> +		i++;
> +		if (i > 3)
> +			i = 0;
> +	}
> +	printk("\bdone\n");
>  }

I'd leave dots here. Its usefull to see if it done something or not.

-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: [PATCH] make swsusp produce nicer screen output

[Erik Rigtorp]

On Mon, Aug 23, 2004 at 07:42:18PM +0200, Pavel Machek wrote:
> Well, it looks nice, be sure to submit smooth version :-).
I'm working on it :).

> > @@ -85,10 +85,17 @@
> >  
> >  static void free_some_memory(void)
> >  {
> > -	printk("Freeing memory: ");
> > -	while (shrink_all_memory(10000))
> > -		printk(".");
> > -	printk("|\n");
> > +	int i = 0;
> > +	char *p = "-\|/";
> > +	
> > +	printk("Freeing memory:  ");
> > +	while (shrink_all_memory(10000)) {
> > +		printk("\b%c", p[i]);
> > +		i++;
> > +		if (i > 3)
> > +			i = 0;
> > +	}
> > +	printk("\bdone\n");
> >  }
> 
> I'd leave dots here. Its usefull to see if it done something or not.

Well, it will display a spinning thingy that is updated every time
shrink_all_memory(10000) returns. Maybe you want to see how much memory was
freed?

And do we need to handle the case when nr_copy_pages < 100?

/Erik

Re: [PATCH] make swsusp produce nicer screen output

[Pavel Machek]

Hi!

> > Well, it looks nice, be sure to submit smooth version :-).
> I'm working on it :).

> > I'd leave dots here. Its usefull to see if it done something or not.
> 
> Well, it will display a spinning thingy that is updated every time
> shrink_all_memory(10000) returns. Maybe you want to see how much memory was
> freed?

Yes, it is quite important to see how many pages were freed even after
freeing stopped. "done (1234 pages freed)" would solve it...

> And do we need to handle the case when nr_copy_pages < 100?

It really should not crash. 100 pages is 4MB. Thats little low but
seems possible.
				Pavel
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: [PATCH] make swsusp produce nicer screen output

[Stefan Seyfried]

Pavel Machek wrote:
> Hi!
> 

>>And do we need to handle the case when nr_copy_pages < 100?
> > 
> It really should not crash. 100 pages is 4MB. Thats little low but
> seems possible.

400k IIUC :-), and although it seems impossible, we still should not 
crash if we counted it wrong for some strange reason.

> 				Pavel

Stefan

Re: [PATCH] make swsusp produce nicer screen output

[Pavel Machek]

Hi!

> >>And do we need to handle the case when nr_copy_pages < 100?
> >> 
> >It really should not crash. 100 pages is 4MB. Thats little low but
> >seems possible.
> 
> 400k IIUC :-), and although it seems impossible, we still should not 
> crash if we counted it wrong for some strange reason.
> 

Yup, you are right. Okay so make it "can display garbage but should
do no other harm on <100 pages".
				Pavel

-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: [PATCH] make swsusp produce nicer screen output

[Erik Rigtorp]

On Tue, Aug 24, 2004 at 11:49:30PM +0200, Pavel Machek wrote:
> Hi!
> 
> > > Well, it looks nice, be sure to submit smooth version :-).
> > I'm working on it :).
> 
> > > I'd leave dots here. Its usefull to see if it done something or not.
> > 
> > Well, it will display a spinning thingy that is updated every time
> > shrink_all_memory(10000) returns. Maybe you want to see how much memory was
> > freed?
> 
> Yes, it is quite important to see how many pages were freed even after
> freeing stopped. "done (1234 pages freed)" would solve it...
Added code for this in the new patch

> 
> > And do we need to handle the case when nr_copy_pages < 100?
> 
> It really should not crash. 100 pages is 4MB. Thats little low but
> seems possible.

Well it's probably best to handle this case, to be on the safe side.

Here's a new version of the patch.

diff -Nru linux-2.6.8.1-mm2/kernel/power/disk.c linux-2.6.8.1-mm2-erkki/kernel/power/disk.c
--- linux-2.6.8.1-mm2/kernel/power/disk.c	2004-08-20 17:10:58.000000000 +0200
+++ linux-2.6.8.1-mm2-erkki/kernel/power/disk.c	2004-08-29 14:16:53.000000000 +0200
@@ -85,10 +85,20 @@
 
 static void free_some_memory(void)
 {
-	printk("Freeing memory: ");
-	while (shrink_all_memory(10000))
-		printk(".");
-	printk("|\n");
+	unsigned int i = 0;
+	unsigned int tmp;
+	unsigned long pages = 0;
+	char *p = "-\\|/";
+	
+	printk("Freeing memory...  ");
+	while (tmp = shrink_all_memory(10000)) {
+		pages += tmp;
+		printk("\b%c", p[i]);
+		i++;
+		if (i > 3)
+			i = 0;
+	}
+	printk("\bdone (%li pages freed)\n", pages);
 }
 
 
diff -Nru linux-2.6.8.1-mm2/kernel/power/swsusp.c linux-2.6.8.1-mm2-erkki/kernel/power/swsusp.c
--- linux-2.6.8.1-mm2/kernel/power/swsusp.c	2004-08-20 17:10:58.000000000 +0200
+++ linux-2.6.8.1-mm2-erkki/kernel/power/swsusp.c	2004-08-29 13:51:55.000000000 +0200
@@ -296,15 +296,21 @@
 {
 	int error = 0;
 	int i;
-
-	printk( "Writing data to swap (%d pages): ", nr_copy_pages );
+	unsigned int mod;
+	
+	if (nr_copy_pages < 100)
+		mod = 1;
+	else
+		mod = nr_copy_pages / 100;
+	
+	printk( "Writing data to swap (%d pages)...     ", nr_copy_pages );
 	for (i = 0; i < nr_copy_pages && !error; i++) {
-		if (!(i%100))
-			printk( "." );
+		if (!(i%mod))
+			printk( "\b\b\b\b%3d%%", i / mod );
 		error = write_page((pagedir_nosave+i)->address,
 					  &((pagedir_nosave+i)->swap_address));
 	}
-	printk(" %d Pages done.\n",i);
+	printk("\b\b\b\bdone\n");
 	return error;
 }
 
@@ -534,8 +540,6 @@
 	if (!pfn_valid(pfn))
 		return 0;
 
-	if (!(pfn%1000))
-		printk(".");
 	page = pfn_to_page(pfn);
 	BUG_ON(PageReserved(page) && PageNosave(page));
 	if (PageNosave(page))
@@ -556,15 +560,28 @@
 {
 	struct zone *zone;
 	unsigned long zone_pfn;
+	unsigned long mod;
+	
+	printk("Counting pages...     ");
 
 	nr_copy_pages = 0;
 
 	for_each_zone(zone) {
 		if (!is_highmem(zone)) {
-			for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
+			if (zone->spanned_pages < 100)
+				mod = 1;
+			else
+				mod = zone->spanned_pages / 100;
+			
+			for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn) {
 				nr_copy_pages += saveable(zone, &zone_pfn);
+				if (!(zone_pfn % mod))
+					printk("\b\b\b\b%3ld%%", zone_pfn / mod);
+			}
 		}
 	}
+	
+	printk("\b\b\b\bdone\n");
 }
 
 
@@ -573,9 +590,17 @@
 	struct zone *zone;
 	unsigned long zone_pfn;
 	struct pbe * pbe = pagedir_nosave;
+	unsigned long mod;
 	
+	printk("Copying pages...     ");
+
 	for_each_zone(zone) {
-		if (!is_highmem(zone))
+		if (!is_highmem(zone)) {
+			if (zone->spanned_pages < 100)
+				mod = 1;
+			else
+				mod = zone->spanned_pages / 100;
+			
 			for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn) {
 				if (saveable(zone, &zone_pfn)) {
 					struct page * page;
@@ -584,9 +609,14 @@
 					/* copy_page is no usable for copying task structs. */
 					memcpy((void *)pbe->address, (void *)pbe->orig_address, PAGE_SIZE);
 					pbe++;
+					if (!(zone_pfn % mod))
+						printk("\b\b\b\b%3ld%%", zone_pfn / mod);
 				}
 			}
+		}
 	}
+	
+	printk("\b\b\b\bdone\n");
 }
 
 
@@ -1150,14 +1180,20 @@
 	struct pbe * p;
 	int error;
 	int i;
-
+	int mod;
+	
+	if (nr_copy_pages < 100)
+		mod = 1;
+	else
+		mod = nr_copy_pages / 100;
+	
 	if ((error = swsusp_pagedir_relocate()))
 		return error;
 
-	printk( "Reading image data (%d pages): ", nr_copy_pages );
+	printk( "Reading image data (%d pages):     ", nr_copy_pages );
 	for(i = 0, p = pagedir_nosave; i < nr_copy_pages && !error; i++, p++) {
-		if (!(i%100))
-			printk( "." );
+		if (!(i%mod))
+			printk( "\b\b\b\b%3d%%", i / mod );
 		error = bio_read_page(swp_offset(p->swap_address),
 				  (void *)p->address);
 	}

Re: [PATCH] make swsusp produce nicer screen output

[Stefan Seyfried]

Erik Rigtorp wrote:
> On Tue, Aug 24, 2004 at 11:49:30PM +0200, Pavel Machek wrote:

>>It really should not crash. 100 pages is 4MB. Thats little low but
>>seems possible.
> 
> Well it's probably best to handle this case, to be on the safe side.

yes, better safe than sorry. And it's not exactly a hot code path.

> Here's a new version of the patch.

i like it,  FWIW :-)

Regards,

   Stefan

Re: [PATCH] make swsusp produce nicer screen output

[Pavel Machek]

Hi!

> > > > Well, it looks nice, be sure to submit smooth version :-).
> > > I'm working on it :).
> > 
> > > > I'd leave dots here. Its usefull to see if it done something or not.
> > > 
> > > Well, it will display a spinning thingy that is updated every time
> > > shrink_all_memory(10000) returns. Maybe you want to see how much memory was
> > > freed?
> > 
> > Yes, it is quite important to see how many pages were freed even after
> > freeing stopped. "done (1234 pages freed)" would solve it...
> Added code for this in the new patch
> 
> > 
> > > And do we need to handle the case when nr_copy_pages < 100?
> > 
> > It really should not crash. 100 pages is 4MB. Thats little low but
> > seems possible.
> 
> Well it's probably best to handle this case, to be on the safe side.
> 
> Here's a new version of the patch.

I like this patch. Can you add short description "better progress
reporting", say that I approved it and mail it to Andrew, Cc patrick
mochel?
								Pavel

> diff -Nru linux-2.6.8.1-mm2/kernel/power/disk.c linux-2.6.8.1-mm2-erkki/kernel/power/disk.c
> --- linux-2.6.8.1-mm2/kernel/power/disk.c	2004-08-20 17:10:58.000000000 +0200
> +++ linux-2.6.8.1-mm2-erkki/kernel/power/disk.c	2004-08-29 14:16:53.000000000 +0200
> @@ -85,10 +85,20 @@
>  
>  static void free_some_memory(void)
>  {
> -	printk("Freeing memory: ");
> -	while (shrink_all_memory(10000))
> -		printk(".");
> -	printk("|\n");
> +	unsigned int i = 0;
> +	unsigned int tmp;
> +	unsigned long pages = 0;
> +	char *p = "-\\|/";
> +	
> +	printk("Freeing memory...  ");
> +	while (tmp = shrink_all_memory(10000)) {
> +		pages += tmp;
> +		printk("\b%c", p[i]);
> +		i++;
> +		if (i > 3)
> +			i = 0;
> +	}
> +	printk("\bdone (%li pages freed)\n", pages);
>  }
>  
>  
> diff -Nru linux-2.6.8.1-mm2/kernel/power/swsusp.c linux-2.6.8.1-mm2-erkki/kernel/power/swsusp.c
> --- linux-2.6.8.1-mm2/kernel/power/swsusp.c	2004-08-20 17:10:58.000000000 +0200
> +++ linux-2.6.8.1-mm2-erkki/kernel/power/swsusp.c	2004-08-29 13:51:55.000000000 +0200
> @@ -296,15 +296,21 @@
>  {
>  	int error = 0;
>  	int i;
> -
> -	printk( "Writing data to swap (%d pages): ", nr_copy_pages );
> +	unsigned int mod;
> +	
> +	if (nr_copy_pages < 100)
> +		mod = 1;
> +	else
> +		mod = nr_copy_pages / 100;
> +	
> +	printk( "Writing data to swap (%d pages)...     ", nr_copy_pages );
>  	for (i = 0; i < nr_copy_pages && !error; i++) {
> -		if (!(i%100))
> -			printk( "." );
> +		if (!(i%mod))
> +			printk( "\b\b\b\b%3d%%", i / mod );
>  		error = write_page((pagedir_nosave+i)->address,
>  					  &((pagedir_nosave+i)->swap_address));
>  	}
> -	printk(" %d Pages done.\n",i);
> +	printk("\b\b\b\bdone\n");
>  	return error;
>  }
>  
> @@ -534,8 +540,6 @@
>  	if (!pfn_valid(pfn))
>  		return 0;
>  
> -	if (!(pfn%1000))
> -		printk(".");
>  	page = pfn_to_page(pfn);
>  	BUG_ON(PageReserved(page) && PageNosave(page));
>  	if (PageNosave(page))
> @@ -556,15 +560,28 @@
>  {
>  	struct zone *zone;
>  	unsigned long zone_pfn;
> +	unsigned long mod;
> +	
> +	printk("Counting pages...     ");
>  
>  	nr_copy_pages = 0;
>  
>  	for_each_zone(zone) {
>  		if (!is_highmem(zone)) {
> -			for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn)
> +			if (zone->spanned_pages < 100)
> +				mod = 1;
> +			else
> +				mod = zone->spanned_pages / 100;
> +			
> +			for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn) {
>  				nr_copy_pages += saveable(zone, &zone_pfn);
> +				if (!(zone_pfn % mod))
> +					printk("\b\b\b\b%3ld%%", zone_pfn / mod);
> +			}
>  		}
>  	}
> +	
> +	printk("\b\b\b\bdone\n");
>  }
>  
>  
> @@ -573,9 +590,17 @@
>  	struct zone *zone;
>  	unsigned long zone_pfn;
>  	struct pbe * pbe = pagedir_nosave;
> +	unsigned long mod;
>  	
> +	printk("Copying pages...     ");
> +
>  	for_each_zone(zone) {
> -		if (!is_highmem(zone))
> +		if (!is_highmem(zone)) {
> +			if (zone->spanned_pages < 100)
> +				mod = 1;
> +			else
> +				mod = zone->spanned_pages / 100;
> +			
>  			for (zone_pfn = 0; zone_pfn < zone->spanned_pages; ++zone_pfn) {
>  				if (saveable(zone, &zone_pfn)) {
>  					struct page * page;
> @@ -584,9 +609,14 @@
>  					/* copy_page is no usable for copying task structs. */
>  					memcpy((void *)pbe->address, (void *)pbe->orig_address, PAGE_SIZE);
>  					pbe++;
> +					if (!(zone_pfn % mod))
> +						printk("\b\b\b\b%3ld%%", zone_pfn / mod);
>  				}
>  			}
> +		}
>  	}
> +	
> +	printk("\b\b\b\bdone\n");
>  }
>  
>  
> @@ -1150,14 +1180,20 @@
>  	struct pbe * p;
>  	int error;
>  	int i;
> -
> +	int mod;
> +	
> +	if (nr_copy_pages < 100)
> +		mod = 1;
> +	else
> +		mod = nr_copy_pages / 100;
> +	
>  	if ((error = swsusp_pagedir_relocate()))
>  		return error;
>  
> -	printk( "Reading image data (%d pages): ", nr_copy_pages );
> +	printk( "Reading image data (%d pages):     ", nr_copy_pages );
>  	for(i = 0, p = pagedir_nosave; i < nr_copy_pages && !error; i++, p++) {
> -		if (!(i%100))
> -			printk( "." );
> +		if (!(i%mod))
> +			printk( "\b\b\b\b%3d%%", i / mod );
>  		error = bio_read_page(swp_offset(p->swap_address),
>  				  (void *)p->address);
>  	}

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Re: [PATCH] make swsusp produce nicer screen output

[Pavel Machek]

Hi!

> > > > Well, it looks nice, be sure to submit smooth version :-).
> > > I'm working on it :).
> > 
> > > > I'd leave dots here. Its usefull to see if it done something or not.
> > > 
> > > Well, it will display a spinning thingy that is updated every time
> > > shrink_all_memory(10000) returns. Maybe you want to see how much memory was
> > > freed?
> > 
> > Yes, it is quite important to see how many pages were freed even after
> > freeing stopped. "done (1234 pages freed)" would solve it...
> Added code for this in the new patch

Okay, wait with that pushing. I merged it here, and did some speedups
(big) in the meantime. Percents for copying memory should no longer be
neccessary.

								Pavel

-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!