On Fri, Mar 07, 2014 at 05:00:43PM -0700, Theo de Raadt wrote:
> > Not necessarily, could be a tsleep in a DVACT_WAKEUP handler of an
> > other driver that makes us do the context switch. The cold = 2 diff
> > didn't reveal any tsleeps in inteldrm on my x220. But then I never
> > had any issues here either. Perhaps somebody who can reproduce the
> > problem should run the cold = 2 diff.
>
> Here is that "testing diff" that a few of us have looked at before,
> trying to find suspend/resume paths which sleep. It is a pretty
> wimpy method. If we hear that it finds some real bugs, then I can
> improve it (of course, we will fix the real bugs it finds seperately)
>
> Any feedback?
I have been running with a similar, equivalent diff in all my trees
for about a month and have not seen anything out of the ordinary.
I am also instrumenting pool_get and malloc to complain when cold=2
as well, when WAITOK is specified. Still nothing interesting seen
except for two occurrences in the hibernate suspending code which
appear to be harmless.
-ml
>
> Index: kern/kern_synch.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_synch.c,v
> retrieving revision 1.112
> diff -u -r1.112 kern_synch.c
> --- kern/kern_synch.c 24 Dec 2013 01:11:04 -0000 1.112
> +++ kern/kern_synch.c 21 Jan 2014 22:27:53 -0000
> @@ -109,6 +109,8 @@
>
> KASSERT((priority & ~(PRIMASK | PCATCH)) == 0);
>
> + if (cold == 2)
> + printf("TSLEEP");
> if (cold || panicstr) {
> int s;
> /*
> Index: arch/i386/i386/apm.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/apm.c,v
> retrieving revision 1.102
> diff -u -r1.102 apm.c
> --- arch/i386/i386/apm.c 6 Dec 2013 21:03:05 -0000 1.102
> +++ arch/i386/i386/apm.c 21 Jan 2014 22:35:45 -0000
> @@ -253,6 +253,7 @@
>
> s = splhigh();
> disable_intr();
> + cold = 2;
> config_suspend(TAILQ_FIRST(&alldevs), DVACT_SUSPEND);
>
> /* XXX
> @@ -274,6 +275,7 @@
> inittodr(time_second);
>
> config_suspend(TAILQ_FIRST(&alldevs), DVACT_RESUME);
> + cold = 0;
> enable_intr();
> splx(s);
>
> Index: arch/loongson/dev/apm.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/loongson/dev/apm.c,v
> retrieving revision 1.17
> diff -u -r1.17 apm.c
> --- arch/loongson/dev/apm.c 6 Dec 2013 21:03:05 -0000 1.17
> +++ arch/loongson/dev/apm.c 21 Jan 2014 22:34:53 -0000
> @@ -374,7 +374,7 @@
>
> s = splhigh();
> (void)disableintr();
> - cold = 1;
> + cold = 2;
>
> rv = config_suspend(TAILQ_FIRST(&alldevs), DVACT_SUSPEND);
>
> Index: arch/zaurus/dev/zaurus_apm.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/zaurus/dev/zaurus_apm.c,v
> retrieving revision 1.24
> diff -u -r1.24 zaurus_apm.c
> --- arch/zaurus/dev/zaurus_apm.c 6 Dec 2013 21:03:05 -0000 1.24
> +++ arch/zaurus/dev/zaurus_apm.c 21 Jan 2014 22:40:12 -0000
> @@ -653,6 +653,7 @@
> #endif /* NWSDISPLAY > 0 */
>
> s = splhigh();
> + cold = 2;
> config_suspend(TAILQ_FIRST(&alldevs), DVACT_SUSPEND);
>
> /* XXX
> @@ -683,6 +684,7 @@
>
> /* NOTREACHED */
> config_suspend(TAILQ_FIRST(&alldevs), DVACT_RESUME);
> + cold = 0;
> splx(s);
>
> bufq_restart();
> Index: dev/acpi/acpi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
> retrieving revision 1.252
> diff -u -r1.252 acpi.c
> --- dev/acpi/acpi.c 20 Jan 2014 00:47:21 -0000 1.252
> +++ dev/acpi/acpi.c 21 Jan 2014 22:09:38 -0000
> @@ -2138,7 +2138,7 @@
>
> s = splhigh();
> disable_intr(); /* PSL_I for resume; PIC/APIC broken until repair */
> - cold = 1; /* Force other code to delay() instead of tsleep() */
> + cold = 2; /* Force other code to delay() instead of tsleep() */
>
> if (config_suspend(TAILQ_FIRST(&alldevs), DVACT_SUSPEND) != 0)
> goto fail_suspend;
>
