> From: Corey Minyard [mailto:tcminyard@xxxxxxxxx] > Sent: Friday, July 26, 2013 8:48 AM > > 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. I think for ACPI IPMI operation region, retries can be implemented in the ASL codes by the BIOS. I'll check if retry=0 is correct. > > > > >> 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. Exactly. I'll try to apply this in this patch, then the PATCH 07 is also need to be re-worked. Thanks and best regards -Lv > > > > Thanks for commenting. > > No problem, thanks for working on this. > > -corey ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
