Re: envsys/gttwsi deadlock

[Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>]

   Date: Wed, 14 Oct 2015 15:53:16 +0000
   From: Taylor R Campbell <campbell+netbsd-tech-kern%mumble.net@localhost>

   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.