On Tue, Mar 26, 2019 at 06:19:00AM -0700, Jason Thorpe wrote: > > On Mar 26, 2019, at 3:44 AM, Christoph Badura <b...@bsd.de> 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