msm: smsm: Fix race condition in SMSM notification
The SMSM snapshot logic takes a snapshot of the current SMSM state,
places it into a FIFO, increments a wakelock count, and then signals a
worker to process the state changes. The worker processes the state
changes and then decrements the wakelock count.
In the previous case, the code would do the following:
Interrupt handler:
1) insert data into FIFO
2) increment wakelock count
Worker:
a) verifies >= 1 snapshots are in FIFO
b) processes snapshot
c) decrements wakelock reference count
If the interrupt handler is interrupted between steps 1 and 2, then the
function flow would be 1abc2. In this case, step c would fail because
the wakelock count would be zero and the wakelock would remain at 1 and
locked even though the FIFO would be empty. This results in the
wakelock not getting released.
To resolve this, the order of 2 (now 1') and 1 (now 2') was swapped
which always ensures 1' will always occur before abc which results in
the proper ordering 1'2'abc.
CRS-Fixed: 349099
Change-Id: I388b4c2f76838f59337e94f83b65fa06e81bb875
Signed-off-by: Eric Holmberg <eholmber@codeaurora.org>
diff --git a/arch/arm/mach-msm/smd.c b/arch/arm/mach-msm/smd.c
index bbfe702..3cf0550 100644
--- a/arch/arm/mach-msm/smd.c
+++ b/arch/arm/mach-msm/smd.c
@@ -2365,26 +2365,19 @@
return;
}
- /* queue state entries */
- for (n = 0; n < SMSM_NUM_ENTRIES; n++) {
- new_state = __raw_readl(SMSM_STATE_ADDR(n));
-
- ret = kfifo_in(&smsm_snapshot_fifo,
- &new_state, sizeof(new_state));
- if (ret != sizeof(new_state)) {
- pr_err("%s: SMSM snapshot failure %d\n", __func__, ret);
- return;
- }
- }
-
- /* queue wakelock usage flag */
- ret = kfifo_in(&smsm_snapshot_fifo,
- &use_wakelock, sizeof(use_wakelock));
- if (ret != sizeof(use_wakelock)) {
- pr_err("%s: SMSM snapshot failure %d\n", __func__, ret);
- return;
- }
-
+ /*
+ * To avoid a race condition with notify_smsm_cb_clients_worker, the
+ * following sequence must be followed:
+ * 1) increment snapshot count
+ * 2) insert data into FIFO
+ *
+ * Potentially in parallel, the worker:
+ * a) verifies >= 1 snapshots are in FIFO
+ * b) processes snapshot
+ * c) decrements reference count
+ *
+ * This order ensures that 1 will always occur before abc.
+ */
if (use_wakelock) {
spin_lock_irqsave(&smsm_snapshot_count_lock, flags);
if (smsm_snapshot_count == 0) {
@@ -2394,7 +2387,44 @@
++smsm_snapshot_count;
spin_unlock_irqrestore(&smsm_snapshot_count_lock, flags);
}
+
+ /* queue state entries */
+ for (n = 0; n < SMSM_NUM_ENTRIES; n++) {
+ new_state = __raw_readl(SMSM_STATE_ADDR(n));
+
+ ret = kfifo_in(&smsm_snapshot_fifo,
+ &new_state, sizeof(new_state));
+ if (ret != sizeof(new_state)) {
+ pr_err("%s: SMSM snapshot failure %d\n", __func__, ret);
+ goto restore_snapshot_count;
+ }
+ }
+
+ /* queue wakelock usage flag */
+ ret = kfifo_in(&smsm_snapshot_fifo,
+ &use_wakelock, sizeof(use_wakelock));
+ if (ret != sizeof(use_wakelock)) {
+ pr_err("%s: SMSM snapshot failure %d\n", __func__, ret);
+ goto restore_snapshot_count;
+ }
+
schedule_work(&smsm_cb_work);
+ return;
+
+restore_snapshot_count:
+ if (use_wakelock) {
+ spin_lock_irqsave(&smsm_snapshot_count_lock, flags);
+ if (smsm_snapshot_count) {
+ --smsm_snapshot_count;
+ if (smsm_snapshot_count == 0) {
+ SMx_POWER_INFO("SMSM snapshot wake unlock\n");
+ wake_unlock(&smsm_snapshot_wakelock);
+ }
+ } else {
+ pr_err("%s: invalid snapshot count\n", __func__);
+ }
+ spin_unlock_irqrestore(&smsm_snapshot_count_lock, flags);
+ }
}
static irqreturn_t smsm_irq_handler(int irq, void *data)