Trading off safety and performance in the kernel

Posted May 12, 2015 20:28 UTC (Tue) by pj (subscriber, #4506)
Parent article: Trading off safety and performance in the kernel

wrt Suspend-without-sync - I suspect whether or not to sync is going to want to be determined on a per-call basis, so it would seem to me that another parameter/flag to the suspend call would be a better solution than a sysctl knob. Otherwise there might be a weird (though I guess not really harmful?) race condition on devices that normally suspend-without-sync but want to sometimes suspend-with-sync: that device that suspends between keystrokes might want to suspend-with-sync on lid-close, for instance.

Trading off safety and performance in the kernel

Posted May 12, 2015 20:36 UTC (Tue) by zuki (subscriber, #41808) [Link] (9 responses)

There's no suspend() call. You just write stuff to /sys/power/disk and /sys/power/state. It's already racy.

Trading off safety and performance in the kernel

Posted May 12, 2015 22:01 UTC (Tue) by neilbrown (subscriber, #359) [Link] (8 responses)

> It's already racy.

Racy in what sense?

Trading off safety and performance in the kernel

Posted May 13, 2015 2:52 UTC (Wed) by krakensden (subscriber, #72039) [Link] (6 responses)

If suspend means "execute this script":

#!/bin/sh
sync
echo -n mem > /sys/power/state

then there's a window between writing to disk and suspending the system where the in-memory filesystem cache can get dirtied.

Trading off safety and performance in the kernel

Posted May 13, 2015 3:03 UTC (Wed) by neilbrown (subscriber, #359) [Link] (1 responses)

Thanks for being specific.

In the enter_state() function in kernel/power/suspend.c we have:

	trace_suspend_resume(TPS("sync_filesystems"), 0, true);
	printk(KERN_INFO "PM: Syncing filesystems ... ");
	sys_sync();
	printk("done.\n");
	trace_suspend_resume(TPS("sync_filesystems"), 0, false);

	pr_debug("PM: Preparing system for %s sleep\n", pm_states[state]);
	error = suspend_prepare(state);

and in suspend_prepare():

	pm_prepare_console();

	error = pm_notifier_call_chain(PM_SUSPEND_PREPARE);
	if (error)
		goto Finish;

	trace_suspend_resume(TPS("freeze_processes"), 0, true);
	error = suspend_freeze_processes();
	trace_suspend_resume(TPS("freeze_processes"), 0, false);

So the freezing of user-space processes happens *after* the sys_sync call. So that race is already present.

Trading off safety and performance in the kernel

Posted May 13, 2015 3:41 UTC (Wed) by chloe_zen (guest, #8258) [Link]

Given fuse, it rather has to be sync-then-suspend, doesn't it?

Trading off safety and performance in the kernel

Posted May 13, 2015 19:12 UTC (Wed) by flussence (guest, #85566) [Link] (3 responses)

A tangentially related aside: "echo -n" no longer works that way in recent versions of dash, which is the default /bin/sh on some distros. You'd need to use printf(1) instead.

Trading off safety and performance in the kernel

Posted May 13, 2015 22:41 UTC (Wed) by neilbrown (subscriber, #359) [Link]

> A tangentially related aside: "echo -n" no longer works that way in recent versions of dash,

I wonder where this habit of using "-n" came from. Just

echo mem > /sys/power/state

If any file in /sys requires "echo -n" then that is kernel bug. Please report it.

Trading off safety and performance in the kernel

Posted May 14, 2015 0:42 UTC (Thu) by mathstuf (subscriber, #69389) [Link] (1 responses)

-n on echo was never portable.

Trading off safety and performance in the kernel

Posted May 15, 2015 17:27 UTC (Fri) by flussence (guest, #85566) [Link]

I'm aware of that, but there's a huge amount of software in the wild convinced that it is: https://bugs.gentoo.org/show_bug.cgi?id=nonbash

Trading off safety and performance in the kernel

Posted May 13, 2015 12:48 UTC (Wed) by zuki (subscriber, #41808) [Link]

>> It's already racy.
> Racy in what sense?

I should have said "non-atomic". OP was wondering if a syscall option can be added to do configuration+execution in one step. I was just pointing out that the configuration step is already separate (e.g. for hibernation).