On Thu, Oct 15, 2015 at 05:11:47PM +0000, Taylor R Campbell wrote:
>    Date: Wed, 14 Oct 2015 15:53:16 +0000
>    From: Taylor R Campbell <[email protected]>
> 
>    This is not gttwsi_wait's fault -- this is envsys's fault.  The
>    correct solution should be to change the API to avoid holding a mutex
>    across calls to a device sensor refresh routine.  Something like
>    kern/kern_rndsink.c, perhaps.
> 
> The code structure should be something like this:
> 
> struct sysmon_envsys {
>       ...
>       unsigned sme_refcnt;
>       unsigned sme_busy:1;
>       ...
> };
> 
> mutex_enter(&sme_global_mtx);
> LIST_FOREACH(sme, &sysmon_envsys_list, sme_list) {
>       sysmon_envsys_acquire(sme);     /* Bump refcnt.  */
>       mutex_exit(&sme_global_mtx);
>       ...
>       /* sme_update_dictionary */
>       TAILQ_FOREACH(sensor, &sme->sme_sensors_list, sensors_head) {
>               sysmon_envsys_busy(sme);        /* Wait for busy flag.  */
>               sysmon_envsys_refresh_sensor(sme, sensor);
>                                               /* Call sme->sme_refresh.  */
>               sysmon_envsys_unbusy(sme);      /* Clear busy flag.  */
>       }
>       ...
>       mutex_enter(&sme_global_mtx);
>       sysmon_envsys_release(sme);     /* Drop refcnt.  */
> }
> mutex_exit(&sme_global_mtx);
> 
> During the call to sysmon_envsys_refresh_sensor, the only `lock' that
> should be held is the busy flag.  The reference count ensures sme
> doesn't go away while sme_global_mtx is dropped; I assume here the
> sensors list is stable during the lifetime of sme.
> 
> Both sysmon_envysys_acquire/release and sysmon_envsys_busy/unbusy
> would hold sme->sme_mtx only internally.  sme_events_check need not
> busy the sensor, so there is no chance it may attempt to wait for I/O.

I looked a bit, and it's not that easy.
sysmon_envsys_refresh_sensor() is called from various places with sme_mtx
held; sometimes the function calling sysmon_envsys_refresh_sensor() is
entered with the lock held. sme_mtx protects various states of sme, and
reworking this requires quite a bit of work (dropping it before calling
sysmon_envsys_refresh_sensor() and reaquiring after is not an option).
More work than I can put in this right now ...

-- 
Manuel Bouyer <[email protected]>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Reply via email to