suspending threads before devices

Andriy Gapon avg at FreeBSD.org
Fri Nov 21 13:02:29 UTC 2014


On 20/11/2014 16:28, Konstantin Belousov wrote:
> Below is the prototyped patch for global process suspension. There is
> debugging sysctl debug.total_stop, which demonstrates the KPI, also I
> used it for light testing. It successfully survived suspend/resume of
> usermode threads in multiuser with mt processes running.

I applied this patch and hooked the code to ACPI suspend and I must that this
greatly improved chances of suspend + resume working even when initiated during
heavy graphic activity on my machine with radeon hardware.  So far it hasn't
failed a single time through a dozen tests.  It even worked
kern.vt.suspendswitch=0 which never worked for me before.

In addition to your changes I also have have https://reviews.freebsd.org/D1004
for initiating VT switch at a more appropriate moment when kern.vt.suspendswitch=1.
But that was not enough, I also had to split power_suspend into power_suspend
and power_suspend_early.  power_suspend_early is called before the userland is
frozen, so that things like aforementioned VT switching could be done.
power_suspend is called after the userland is frozen and it is more appropriate
for things like powering down disks[*].

The code that I am running is here:
https://github.com/avg-I/freebsd/compare/wip/kib-userland-freeze

Speaking about the code I probably would like the names to be polished up a
little bit.
E.g. maybe proc_stop_total -> proc_stop_all or even stop_all_procs.  'Total' is
slightly confusing in this context.
SINGLE_TOTAL sounds very confusing and non-descriptive to me, but I can not
think up a better name at the moment.
Also, maybe met_stopped -> seen_stopped.  Likewise, did_stop -> stopped_some?
Finally, some calls to thread_unsuspend_one() look quite ugly because of line
wrapping.  Maybe thread_single() code could be restructured / reformatted a
little bit to avoid that.

Finally, it seems that the code assumes that the calling process (curproc when
proc_stop_total is called) is single-threaded.  I think that that is a fine
assumption, but maybe it needs to be spelled out explicitly and asserted.

I should add that the code logic is good, so I only have 'color of bikeshed'
comments on its style.
So, I like your change a lot!  Thanks!

[*] And that needs to be done via an event handler only because of the lack of
proper integration between the bus code and CAM.

-- 
Andriy Gapon


More information about the freebsd-arch mailing list