Source-Changes-HG archive

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

[src/netbsd-8]: src/sys/dev/sysmon Pull up following revision(s) (requested b...



details:   https://anonhg.NetBSD.org/src/rev/c02cd1999441
branches:  netbsd-8
changeset: 851031:c02cd1999441
user:      snj <snj%NetBSD.org@localhost>
date:      Sat Sep 23 17:22:48 2017 +0000

description:
Pull up following revision(s) (requested by pgoyette in ticket #281):
        sys/dev/sysmon/sysmon_envsys.c: 1.140-1.141
        sys/dev/sysmon/sysmon_envsys_events.c: 1.120-1.121
        sys/dev/sysmon/sysmonvar.h: 1.50
Fixes a problem that some driver(e.g. acpitz(4) or coretemp(5)) which
use sysmon_envsys sleep waiting at "rndsrc" when "drvctl -d".
Don't call rnd_detach_source() in sme_remove_event() which is called
from sme_event_unregister_all(). Instead, call rnd_detach_source() in
sysmon_envsys_sensor_detach() and call sysmon_envsys_sensor_detach()
before sme_event_unregister_sensor(). Each sensor(envsys_data) has each
rnd_src, but some sme_events point to the same rnd_src in a sensor.
Calling rnd_detach_souce() twice with the same rnd_src brokes a reference
count in rnd_src. OK'd by pgoyette@.
--
Improve tracking of the state of an event's callout, and protect all
queries or modifications of the state with the sme_mtx mutex.
Detach the rndsrc before re-attaching it (with potentially new values).
Clean up some lock-ordering issues.
And a couple of KNF issues for good measure!
Should address PR kern/52533

diffstat:

 sys/dev/sysmon/sysmon_envsys.c        |  52 ++++++++++++++++++++++++++++------
 sys/dev/sysmon/sysmon_envsys_events.c |  51 ++++++++++++++++++---------------
 sys/dev/sysmon/sysmonvar.h            |   9 ++++-
 3 files changed, 78 insertions(+), 34 deletions(-)

diffs (truncated from 317 to 300 lines):

diff -r 488335041518 -r c02cd1999441 sys/dev/sysmon/sysmon_envsys.c
--- a/sys/dev/sysmon/sysmon_envsys.c    Sat Sep 23 17:20:13 2017 +0000
+++ b/sys/dev/sysmon/sysmon_envsys.c    Sat Sep 23 17:22:48 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: sysmon_envsys.c,v 1.139 2015/12/14 01:08:47 pgoyette Exp $     */
+/*     $NetBSD: sysmon_envsys.c,v 1.139.10.1 2017/09/23 17:22:48 snj Exp $     */
 
 /*-
  * Copyright (c) 2007, 2008 Juan Romero Pardines.
@@ -64,7 +64,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys.c,v 1.139 2015/12/14 01:08:47 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys.c,v 1.139.10.1 2017/09/23 17:22:48 snj Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -525,6 +525,8 @@
 {
        struct sysmon_envsys *sme;
 
+       CTASSERT(SME_CALLOUT_INVALID == 0);
+
        sme = kmem_zalloc(sizeof(*sme), KM_SLEEP);
        TAILQ_INIT(&sme->sme_sensors_list);
        LIST_INIT(&sme->sme_events_list);
@@ -650,9 +652,12 @@
        /*
         * remove it, unhook from rnd(4), and decrement the sensors count.
         */
+       if (oedata->flags & ENVSYS_FHAS_ENTROPY)
+               rnd_detach_source(&oedata->rnd_src);
        sme_event_unregister_sensor(sme, edata);
        if (LIST_EMPTY(&sme->sme_events_list)) {
-               sme_events_halt_callout(sme);
+               if (sme->sme_callout_state == SME_CALLOUT_READY)
+                       sme_events_halt_callout(sme);
                destroy = true;
        }
        TAILQ_REMOVE(&sme->sme_sensors_list, edata, sensors_head);
@@ -970,6 +975,7 @@
 {
        prop_array_t array;
        struct sysmon_envsys *osme;
+       envsys_data_t *edata;
 
        KASSERT(sme != NULL);
 
@@ -987,6 +993,10 @@
        LIST_REMOVE(sme, sme_list);
        mutex_exit(&sme_global_mtx);
 
+       TAILQ_FOREACH(edata, &sme->sme_sensors_list, sensors_head) {
+               sysmon_envsys_sensor_detach(sme, edata);
+       }
+
        /*
         * Unregister all events associated with device.
         */
@@ -1262,15 +1272,23 @@
                        }
 
                        /*
-                        * Finally, remove any old limits event, then
-                        * install a new event (which will update the
-                        * dictionary)
+                        * If the sensor is providing entropy data,
+                        * get rid of the rndsrc;  we'll provide a new
+                        * one shortly.
+                        */
+                       if (edata->flags & ENVSYS_FHAS_ENTROPY)
+                               rnd_detach_source(&edata->rnd_src);
+
+                       /*
+                        * Remove the old limits event, if any
                         */
                        sme_event_unregister(sme, edata->desc,
                            PENVSYS_EVENT_LIMITS);
 
                        /*
-                        * Find the correct units for this sensor.
+                        * Create and install a new event (which will
+                        * update the dictionary) with the correct
+                        * units.
                         */
                        sdt_units = sme_find_table_entry(SME_DESC_UNITS,
                            edata->units);
@@ -1283,6 +1301,11 @@
                                    &lims, props, PENVSYS_EVENT_LIMITS,
                                    sdt_units->crittype);
                        }
+
+                       /* Finally, if the sensor provides entropy,
+                        * create an additional event entry and attach
+                        * the rndsrc
+                        */
                        if (edata->flags & ENVSYS_FHAS_ENTROPY) {
                                sme_event_register(sdict, edata, sme,
                                    &lims, props, PENVSYS_EVENT_NULL,
@@ -1301,8 +1324,17 @@
                 * Restore default timeout value.
                 */
                sme->sme_events_timeout = SME_EVENTS_DEFTIMEOUT;
+
+               /*
+                * Note that we need to hold the sme_mtx while calling
+                * sme_schedule_callout().  Thus to avoid dropping,
+                * reacquiring, and dropping it again, we just tell
+                * sme_envsys_release() that the mutex is already owned.
+                */
+               mutex_enter(&sme->sme_mtx);
                sme_schedule_callout(sme);
-               sysmon_envsys_release(sme, false);
+               sysmon_envsys_release(sme, true);
+               mutex_exit(&sme->sme_mtx);
        }
        mutex_exit(&sme_global_mtx);
 }
@@ -1343,7 +1375,9 @@
         */
        if (sme->sme_events_timeout == 0) {
                sme->sme_events_timeout = SME_EVENTS_DEFTIMEOUT;
+               mutex_enter(&sme->sme_mtx);
                sme_schedule_callout(sme);
+               mutex_exit(&sme->sme_mtx);
        }
 
        if (!prop_dictionary_set_uint64(pdict, "refresh-timeout",
@@ -1817,7 +1851,7 @@
                                        sme_schedule_callout(sme);
                                }
                                mutex_exit(&sme->sme_mtx);
-               }
+                       }
                }
                return error;
 
diff -r 488335041518 -r c02cd1999441 sys/dev/sysmon/sysmon_envsys_events.c
--- a/sys/dev/sysmon/sysmon_envsys_events.c     Sat Sep 23 17:20:13 2017 +0000
+++ b/sys/dev/sysmon/sysmon_envsys_events.c     Sat Sep 23 17:22:48 2017 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: sysmon_envsys_events.c,v 1.119 2017/06/01 02:45:11 chs Exp $ */
+/* $NetBSD: sysmon_envsys_events.c,v 1.119.2.1 2017/09/23 17:22:48 snj Exp $ */
 
 /*-
  * Copyright (c) 2007, 2008 Juan Romero Pardines.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys_events.c,v 1.119 2017/06/01 02:45:11 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sysmon_envsys_events.c,v 1.119.2.1 2017/09/23 17:22:48 snj Exp $");
 
 #include <sys/param.h>
 #include <sys/types.h>
@@ -313,7 +313,7 @@
        /*
         * Initialize the events framework if it wasn't initialized before.
         */
-       if ((sme->sme_flags & SME_CALLOUT_INITIALIZED) == 0)
+       if (sme->sme_callout_state == SME_CALLOUT_INVALID)
                error = sme_events_init(sme);
 
        /*
@@ -374,7 +374,7 @@
        }
 
        if (LIST_EMPTY(&sme->sme_events_list) &&
-           sme->sme_flags & SME_CALLOUT_INITIALIZED) {
+           sme->sme_callout_state == SME_CALLOUT_READY) {
                sme_events_halt_callout(sme);
                destroy = true;
        }
@@ -480,8 +480,6 @@
 
        KASSERT(mutex_owned(&sme->sme_mtx));
 
-       if (see->see_edata->flags & ENVSYS_FHAS_ENTROPY)
-               rnd_detach_source(&see->see_edata->rnd_src);
        LIST_REMOVE(see, see_list);
        kmem_free(see, sizeof(*see));
 }
@@ -572,7 +570,7 @@
 
        callout_init(&sme->sme_callout, CALLOUT_MPSAFE);
        callout_setfunc(&sme->sme_callout, sme_events_check, sme);
-       sme->sme_flags |= SME_CALLOUT_INITIALIZED;
+       sme->sme_callout_state = SME_CALLOUT_READY;
        sme_schedule_callout(sme);
        DPRINTF(("%s: events framework initialized for '%s'\n",
            __func__, sme->sme_name));
@@ -591,8 +589,9 @@
        uint64_t timo;
 
        KASSERT(sme != NULL);
+       KASSERT(mutex_owned(&sme->sme_mtx));
 
-       if ((sme->sme_flags & SME_CALLOUT_INITIALIZED) == 0)
+       if (sme->sme_callout_state != SME_CALLOUT_READY)
                return;
 
        if (sme->sme_events_timeout)
@@ -612,14 +611,18 @@
 void
 sme_events_halt_callout(struct sysmon_envsys *sme)
 {
+
        KASSERT(mutex_owned(&sme->sme_mtx));
+       KASSERT(sme->sme_callout_state == SME_CALLOUT_READY);
 
        /*
-        * Unset before callout_halt to ensure callout is not scheduled again
-        * during callout_halt.
+        * Set HALTED before callout_halt to ensure callout is not
+        * scheduled again during callout_halt.  (callout_halt()
+        * can potentially release the mutex, so an active callout
+        * could reschedule itself if it grabs the mutex.)
         */
-       sme->sme_flags &= ~SME_CALLOUT_INITIALIZED;
 
+       sme->sme_callout_state = SME_CALLOUT_HALTED;
        callout_halt(&sme->sme_callout, &sme->sme_mtx);
 }
 
@@ -632,11 +635,15 @@
 void
 sme_events_destroy(struct sysmon_envsys *sme)
 {
+
        KASSERT(!mutex_owned(&sme->sme_mtx));
-       KASSERT((sme->sme_flags & SME_CALLOUT_INITIALIZED) == 0);
 
-       callout_destroy(&sme->sme_callout);
-       workqueue_destroy(sme->sme_wq);
+       if (sme->sme_callout_state == SME_CALLOUT_HALTED) {
+               callout_destroy(&sme->sme_callout);
+               sme->sme_callout_state = SME_CALLOUT_INVALID;
+               workqueue_destroy(sme->sme_wq);
+       }
+       KASSERT(sme->sme_callout_state == SME_CALLOUT_INVALID);
 
        DPRINTF(("%s: events framework destroyed for '%s'\n",
            __func__, sme->sme_name));
@@ -736,13 +743,7 @@
                mutex_exit(&sme->sme_work_mtx);
                return;
        }
-       if (!mutex_tryenter(&sme->sme_mtx)) {
-               /* can't get lock - try again later */
-               if (!sysmon_low_power)
-                       sme_schedule_callout(sme);
-               mutex_exit(&sme->sme_work_mtx);
-               return;
-       }
+       mutex_enter(&sme->sme_mtx);
        LIST_FOREACH(see, &sme->sme_events_list, see_list) {
                workqueue_enqueue(sme->sme_wq, &see->see_wk, NULL);
                see->see_edata->flags |= ENVSYS_FNEED_REFRESH;
@@ -750,8 +751,8 @@
        }
        if (!sysmon_low_power)
                sme_schedule_callout(sme);
+       mutex_exit(&sme->sme_mtx);
        mutex_exit(&sme->sme_work_mtx);
-       mutex_exit(&sme->sme_mtx);
 }
 
 /*
@@ -1005,12 +1006,16 @@
                 */
                if (!sysmon_low_power && sme_event_check_low_power()) {
                        struct penvsys_state pes;
+                       struct sysmon_envsys *sme = see->see_sme;
 
                        /*
                         * Stop the callout and send the 'low-power' event.
                         */
                        sysmon_low_power = true;
-                       callout_stop(&see->see_sme->sme_callout);
+                       KASSERT(mutex_owned(&sme->sme_mtx));
+                       KASSERT(sme->sme_callout_state == SME_CALLOUT_READY);
+                       callout_stop(&sme->sme_callout);
+                       sme->sme_callout_state = SME_CALLOUT_HALTED;
                        pes.pes_type = PENVSYS_TYPE_BATTERY;
                        sysmon_penvsys_event(&pes, PENVSYS_EVENT_LOW_POWER);
                }
diff -r 488335041518 -r c02cd1999441 sys/dev/sysmon/sysmonvar.h
--- a/sys/dev/sysmon/sysmonvar.h        Sat Sep 23 17:20:13 2017 +0000
+++ b/sys/dev/sysmon/sysmonvar.h        Sat Sep 23 17:22:48 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: sysmonvar.h,v 1.49 2015/04/23 23:22:03 pgoyette Exp $  */
+/*     $NetBSD: sysmonvar.h,v 1.49.10.1 2017/09/23 17:22:48 snj Exp $  */
 
 /*-
  * Copyright (c) 2000 Zembu Labs, Inc.
@@ -163,7 +163,6 @@
        int sme_flags;                  /* additional flags */
 #define SME_FLAG_BUSY          0x00000001      /* device busy */
 #define SME_DISABLE_REFRESH    0x00000002      /* disable sme_refresh */



Home | Main Index | Thread Index | Old Index