Comments on the patch inline. I have no idea why those changes would make anything work. The first change might result in a memory leak, but that's it. The second change is clearly wrong and will almost always result in a double unlock.
You are sure that you apply this patch and it starts working correctly, then remove it and it stops? I can't understand why those changes would make a difference. -corey Jonathan Fournier wrote: > Hi, > > I have an "issue" I'm able to fix by backtracking a change in OpenIPMI > but I don't understand why it fixes the thing, if possible I would > like to know about that change. > > I'm using an older revision of OpenIPMI (1.4.26) and openhpi/libipmi > plugin (2.2.1). > > When I generate sensor threshold events I don't see the events in > hpigetevents or hpi_cmd/event enable. But the events are actually > displayed by runing hpiel. I spent some time trying to figure out > where the issue was (openhpi, OpenIPMI, the IPMI driver etc) and > managed to find a change that introduced that behavior (I tried a lot > of hpi/ipmi packages combos from older to latest ones). The change > that introduced that (see the patch to backtrack it) was the commit on > 2006-02-28 09:50 > > 2006-02-28 09:50 cminyard > > * ChangeLog (1.527.2.134), lib/chassis.c (1.20.2.2), lib/mc.c > (1.88.2.22), ChangeLog (1.756), lib/chassis.c (1.22), lib/mc.c > (1.115): More minor fixes. > > 2006-02-28 03:54 cminyard > > * lib/: mc.c (1.114), mc.c (1.88.2.21): Missed something in the > last change. > > Any comments on that change? It's still present in 2.0.13. I would > like to know if my IPMC could be doing something wrong? The > combination of the spinlock move and the call to > ipmi_mc_remove_active_handler() introduced the error in my case. By > backtracking a subset of the changes as it was on 2006-02-28 03:54 > with that patch the problem goes away. > > Regards, > > /jonathan > > --- CUT HERE --- > > Signed-off-by: Jonathan Fournier <[EMAIL PROTECTED]> > > --- a/lib/chassis.c > +++ b/lib/chassis.c > @@ -362,13 +362,10 @@ chassis_mc_control_active_handler(ipmi_m > ipmi_domain_t *domain = ipmi_mc_get_domain(mc); > > if (active) > return; > > - ipmi_mc_remove_active_handler(mc, > - chassis_mc_control_active_handler, > - control); > This doesn't really hurt anything, I guess, though it may result in a memory leak. I think it's just there for consistency. > _ipmi_domain_entity_lock(domain); > _ipmi_entity_get(entity); > _ipmi_domain_entity_unlock(domain); > ipmi_control_destroy(control); > _ipmi_entity_put(entity); > --- a/lib/mc.c > +++ b/lib/mc.c > @@ -1382,12 +1382,12 @@ mc_reread_sel_timeout(void *cb_data, os_ > lock, so just don't start the timer and everything should > be happy. */ > DEBUG_INFO(info); > info->processing = 0; > info->timer_running = 0; > - ipmi_unlock(info->lock); > } > + ipmi_unlock(info->lock); > } > This is very bizarre. Read the code, this is clearly wrong. The callback unlocks the lock, you only unlock it here if the callback fails. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
