On 07/25/2013 07:16 PM, Zheng, Lv wrote: >> >> If I understand this correctly, the problem would be if: >> >> rem_time = wait_for_completion_timeout(&tx_msg->tx_complete, >> IPMI_TIMEOUT); >> >> returns on a timeout, then checks msg_done and races with something setting >> msg_done. If that is the case, you would need the smp_rmb() before checking >> msg_done. >> >> However, the timeout above is unnecessary. You are using >> ipmi_request_settime(), so you can set the timeout when the IPMI command >> fails and returns a failure message. The driver guarantees a return message >> for each request. Just remove the timeout from the completion, set the >> timeout and retries in the ipmi request, and the completion should handle the >> barrier issues. > It's just difficult for me to determine retry count and timeout value, maybe > retry=0, timeout=IPMI_TIMEOUT is OK. > The code of the timeout completion is already there, I think the quick fix > code should not introduce this logic. > I'll add a new patch to apply your comment.
Since it is a local BMC, I doubt a retry is required. That is probably fine. Or you could set retry=1 and timeout=IPMI_TIMEOUT/2 if you wanted to be more sure, but I doubt it would make a difference. The only time you really need to worry about retries is if you are resetting the BMC or it is being overloaded. > >> Plus, from a quick glance at the code, it doesn't look like it will properly >> handle a >> situation where the timeout occurs and is handled then the response comes in >> later. > PATCH 07 fixed this issue. > Here we just need the smp_rmb() or holding tx_msg_lock() around the > acpi_format_ipmi_response(). If you apply the fix like I suggest, then the race goes away. If there's no timeout and it just waits for the completion, things get a lot simpler. > > Thanks for commenting. No problem, thanks for working on this. -corey ------------------------------------------------------------------------------ See everything from the browser to the database with AppDynamics Get end-to-end visibility with application monitoring from AppDynamics Isolate bottlenecks and diagnose root cause in seconds. Start your free trial of AppDynamics Pro today! http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk _______________________________________________ Openipmi-developer mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openipmi-developer
