tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: scheduler support to lock user processes out?



So I've done some research what others are doing here:
-FreeBSD acquires the big kernel lock before suspending
 devices, holding it all the time until after resume.
-Linux "freezes" the user processes, controlled by special flags
 in the process structure which are appearently honoured
 somehow by the scheduler. Pretty complex...

dyoung%pobox.com@localhost said:
> My point was that a driver has a bug if it reads/writes registers from
> a hardware device that has been powered down, regardless of how/why it
> was suspended. 

To suspend any device anytime shouldn't be a purpose in itself.

> I don't think that fixed policies such as, "the system may cancel
> user-requested suspension" or "users may not suspend drivers that are
> in-use" belong in the kernel.

That's not policy, that's a technical requirement imho.

> We already do suspend devices that are in-use.
> [...]

Yes, but it has shown that it is necessary to keep other
processes out. Which is pretty well done by switching
secondary CPUs offline, except some corner cases, as I wrote
in my original mail.
This is fine in a global system suspend situation, but
not acceptable at runtime.

I've just tried the kernel_lock way as FreeBSD does, instead
of shutting down secondary CPUs early. It works equally well
so far. In combination with a patch which makes the X server
(which accesses hardware directly, not caring about kernel
locks) detach early, system suspend/resume is rock-solid for me
(even with accelerated X running).
See the appended patch. (not completely clean because wsdisplay
is not mi)

best regards
Matthias






-------------------------------------------------------------------
-------------------------------------------------------------------
Forschungszentrum Juelich GmbH
52425 Juelich

Sitz der Gesellschaft: Juelich
Eingetragen im Handelsregister des Amtsgerichts Dueren Nr. HR B 3498
Vorsitzende des Aufsichtsrats: MinDir'in Baerbel Brumme-Bothe
Geschaeftsfuehrung: Prof. Dr. Achim Bachem (Vorsitzender),
Dr. Ulrich Krafft (stellv. Vorsitzender), Prof. Dr. Harald Bolt,
Dr. Sebastian M. Schmidt
-------------------------------------------------------------------
-------------------------------------------------------------------
#
# old_revision [fcb386a5056230b65c41d5757cc3ea3a18ea9fa0]
#
# patch "sys/dev/wscons/wsdisplay.c"
#  from [f76bf36eae09e18d2447eb3a07890d80488ab250]
#    to [20cbdd9e867e55493eb38c7c845356007d0ff5e4]
# 
# patch "sys/dev/wscons/wsdisplayvar.h"
#  from [b62217cd78f193d083cf920069b3b48992746bee]
#    to [78d1f778892de8b2de1905174929aa9eeeec15c5]
# 
# patch "sys/kern/kern_pmf.c"
#  from [83f81074adc57266851705a948ac78bd131ef807]
#    to [7015f79d8b6f2c52b52871474dcb17b9507d8503]
#
============================================================
--- sys/dev/wscons/wsdisplay.c  f76bf36eae09e18d2447eb3a07890d80488ab250
+++ sys/dev/wscons/wsdisplay.c  20cbdd9e867e55493eb38c7c845356007d0ff5e4
@@ -131,6 +131,8 @@ struct wsdisplay_softc {
 
        int sc_flags;
 #define SC_SWITCHPENDING 1
+#define SC_SWITCHERROR 2
+#define SC_XATTACHED 4 /* X server active */
        kmutex_t sc_flagsmtx; /* for flags, might also be used for focus */
        kcondvar_t sc_flagscv;
 
@@ -160,7 +162,6 @@ static void wsdisplay_noemul_attach(devi
 static void wsdisplay_emul_attach(device_t, device_t, void *);
 static int wsdisplay_noemul_match(device_t, struct cfdata *, void *);
 static void wsdisplay_noemul_attach(device_t, device_t, void *);
-static bool wsdisplay_resume(device_t dv);
 static bool wsdisplay_suspend(device_t dv);
 
 CFATTACH_DECL_NEW(wsdisplay_emul, sizeof (struct wsdisplay_softc),
@@ -609,63 +610,93 @@ wsdisplay_swdone_cb(void *arg, int error
 
        mutex_enter(&sc->sc_flagsmtx);
        KASSERT(sc->sc_flags & SC_SWITCHPENDING);
+       if (error)
+               sc->sc_flags |= SC_SWITCHERROR;
        sc->sc_flags &= ~SC_SWITCHPENDING;
        cv_signal(&sc->sc_flagscv);
        mutex_exit(&sc->sc_flagsmtx);
 }
 
-static bool
-wsdisplay_suspend(device_t dv)
+static int
+wsdisplay_dosync(struct wsdisplay_softc *sc, int attach)
 {
-       struct wsdisplay_softc *sc = device_private(dv);
        struct wsscreen *scr;
+       int (*op)(void *, int, void (*)(void *, int, int), void *);
        int res;
 
        scr = sc->sc_focus;
        if (!scr || !scr->scr_syncops)
-               return true;
+               return 0; /* XXX check SCR_GRAPHICS? */
 
        sc->sc_flags |= SC_SWITCHPENDING;
-       res = (*scr->scr_syncops->detach)(scr->scr_synccookie, 1,
-                                         wsdisplay_swdone_cb, sc);
-       if (res != EAGAIN) {
+       sc->sc_flags &= ~SC_SWITCHERROR;
+       if (attach)
+               op = scr->scr_syncops->attach;
+       else
+               op = scr->scr_syncops->detach;
+       res = (*op)(scr->scr_synccookie, 1, wsdisplay_swdone_cb, sc);
+       if (res == EAGAIN) {
+               /* wait for callback */
+               mutex_enter(&sc->sc_flagsmtx);
+               while (sc->sc_flags & SC_SWITCHPENDING)
+                       cv_wait_sig(&sc->sc_flagscv, &sc->sc_flagsmtx);
+               mutex_exit(&sc->sc_flagsmtx);
+               if (sc->sc_flags & SC_SWITCHERROR)
+                       return (EIO); /* XXX pass real error */
+       } else {
                sc->sc_flags &= ~SC_SWITCHPENDING;
-               return (res == 0);
+               if (res)
+                       return (res);
        }
+       if (attach)
+               sc->sc_flags |= SC_XATTACHED;
+       else
+               sc->sc_flags &= ~SC_XATTACHED;
+       return 0;
+}
 
-       /* wait for callback */
-       mutex_enter(&sc->sc_flagsmtx);
-       while (sc->sc_flags & SC_SWITCHPENDING)
-               cv_wait_sig(&sc->sc_flagscv, &sc->sc_flagsmtx);
-       mutex_exit(&sc->sc_flagsmtx);
-       return true;
+int
+wsdisplay_handlex(int resume)
+{
+       int i, res;
+       device_t dv;
+
+       for (i = 0; i < wsdisplay_cd.cd_ndevs; i++) {
+               dv = wsdisplay_cd.cd_devs[i];
+               if (!dv)
+                       continue;
+               res = wsdisplay_dosync(device_private(dv), resume);
+               if (res)
+                       return (res);
+       }
+       return (0);
 }
 
 static bool
-wsdisplay_resume(device_t dv)
+wsdisplay_suspend(device_t dv)
 {
        struct wsdisplay_softc *sc = device_private(dv);
-       struct wsscreen *scr;
-       int res;
-
-       scr = sc->sc_focus;
-       if (!scr || !scr->scr_syncops)
-               return true;
-
-       sc->sc_flags |= SC_SWITCHPENDING;
-       res = (*scr->scr_syncops->attach)(scr->scr_synccookie, 1,
-                                         wsdisplay_swdone_cb, sc);
-       if (res != EAGAIN) {
-               sc->sc_flags &= ~SC_SWITCHPENDING;
-               return (res == 0);
+#ifdef DIAGNOSTIC
+       struct wsscreen *scr = sc->sc_focus;
+       if (sc->sc_flags & SC_XATTACHED) {
+               KASSERT(scr && scr->scr_syncops);
        }
-
-       /* wait for callback */
-       mutex_enter(&sc->sc_flagsmtx);
-       while (sc->sc_flags & SC_SWITCHPENDING)
-               cv_wait_sig(&sc->sc_flagscv, &sc->sc_flagsmtx);
-       mutex_exit(&sc->sc_flagsmtx);
-       return true;
+#endif
+#if 1
+       /*
+        * XXX X servers should have been detached earlier.
+        * pmf currently ignores our return value and suspends the system
+        * after device suspend failures. We try to avoid bigger damage
+        * and try to detach the X server here. This is not safe because
+        * other parts of the system which the X server deals with
+        * might already be suspended.
+        */
+       if (sc->sc_flags & SC_XATTACHED) {
+               printf("%s: emergency X server detach\n", device_xname(dv));
+               wsdisplay_dosync(sc, 0);
+       }
+#endif
+       return (!(sc->sc_flags & SC_XATTACHED));
 }
 
 /* Print function (for parent devices). */
@@ -768,7 +799,7 @@ wsdisplay_common_attach(struct wsdisplay
        if (i > start)
                wsdisplay_addscreen_print(sc, start, i-start);
 
-       if (!pmf_device_register(sc->sc_dev, wsdisplay_suspend, 
wsdisplay_resume))
+       if (!pmf_device_register(sc->sc_dev, wsdisplay_suspend, NULL))
                aprint_error_dev(sc->sc_dev, "couldn't establish power 
handler\n");
 }
 
@@ -1723,6 +1754,9 @@ wsdisplay_switch3(device_t dv, int error
                return (wsdisplay_switch1(dv, 0, waitok));
        }
 
+       if (scr->scr_syncops && !error)
+               sc->sc_flags |= SC_XATTACHED;
+
        sc->sc_flags &= ~SC_SWITCHPENDING;
 
        if (!error && (scr->scr_flags & SCR_WAITACTIVE))
@@ -1819,6 +1853,7 @@ wsdisplay_switch1(device_t dv, int error
        if (no == WSDISPLAY_NULLSCREEN) {
                sc->sc_flags &= ~SC_SWITCHPENDING;
                if (!error) {
+                       sc->sc_flags &= ~SC_XATTACHED;
                        sc->sc_focus = 0;
                }
                wakeup(sc);
@@ -1837,6 +1872,8 @@ wsdisplay_switch1(device_t dv, int error
                return (error);
        }
 
+       sc->sc_flags &= ~SC_XATTACHED;
+
        error = (*sc->sc_accessops->show_screen)(sc->sc_accesscookie,
                                                 scr->scr_dconf->emulcookie,
                                                 waitok,
@@ -1891,6 +1928,8 @@ wsdisplay_switch(device_t dv, int no, in
                sc->sc_oldscreen = sc->sc_focusidx;
 
        if (scr->scr_syncops) {
+               if (!(sc->sc_flags & SC_XATTACHED)) /* nothing to do here */
+                       return (wsdisplay_switch1(dv, 0, waitok));
                res = (*scr->scr_syncops->detach)(scr->scr_synccookie, waitok,
          sc->sc_isconsole && wsdisplay_cons_pollmode ? 0 : 
wsdisplay_switch1_cb, dv);
                if (res == EAGAIN) {
@@ -1947,6 +1986,8 @@ wsscreen_attach_sync(struct wsscreen *sc
        }
        scr->scr_syncops = ops;
        scr->scr_synccookie = cookie;
+       if (scr == scr->sc->sc_focus)
+               scr->sc->sc_flags |= SC_XATTACHED;
        return (0);
 }
 
@@ -1956,6 +1997,8 @@ wsscreen_detach_sync(struct wsscreen *sc
        if (!scr->scr_syncops)
                return (EINVAL);
        scr->scr_syncops = 0;
+       if (scr == scr->sc->sc_focus)
+               scr->sc->sc_flags &= ~SC_XATTACHED;
        return (0);
 }
 
============================================================
--- sys/dev/wscons/wsdisplayvar.h       b62217cd78f193d083cf920069b3b48992746bee
+++ sys/dev/wscons/wsdisplayvar.h       78d1f778892de8b2de1905174929aa9eeeec15c5
@@ -174,6 +174,8 @@ int wsemuldisplaydevprint(void *, const 
 int    wsdisplaydevprint(void *, const char *);
 int    wsemuldisplaydevprint(void *, const char *);
 
+int    wsdisplay_handlex(int);
+
 /*
  * Console interface.
  */
============================================================
--- sys/kern/kern_pmf.c 83f81074adc57266851705a948ac78bd131ef807
+++ sys/kern/kern_pmf.c 7015f79d8b6f2c52b52871474dcb17b9507d8503
@@ -48,6 +48,12 @@ __KERNEL_RCSID(0, "$NetBSD: kern_pmf.c,v
 #include <sys/workqueue.h>
 #include <prop/proplib.h>
 
+/* XXX */
+#include "wsdisplay.h"
+#if NWSDISPLAY > 0
+#include <dev/wscons/wsdisplayvar.h>
+#endif
+
 #ifdef PMF_DEBUG
 int pmf_debug_event;
 int pmf_debug_idle;
@@ -214,6 +220,11 @@ pmf_system_resume(void)
        }
        aprint_debug(".\n");
 
+       KERNEL_UNLOCK_ONE(0);
+#if NWSDISPLAY > 0
+       if (rv)
+               wsdisplay_handlex(1);
+#endif
        return rv;
 }
 
@@ -225,6 +236,11 @@ pmf_system_suspend(void)
 
        if (!pmf_check_system_drivers())
                return false;
+#if NWSDISPLAY > 0
+       if (wsdisplay_handlex(0))
+               return false;
+#endif
+       KERNEL_LOCK(1, 0);
 
        /*
         * Flush buffers only if the shutdown didn't do so


Home | Main Index | Thread Index | Old Index