swappiness=0 makes software suspend fail.

swappiness=0 makes software suspend fail.

[Rob Landley]

With swappiness at the default (60), software suspend frees all the memory it 
needs.  With swappiness at 0, software suspend basically doesn't free any 
memory, and the suspend gets aborted.

Just thought I'd mention it.  Tried on 2.6.6...

Rob
-- 
www.linucon.org: Linux Expo and Science Fiction Convention
October 8-10, 2004 in Austin Texas.  (I'm the con chair.)

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

> With swappiness at the default (60), software suspend frees all the memory it 
> needs.  With swappiness at 0, software suspend basically doesn't free any 
> memory, and the suspend gets aborted.
> 
> Just thought I'd mention it.  Tried on 2.6.6...

Uh, yes, right.

That explains why some people see bad problems I could not
reproduce. Thanks a lot.

Stefan, we may want to do echo 100 > /proc/sys/vm/swappiness in
suspend script...

									Pavel
-- 
934a471f20d6580d5aad759bf0d97ddc

Re: swappiness=0 makes software suspend fail.

[Nigel Cunningham]

Hi.

Pavel Machek wrote:
>>With swappiness at the default (60), software suspend frees all the memory it 
>>needs.  With swappiness at 0, software suspend basically doesn't free any 
>>memory, and the suspend gets aborted.
>>
>>Just thought I'd mention it.  Tried on 2.6.6...
> 
> 
> Uh, yes, right.
> 
> That explains why some people see bad problems I could not
> reproduce. Thanks a lot.
> 
> Stefan, we may want to do echo 100 > /proc/sys/vm/swappiness in
> suspend script...
> 
> 									Pavel

This applies to suspend2 for 2.6 as well. I recently changed to using the same routines to free memory.

Nigel


-- 
Nigel & Michelle Cunningham
C/- Westminster Presbyterian Church Belconnen
61 Templeton Street, Cook, ACT 2614.
+61 (2) 6251 7727(wk); +61 (2) 6254 0216 (home)

After homosexuality, they'll be arguing paedophilia is normal.

Re: swappiness=0 makes software suspend fail.

[Stuart Young]

On Sat, 29 May 2004 07:56, Pavel Machek wrote:
> Stefan, we may want to do echo 100 > /proc/sys/vm/swappiness in
> suspend script...

Really, you should save that value somewhere and then restore it after 
suspend, or those people who do use /proc/sys/vm/swappiness will likely 
complain about it (ie: me).

-- 
 Stuart Young (aka Cef)
 cef-lkml@optusnet.com.au is for LKML and related email only

Re: swappiness=0 makes software suspend fail.

[Nigel Cunningham]

Hi.

Stuart Young wrote:
> On Sat, 29 May 2004 07:56, Pavel Machek wrote:
> 
>>Stefan, we may want to do echo 100 > /proc/sys/vm/swappiness in
>>suspend script...
> 
> 
> Really, you should save that value somewhere and then restore it after 
> suspend, or those people who do use /proc/sys/vm/swappiness will likely 
> complain about it (ie: me).

Yes. This doesn't need to be done by the script. I'll change suspend2 so it saves and restores the 
value.

Nigel
-- 
Nigel & Michelle Cunningham
C/- Westminster Presbyterian Church Belconnen
61 Templeton Street, Cook, ACT 2614.
+61 (417) 100 574 (mobile)

After homosexuality, they'll be arguing paedophilia is normal.

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

> >>Stefan, we may want to do echo 100 > /proc/sys/vm/swappiness in
> >>suspend script...
> >
> >
> >Really, you should save that value somewhere and then restore it after 
> >suspend, or those people who do use /proc/sys/vm/swappiness will likely 
> >complain about it (ie: me).
> 
> Yes. This doesn't need to be done by the script. I'll change suspend2 so it 
> saves and restores the value.

That's wrong solution.

Right solution is to make sure that shrink_all_memory() works, no
matter how swappiness is set.
								Pavel
-- 
934a471f20d6580d5aad759bf0d97ddc

Re: swappiness=0 makes software suspend fail.

[Andrew Morton]

Pavel Machek <pavel@suse.cz> wrote:
>
> Right solution is to make sure that shrink_all_memory() works, no
>  matter how swappiness is set.

off-by-one in balance_pgdat() was the main problem.


--- 25/mm/vmscan.c~shrink_all_memory-fix	2004-05-31 03:04:05.669374824 -0700
+++ 25-akpm/mm/vmscan.c	2004-05-31 03:04:05.673374216 -0700
@@ -813,8 +813,7 @@ shrink_caches(struct zone **zones, int p
 		struct zone *zone = zones[i];
 		int max_scan;
 
-		if (zone->free_pages < zone->pages_high)
-			zone->temp_priority = priority;
+		zone->temp_priority = priority;
 
 		if (zone->all_unreclaimable && priority != DEF_PRIORITY)
 			continue;	/* Let kswapd poll it */
@@ -945,7 +944,7 @@ static int balance_pgdat(pg_data_t *pgda
 		zone->temp_priority = DEF_PRIORITY;
 	}
 
-	for (priority = DEF_PRIORITY; priority; priority--) {
+	for (priority = DEF_PRIORITY; priority >= 0; priority--) {
 		int all_zones_ok = 1;
 		int end_zone = 0;	/* Inclusive.  0 = ZONE_DMA */
 
@@ -995,6 +994,8 @@ scan:
 					all_zones_ok = 0;
 			}
 			zone->temp_priority = priority;
+			if (zone->prev_priority > priority)
+				zone->prev_priority = priority;
 			max_scan = (zone->nr_active + zone->nr_inactive)
 								>> priority;
 			reclaimed = shrink_zone(zone, max_scan, GFP_KERNEL,
_

Re: swappiness=0 makes software suspend fail.

[Nick Piggin]

Andrew Morton wrote:
> Pavel Machek <pavel@suse.cz> wrote:
> 
>>Right solution is to make sure that shrink_all_memory() works, no
>> matter how swappiness is set.
> 
> 
> off-by-one in balance_pgdat() was the main problem.
> 

I think it might be intentional, because 1/2 + 1/4 + ... + 1/4096 ~ 1.

If so, it instead needs fixing in try_to_free_pages, and a comment.

Re: swappiness=0 makes software suspend fail.

[Andrew Morton]

btw, software suspend wrecks your swap partition if you suspend to swap but
do not resume from swap - you need to run mkswap again.  Seems odd.

Re: swappiness=0 makes software suspend fail.

[Rob Landley]

On Monday 31 May 2004 05:17, Andrew Morton wrote:
> btw, software suspend wrecks your swap partition if you suspend to swap but
> do not resume from swap - you need to run mkswap again.  Seems odd.

I think it's intentional, so that if you you boot to a different kernel swapon 
-a won't automount the swap partition and hork your saved image.

Of course, mounting/fscking any of the filesystems in question would kinda 
screw that up too, and if the swap partition's is in the other kernel's fstab 
then presumably overlapping filesystems probably are too.  (Intentional isn't 
necessarily the same thing as right... :)

Rob

-- 
www.linucon.org: Linux Expo and Science Fiction Convention
October 8-10, 2004 in Austin Texas.  (I'm the con chair.)

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

> > btw, software suspend wrecks your swap partition if you suspend to swap but
> > do not resume from swap - you need to run mkswap again.  Seems odd.
> 
> I think it's intentional, so that if you you boot to a different kernel swapon 
> -a won't automount the swap partition and hork your saved image.

Actually, we *want* to hork that saved image, because it is extremely
dangerous to resume from it.

We also want to kill suspend signature ASAP, so that if driver kills
resume and user presses reset, we will not try to resume again and
fail in exactly same way.
								Pavel
-- 
Horseback riding is like software...
...vgf orggre jura vgf serr.

Re: swappiness=0 makes software suspend fail.

[Nigel Cunningham]

Hi.

Pavel Machek wrote:
>>>btw, software suspend wrecks your swap partition if you suspend to swap but
>>>do not resume from swap - you need to run mkswap again.  Seems odd.
>>
>>I think it's intentional, so that if you you boot to a different kernel swapon 
>>-a won't automount the swap partition and hork your saved image.
> 
> 
> Actually, we *want* to hork that saved image, because it is extremely
> dangerous to resume from it.
> 
> We also want to kill suspend signature ASAP, so that if driver kills
> resume and user presses reset, we will not try to resume again and
> fail in exactly same way.
> 								Pavel

Suspend2 fixes the header and records when you've attempted to resume from it. If you try a second 
time it gives you the option of invalidating the image or trying to resume. Pavel, feel free to grab 
the code out of suspend2 if you want.

Nigel
-- 
Nigel & Michelle Cunningham
C/- Westminster Presbyterian Church Belconnen
61 Templeton Street, Cook, ACT 2614.
+61 (417) 100 574 (mobile)

Intolerance (n): Holding a view point with any degree of
  conviction.

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

> >>>btw, software suspend wrecks your swap partition if you suspend to swap 
> >>>but
> >>>do not resume from swap - you need to run mkswap again.  Seems odd.
> >>
> >>I think it's intentional, so that if you you boot to a different kernel 
> >>swapon -a won't automount the swap partition and hork your saved image.
> >
> >
> >Actually, we *want* to hork that saved image, because it is extremely
> >dangerous to resume from it.
> >
> >We also want to kill suspend signature ASAP, so that if driver kills
> >resume and user presses reset, we will not try to resume again and
> >fail in exactly same way.

> Suspend2 fixes the header and records when you've attempted to resume from 
> it. If you try a second time it gives you the option of invalidating the 
> image or trying to resume. Pavel, feel free to grab the code out of 
> suspend2 if you want.

It should be simpler to just move already-existing signature changing
code to right place, but thanks anyway.
								Pavel

-- 
934a471f20d6580d5aad759bf0d97ddc

Re: swappiness=0 makes software suspend fail.

[Flavio Stanchina]

Rob Landley wrote:
> Of course, mounting/fscking any of the filesystems in question would kinda 
> screw that up too, [...]

That reminds me of a question I wanted to ask for a long time.

Why doesn't suspend just remount everything read-only before saving the
memory image? Would that be impossible in this context? I find it quite
scary to have my filesystems dirty *and* part of my files saved in the
memory image.

-- 
Ciao, Flavio

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

> >Of course, mounting/fscking any of the filesystems in question would kinda 
> >screw that up too, [...]
> 
> That reminds me of a question I wanted to ask for a long time.
> 
> Why doesn't suspend just remount everything read-only before saving the
> memory image? Would that be impossible in this context? I find it quite
> scary to have my filesystems dirty *and* part of my files saved in the
> memory image.

Try umount / on busy system some day. No, its not possible in this
context.

OTOH swsuspend does sync(), so your filesystems are not in *that*
scary state.

									Pavel

-- 
934a471f20d6580d5aad759bf0d97ddc

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

> btw, software suspend wrecks your swap partition if you suspend to swap but
> do not resume from swap - you need to run mkswap again.  Seems odd.

Its half-intentional. We need to change signature to something else so
that kernel knows "this is suspend partition", and I never got around
to fixing it back on unsuccessfull suspend.

I believe stefan has some script that fixes swap signature using dd if
it detects suspend signature...
								Pavel

-- 
Horseback riding is like software...
...vgf orggre jura vgf serr.

Re: swappiness=0 makes software suspend fail.

[Stefan Seyfried]

On Mon, May 31, 2004 at 01:50:49PM +0200, Pavel Machek wrote:

> I believe stefan has some script that fixes swap signature using dd if
> it detects suspend signature...

this is in boot.swap initscript:

check_swap_sig () {
    local part="$(get_swap_id)"
    local where what type rest p c
    while read  where what type rest ; do
        test "$type" = "swap" || continue
        c=continue
        for p in $part ; do
            test "$p" = "$where" && c=true
        done
        $c
        case "$(dd if=$where bs=1 count=6 skip=4086 2>/dev/null)" in
        S1SUSP|S2SUSP) mkswap $where
        esac
    done < /etc/fstab
}

so if the user passes "noresume", the boot scripts fix up the mess :-)
An unconditional "mkswap" on the swap partition is a not-so-good idea
unless you check the ID of the partition from the partition table since
mkswap will create swap on anything you give him which may not always
be what you wanted.

Anyway, i think it is not too bad doing such cleanup stuff in userspace.

-- 
Stefan Seyfried                     QA / R&D mobile devices, SUSE LINUX AG

"Any ideas, John?"
"Well, surrounding thems out."

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

Andrew, in 2.6.6 shrink_all_memory() does not work if swappiness ==
0. shrink_all_memory() calls balance_pgdat(), that calls
shrink_zone(), and that calls refill_inactive_zone(), which looks at
swappiness.

Additional parameter to all these calls neutralizing swappiness would
help, as would temporarily setting swappiness to 100 in
shrink_all_memory. Is there a less ugly solution?
								Pavel
-- 
934a471f20d6580d5aad759bf0d97ddc

Re: swappiness=0 makes software suspend fail.

[Nick Piggin]

Pavel Machek wrote:
> Hi!
> 
> Andrew, in 2.6.6 shrink_all_memory() does not work if swappiness ==
> 0. shrink_all_memory() calls balance_pgdat(), that calls
> shrink_zone(), and that calls refill_inactive_zone(), which looks at
> swappiness.
> 
> Additional parameter to all these calls neutralizing swappiness would
> help, as would temporarily setting swappiness to 100 in
> shrink_all_memory. Is there a less ugly solution?

I have a cleanup patch that allows this sort of thing to easily
be passed into the lower levels of reclaim functions. I don't
know if it would be to Andrew's taste though...

It basically replaces all function parameters in vmscan.c with

struct scan_control {
	unsigned long nr_to_scan;
	unsigned long nr_scanned;
	unsigned long nr_reclaimed;
	unsigned int gfp_mask;
	struct page_state ps;
	int may_writepage;
};

So you could easily add a field for swsusp.

Until something like this goes through, please don't fuglify
vmscan.c any more than it is... do the saving and restoring
thing that Nigel suggested please.

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

> >Andrew, in 2.6.6 shrink_all_memory() does not work if swappiness ==
> >0. shrink_all_memory() calls balance_pgdat(), that calls
> >shrink_zone(), and that calls refill_inactive_zone(), which looks at
> >swappiness.
> >
> >Additional parameter to all these calls neutralizing swappiness would
> >help, as would temporarily setting swappiness to 100 in
> >shrink_all_memory. Is there a less ugly solution?
> 
> I have a cleanup patch that allows this sort of thing to easily
> be passed into the lower levels of reclaim functions. I don't
> know if it would be to Andrew's taste though...
> 
> It basically replaces all function parameters in vmscan.c with
> 
> struct scan_control {
> 	unsigned long nr_to_scan;
> 	unsigned long nr_scanned;
> 	unsigned long nr_reclaimed;
> 	unsigned int gfp_mask;
> 	struct page_state ps;
> 	int may_writepage;
> };
> 
> So you could easily add a field for swsusp.
> 
> Until something like this goes through, please don't fuglify
> vmscan.c any more than it is... do the saving and restoring
> thing that Nigel suggested please.

Okay, this should solve it.
							Pavel

--- clean/mm/vmscan.c	2004-05-20 23:08:37.000000000 +0200
+++ linux/mm/vmscan.c	2004-05-30 21:45:41.000000000 +0200
@@ -1098,10 +1098,13 @@
 	pg_data_t *pgdat;
 	int nr_to_free = nr_pages;
 	int ret = 0;
+	int old_swappiness = vm_swappiness;
 	struct reclaim_state reclaim_state = {
 		.reclaimed_slab = 0,
 	};
 
+	vm_swappiness = 100;
+
 	current->reclaim_state = &reclaim_state;
 	for_each_pgdat(pgdat) {
 		int freed;
@@ -1115,6 +1118,8 @@
 			break;
 	}
 	current->reclaim_state = NULL;
+
+	vm_swappiness = old_swappiness;
 	return ret;
 }
 #endif


-- 
934a471f20d6580d5aad759bf0d97ddc

Re: swappiness=0 makes software suspend fail.

[Oliver Neukum]

Am Sonntag, 30. Mai 2004 21:47 schrieb Pavel Machek:
> > Until something like this goes through, please don't fuglify
> > vmscan.c any more than it is... do the saving and restoring
> > thing that Nigel suggested please.
> 

Isn't that a race condition with setting swapiness?

	Regards
		Oliver

Re: swappiness=0 makes software suspend fail.

[Rob Landley]

On Sunday 30 May 2004 18:23, Oliver Neukum wrote:
> Am Sonntag, 30. Mai 2004 21:47 schrieb Pavel Machek:
> > > Until something like this goes through, please don't fuglify
> > > vmscan.c any more than it is... do the saving and restoring
> > > thing that Nigel suggested please.
>
> Isn't that a race condition with setting swapiness?

During suspend?  Not really.  If it's done from the userspace script, it's 
done before the suspend is triggered, and undone after suspend comes back.  
No problem there.  If it's done in the kernel, then all the userspace 
thingies that might play with it have already been frozen, it's never touched 
from interrupt context...  What would you be locking _against_?

Swappiness isn't really twiddled from a lot of places.  Maybe your init script 
touches it.  Other than that, you have to be root, and you pretty much have 
to do it manually.  Touching this tuning knob is about as common as touching 
/proc/sys/net/ipv4/tcp_ecn or /proc/sys/kernel/panic.

The failure condition is graceful, by the way.  The suspend doesn't free 
enough memory, and thus resumes userspace immediately.  Kind of annoying if 
you've packed away your laptop to let it power down (since it can take a good 
45 seconds to do so, depending on how fragmented your swap file and memory 
and such are...)

> 	Regards
> 		Oliver

Rob

-- 
www.linucon.org: Linux Expo and Science Fiction Convention
October 8-10, 2004 in Austin Texas.  (I'm the con chair.)

Re: swappiness=0 makes software suspend fail.

[Stuart Young]

On Mon, 31 May 2004 05:47, Pavel Machek wrote:
> > Nick Piggin wrote:
> > > Pavel Machek wrote: 
>
> > >Andrew, in 2.6.6 shrink_all_memory() does not work if swappiness ==
> > >0. shrink_all_memory() calls balance_pgdat(), that calls
> > >shrink_zone(), and that calls refill_inactive_zone(), which looks at
> > >swappiness.
> > >
> > >Additional parameter to all these calls neutralizing swappiness would
> > >help, as would temporarily setting swappiness to 100 in
> > >shrink_all_memory. Is there a less ugly solution?
> >
> > I have a cleanup patch that allows this sort of thing to easily
> > be passed into the lower levels of reclaim functions. I don't
> > know if it would be to Andrew's taste though...
> >
> > It basically replaces all function parameters in vmscan.c with
> >
> > struct scan_control {
> > 	unsigned long nr_to_scan;
> > 	unsigned long nr_scanned;
> > 	unsigned long nr_reclaimed;
> > 	unsigned int gfp_mask;
> > 	struct page_state ps;
> > 	int may_writepage;
> > };
> >
> > So you could easily add a field for swsusp.
> >
> > Until something like this goes through, please don't fuglify
> > vmscan.c any more than it is... do the saving and restoring
> > thing that Nigel suggested please.
>
> Okay, this should solve it.
> 							Pavel
>
> --- clean/mm/vmscan.c	2004-05-20 23:08:37.000000000 +0200
> +++ linux/mm/vmscan.c	2004-05-30 21:45:41.000000000 +0200
> @@ -1098,10 +1098,13 @@
>  	pg_data_t *pgdat;
>  	int nr_to_free = nr_pages;
>  	int ret = 0;
> +	int old_swappiness = vm_swappiness;
>  	struct reclaim_state reclaim_state = {
>  		.reclaimed_slab = 0,
>  	};
>
> +	vm_swappiness = 100;
> +
>  	current->reclaim_state = &reclaim_state;
>  	for_each_pgdat(pgdat) {
>  		int freed;
> @@ -1115,6 +1118,8 @@
>  			break;
>  	}
>  	current->reclaim_state = NULL;
> +
> +	vm_swappiness = old_swappiness;
>  	return ret;
>  }
>  #endif

Good stuff. I've cc'ed Con Kolivas in on this as he's just recently posted his 
updated "Autoregulated VM swappiness" patch. In particular, this could also 
cause some issues if it made it into the main tree, as then this code might 
fail/cause issues (eg: as both could end up writing to vm_swappiness at the 
same time). This could possibly be a race condition as per Oliver's earlier 
observation (even if it's non-fatal, it's at least annoying).

Just thinking that Con could check to see somehow wether we're going to 
suspend, and not touch vm_swappiness anymore if that's the case. Yes I know 
the code isn't in the main tree yet, but who knows when another writer to 
vm_swappiness might end up in the main tree. It may also prove a good example 
for other potential writers. Pavel, Nigel, (anyone?) suggestions?

-- 
 Stuart Young (aka Cef)
 cef-lkml@optusnet.com.au is for LKML and related email only

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

> > --- clean/mm/vmscan.c	2004-05-20 23:08:37.000000000 +0200
> > +++ linux/mm/vmscan.c	2004-05-30 21:45:41.000000000 +0200
> > @@ -1098,10 +1098,13 @@
> >  	pg_data_t *pgdat;
> >  	int nr_to_free = nr_pages;
> >  	int ret = 0;
> > +	int old_swappiness = vm_swappiness;
> >  	struct reclaim_state reclaim_state = {
> >  		.reclaimed_slab = 0,
> >  	};
> >
> > +	vm_swappiness = 100;
> > +
> >  	current->reclaim_state = &reclaim_state;
> >  	for_each_pgdat(pgdat) {
> >  		int freed;
> > @@ -1115,6 +1118,8 @@
> >  			break;
> >  	}
> >  	current->reclaim_state = NULL;
> > +
> > +	vm_swappiness = old_swappiness;
> >  	return ret;
> >  }
> >  #endif
> 
> Good stuff. I've cc'ed Con Kolivas in on this as he's just recently posted his 
> updated "Autoregulated VM swappiness" patch. In particular, this could also 
> cause some issues if it made it into the main tree, as then this code might 
> fail/cause issues (eg: as both could end up writing to vm_swappiness at the 
> same time). This could possibly be a race condition as per Oliver's earlier 
> observation (even if it's non-fatal, it's at least annoying).

During suspend, all processes are frozen and we are running on single
processor. I do not think we can race with anyone.
								Pavel
-- 
934a471f20d6580d5aad759bf0d97ddc

Re: swappiness=0 makes software suspend fail.

[Pavel Machek]

Hi!

> > Stefan, we may want to do echo 100 > /proc/sys/vm/swappiness in
> > suspend script...
> 
> Really, you should save that value somewhere and then restore it after 
> suspend, or those people who do use /proc/sys/vm/swappiness will likely 
> complain about it (ie: me).

Yes, that was what I meant.
								Pavel
-- 
934a471f20d6580d5aad759bf0d97ddc