mei_cl_complete() seems to be missing a memory barrier which might cause
the waker to not notice the waiter and miss sending a wake_up as in the
following figure.
mei_cl_complete mei_cl_write
------------------------------------------------------------------------
mutex_unlock(&dev->device_lock);
...
if (waitqueue_active(&cl->tx_wait)) {
/* The CPU might reorder the test for
the waitqueue up here, before
prior writes complete */
/* wait_event_interruptible */
/* __wait_event_interruptible */
/* ___wait_event */
long __int = prepare_to_wait_event(
&wq, &__wait, state);
if (cl->writing_state ==
MEI_WRITE_COMPLETE)
cl->writing_state = MEI_WRITE_COMPLETE;
} else {
...
------------------------------------------------------------------------
There are three other place in drivers/misc/mei/ which have similar
code. The attached patch either adds the missing memory barrier, or
removes the call to waitqueue_active() leaving just wake_up() behind.
The latter fixes the problem because the call to spin_lock_irqsave() in
wake_up() will be an ACQUIRE operation.
I found this issue when I was looking through the linux source code
for places calling waitqueue_active() before wake_up*(), but without
preceding memory barriers, after sending a patch to fix a similar
issue in drivers/tty/n_tty.c (Details about the original issue can be
found here: https://lkml.org/lkml/2015/9/28/849).
Signed-off-by: Kosuke Tatsukawa <ta...@ab.jp.nec.com>
---
drivers/misc/mei/client.c | 10 ++++++++++
drivers/misc/mei/hw-me.c | 3 +--
drivers/misc/mei/hw-txe.c | 3 +--
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index a6c87c7..546aa2b 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -1711,6 +1711,11 @@ void mei_cl_complete(struct mei_cl *cl, struct mei_cl_cb
*cb)
case MEI_FOP_WRITE:
mei_io_cb_free(cb);
cl->writing_state = MEI_WRITE_COMPLETE;
+ /*
+ * Memory barrier is required here to make sure change to
+ * cl->writing_state is visible from other CPUs.
+ */
+ smp_mb();
if (waitqueue_active(&cl->tx_wait)) {
wake_up_interruptible(&cl->tx_wait);
} else {
@@ -1721,6 +1726,11 @@ void mei_cl_complete(struct mei_cl *cl, struct mei_cl_cb
*cb)
case MEI_FOP_READ:
list_add_tail(&cb->list, &cl->rd_completed);
+ /*
+ * Memory barrier is required here to make sure change to
+ * cl->rd_completed is visible from other CPUs.
+ */
+ smp_mb();
if (waitqueue_active(&cl->rx_wait))
wake_up_interruptible_all(&cl->rx_wait);
else
diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
index 65511d3..0bc45ef 100644
--- a/drivers/misc/mei/hw-me.c
+++ b/drivers/misc/mei/hw-me.c
@@ -943,8 +943,7 @@ static void mei_me_pg_legacy_intr(struct mei_device *dev)
dev->pg_event = MEI_PG_EVENT_INTR_RECEIVED;
hw->pg_state = MEI_PG_OFF;
- if (waitqueue_active(&dev->wait_pg))
- wake_up(&dev->wait_pg);
+ wake_up(&dev->wait_pg);
}
/**
diff --git a/drivers/misc/mei/hw-txe.c b/drivers/misc/mei/hw-txe.c
index bae680c..b9ce17d 100644
--- a/drivers/misc/mei/hw-txe.c
+++ b/drivers/misc/mei/hw-txe.c
@@ -1091,8 +1091,7 @@ irqreturn_t mei_txe_irq_thread_handler(int irq, void
*dev_id)
dev_dbg(dev->dev,
"Aliveness Interrupt: Status: %d\n", hw->aliveness);
dev->pg_event = MEI_PG_EVENT_RECEIVED;
- if (waitqueue_active(&hw->wait_aliveness_resp))
- wake_up(&hw->wait_aliveness_resp);
+ wake_up(&hw->wait_aliveness_resp);
}
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
