diff options
| author | Corey Minyard <cminyard@mvista.com> | 2015-02-19 09:25:49 -0500 |
|---|---|---|
| committer | Corey Minyard <cminyard@mvista.com> | 2015-02-19 21:58:42 -0500 |
| commit | 1d86e29b4a612eb01c39daa48749ab7964e77e03 (patch) | |
| tree | 8fc87a38ad0b05994db7eefcb58ea4ac085deb25 | |
| parent | d6c5dc18d863338528f4e89e8dba9449c6e30f4e (diff) | |
ipmi: Fix a memory ordering issue
From a locking point of view it is safe to check waiting_msg without
a lock, but there is a memory ordering issue that causes it to
possibly not be set right when viewed from another processor. We are
already claiming a lock right after that, move the check to inside
the lock to enforce the memory ordering.
Signed-off-by: Corey Minyard <cminyard@mvista.com>
| -rw-r--r-- | drivers/char/ipmi/ipmi_si_intf.c | 14 |
1 files changed, 10 insertions, 4 deletions
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 321ecb26df6a..f6646ed3047e 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c | |||
| @@ -932,9 +932,6 @@ static void sender(void *send_info, | |||
| 932 | enum si_sm_result result; | 932 | enum si_sm_result result; |
| 933 | unsigned long flags; | 933 | unsigned long flags; |
| 934 | 934 | ||
| 935 | BUG_ON(smi_info->waiting_msg); | ||
| 936 | smi_info->waiting_msg = msg; | ||
| 937 | |||
| 938 | debug_timestamp("Enqueue"); | 935 | debug_timestamp("Enqueue"); |
| 939 | 936 | ||
| 940 | if (smi_info->run_to_completion) { | 937 | if (smi_info->run_to_completion) { |
| @@ -942,7 +939,7 @@ static void sender(void *send_info, | |||
| 942 | * If we are running to completion, start it and run | 939 | * If we are running to completion, start it and run |
| 943 | * transactions until everything is clear. | 940 | * transactions until everything is clear. |
| 944 | */ | 941 | */ |
| 945 | smi_info->curr_msg = smi_info->waiting_msg; | 942 | smi_info->curr_msg = msg; |
| 946 | smi_info->waiting_msg = NULL; | 943 | smi_info->waiting_msg = NULL; |
| 947 | 944 | ||
| 948 | /* | 945 | /* |
| @@ -960,6 +957,15 @@ static void sender(void *send_info, | |||
| 960 | } | 957 | } |
| 961 | 958 | ||
| 962 | spin_lock_irqsave(&smi_info->si_lock, flags); | 959 | spin_lock_irqsave(&smi_info->si_lock, flags); |
| 960 | /* | ||
| 961 | * The following two lines don't need to be under the lock for | ||
| 962 | * the lock's sake, but they do need SMP memory barriers to | ||
| 963 | * avoid getting things out of order. We are already claiming | ||
| 964 | * the lock, anyway, so just do it under the lock to avoid the | ||
| 965 | * ordering problem. | ||
| 966 | */ | ||
| 967 | BUG_ON(smi_info->waiting_msg); | ||
| 968 | smi_info->waiting_msg = msg; | ||
| 963 | check_start_timer_thread(smi_info); | 969 | check_start_timer_thread(smi_info); |
| 964 | spin_unlock_irqrestore(&smi_info->si_lock, flags); | 970 | spin_unlock_irqrestore(&smi_info->si_lock, flags); |
| 965 | } | 971 | } |
