Re: panic in sysmon_envsys_unregister()

[Christoph Badura <bad%bsd.de@localhost>]

On Tue, Mar 26, 2019 at 06:19:00AM -0700, Jason Thorpe wrote:
> > On Mar 26, 2019, at 3:44 AM, Christoph Badura <bad%bsd.de@localhost> wrote:
> > This is the relevant code in sys/dev/sysmon/sysmon_envsys.c:sysmon_envsys_unregister()
> > 
> > 	TAILQ_FOREACH(edata, &sme->sme_sensors_list, sensors_head) {
> > 		sysmon_envsys_sensor_detach(sme, edata);
> > 	}
> > 
> > sysmon_envsys_sensor_detach() TAILQ_FOREACHs over sme_sensors_list itself
> > and removes edata.
> > 
> > Is using TAILQ_FOREACH_SAFE in sysmon_envsys_unregister() the right fix?
> 
> Either that or a TAILQ_FIRST() each time.

Sorry, I meant the question in a more general way.
sysmon_envsys_sensor_detach() iterates over the sensor_list with sme_mtx
held etc.  Therefore it looks fishy to me to iterate over the sensor_list
without sme_mtx held.

Maybe it would would be more correct to introduce
sysmon_envsys_sensor_detach_all() and factor out the common code.

Also it seems that sme_events_destroy() is only ever called if the
events_list happens to be empty.  And I can't see where the sme work
queue is destroy elsewhere.

And while looking for places that work queues get destroyed in
dev/sysmon/ it seems that swwdog's workqueue is created twice when
loaded as a module.

--chris