Skip to contentSkip to conversation

swappiness=0 makes software suspend fail.

DISCUSSION Rob Landley 10 participants 25 messages May 28, 2004 – Jun 4, 2004

Subsystems (3)
Memory management MGLRU MM reclaim
lore
Share & tools
lore

Rob Landley

author
#0 lore raw

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

Signature · 2 lines
www.linucon.org: Linux Expo and Science Fiction Convention
October 8-10, 2004 in Austin Texas.  (I'm the con chair.)
#1 lore raw

Hi!

Quoted 5 lines

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

Signature · 1 line
934a471f20d6580d5aad759bf0d97ddc
#2 lore raw

Hi.

Quoted 7 lines · Pavel Machek
Quoted 3 lines

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.

Quoted 7 lines

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

Signature · 6 lines
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.
#3 lore raw
In reply to Pavel Machek
Quoted 2 lines · Pavel Machek

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).

Signature · 2 lines
 Stuart Young (aka Cef)
 [email protected] is for LKML and related email only
#4 lore raw

Hi.

Quoted 7 lines · Stuart Young
Quoted 2 lines · Pavel Machek

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

Signature · 6 lines
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.
#5 lore raw

Hi!

Quoted 10 lines
Quoted 7 lines
Quoted 2 lines

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

Signature · 1 line
934a471f20d6580d5aad759bf0d97ddc
#6 lore raw
Quoted 3 lines · Pavel Machek

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.

mm/vmscan.c renamed from mm/vmscan.c~shrink_all_memory-fix +4 -3
@@ -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,

_

#7 lore raw
Quoted 8 lines · Andrew Morton
Quoted 2 lines · Pavel Machek

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.

author
#9 lore raw
Quoted 2 lines · 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.

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

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

Hi!

Quoted 5 lines
Quoted 2 lines

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

Signature · 2 lines
Horseback riding is like software...
...vgf orggre jura vgf serr.
#11 lore raw

Hi.

Quoted 7 lines · Pavel Machek
Quoted 5 lines
Quoted 2 lines

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.

Quoted 7 lines

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

Signature · 7 lines
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.
#12 lore raw

Hi!

Quoted 11 lines
Quoted 11 lines
Quoted 6 lines
Quoted 3 lines

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.

Quoted 8 lines
Quoted 3 lines

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

Signature · 1 line
934a471f20d6580d5aad759bf0d97ddc
#13 lore raw
In reply to Rob Landley
Quoted 1 line · Rob Landley

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.

Signature · 1 line
Ciao, Flavio
#14 lore raw

Hi!

Quoted 9 lines
Quoted 2 lines

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

Signature · 1 line
934a471f20d6580d5aad759bf0d97ddc
#15 lore raw
In reply to Andrew Morton

Hi!

Quoted 2 lines

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

Signature · 2 lines
Horseback riding is like software...
...vgf orggre jura vgf serr.
#16 lore raw
Quoted 2 lines · Pavel Machek

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.

Signature · 4 lines
Stefan Seyfried                     QA / R&D mobile devices, SUSE LINUX AG

"Any ideas, John?"
"Well, surrounding thems out."
#17 lore raw
In reply to Nigel Cunningham

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

Signature · 1 line
934a471f20d6580d5aad759bf0d97ddc
#18 lore raw
Quoted 10 lines · 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.

#19 lore raw

Hi!

Quoted 24 lines
Quoted 8 lines

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;
};
Quoted 5 lines

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

mm/vmscan.c +5 -0
@@ -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
Signature · 1 line
934a471f20d6580d5aad759bf0d97ddc
#20 lore raw

Am Sonntag, 30. Mai 2004 21:47 schrieb Pavel Machek:

Quoted 4 lines
Quoted 3 lines

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

author
#21 lore raw
Quoted 6 lines · Oliver Neukum

Am Sonntag, 30. Mai 2004 21:47 schrieb Pavel Machek:

Quoted 3 lines
Quoted 3 lines

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...)

Quoted 2 lines

Regards
Oliver

Rob

Signature · 2 lines
www.linucon.org: Linux Expo and Science Fiction Convention
October 8-10, 2004 in Austin Texas.  (I'm the con chair.)
#22 lore raw
In reply to Pavel Machek
Quoted 53 lines · Pavel Machek
Quoted 27 lines
Quoted 10 lines · Nick Piggin

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;
};
Quoted 5 lines

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

mm/vmscan.c +3 -0
@@ -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;

mm/vmscan.c line 1125

Quoted 8 lines
mm/vmscan.c +2 -0
@@ -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?

Signature · 2 lines
 Stuart Young (aka Cef)
 [email protected] is for LKML and related email only
#23 lore raw

Hi!

Quoted 24 lines
Quoted 24 lines
mm/vmscan.c +5 -0
@@ -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;
 }
Quoted 8 lines

mm/vmscan.c line 1125

Quoted 1 line
mm/vmscan.c +0 -0
 #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

Signature · 1 line
934a471f20d6580d5aad759bf0d97ddc
#24 lore raw
In reply to Stuart Young

Hi!

Quoted 6 lines
Quoted 2 lines

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

Signature · 1 line
934a471f20d6580d5aad759bf0d97ddc