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 /drivers/char/ipmi | |
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>
Diffstat (limited to 'drivers/char/ipmi')
-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 | } |