Commit 25832f71d1e7309dee2d8a81743ac49d85d96d18

Authored by Tomi Valkeinen
Committed by Jyri Sarha
1 parent f44cd572a6

drm/omap: fix race conditon in DMM

The omapdrm DMM code sometimes crashes with:

WARNING: CPU: 0 PID: 1235 at lib/list_debug.c:36 __list_add+0x8c/0xbc()
list_add double add: new=e9265368, prev=e90139c4, next=e9265368.

This is caused by the code calling release_engine() twice for the same
engine.

dmm_txn_commit(wait=true) call is supposed to wait until the DMM
transaction has been finished. And it does that, but it does not wait
for the irq handler to finish.

What happens is that the irq handler is triggered, and it either wakes
up the thread that called dmm_txn_commit(), or that thread never even
slept because the transaction was finished in the HW very quickly. That
thread then continues executing, even if the irq handler is not yet
finished, and a new transaction may be initiated. If that transaction is
async (i.e. wait=false), a 'async' flag is set to true. The original irq
handler, which has yet not finished, then sees the transaction as
'async', even if it was supposed to be 'sync'.

When that happens, the irq handler does an extra release_engine() call
because it thinks it need to release the engine, leading to the crash.

This patch fixes the issue by using completion to ensure that the irq
handler has finished before a dmm_txn_commit(wait=true) may continue.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>

Showing 2 changed files with 9 additions and 8 deletions Side-by-side Diff

drivers/gpu/drm/omapdrm/omap_dmm_priv.h
... ... @@ -148,7 +148,7 @@
148 148  
149 149 bool async;
150 150  
151   - wait_queue_head_t wait_for_refill;
  151 + struct completion compl;
152 152  
153 153 struct list_head idle_node;
154 154 };
drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
... ... @@ -29,6 +29,7 @@
29 29 #include <linux/mm.h>
30 30 #include <linux/time.h>
31 31 #include <linux/list.h>
  32 +#include <linux/completion.h>
32 33  
33 34 #include "omap_dmm_tiler.h"
34 35 #include "omap_dmm_priv.h"
35 36  
... ... @@ -146,10 +147,10 @@
146 147  
147 148 for (i = 0; i < dmm->num_engines; i++) {
148 149 if (status & DMM_IRQSTAT_LST) {
149   - wake_up_interruptible(&dmm->engines[i].wait_for_refill);
150   -
151 150 if (dmm->engines[i].async)
152 151 release_engine(&dmm->engines[i]);
  152 +
  153 + complete(&dmm->engines[i].compl);
153 154 }
154 155  
155 156 status >>= 8;
... ... @@ -273,7 +274,8 @@
273 274  
274 275 /* mark whether it is async to denote list management in IRQ handler */
275 276 engine->async = wait ? false : true;
276   - /* verify that the irq handler sees the 'async' value */
  277 + reinit_completion(&engine->compl);
  278 + /* verify that the irq handler sees the 'async' and completion value */
277 279 smp_mb();
278 280  
279 281 /* kick reload */
... ... @@ -281,9 +283,8 @@
281 283 dmm->base + reg[PAT_DESCR][engine->id]);
282 284  
283 285 if (wait) {
284   - if (wait_event_interruptible_timeout(engine->wait_for_refill,
285   - wait_status(engine, DMM_PATSTATUS_READY) == 0,
286   - msecs_to_jiffies(1)) <= 0) {
  286 + if (!wait_for_completion_timeout(&engine->compl,
  287 + msecs_to_jiffies(1))) {
287 288 dev_err(dmm->dev, "timed out waiting for done\n");
288 289 ret = -ETIMEDOUT;
289 290 }
... ... @@ -719,7 +720,7 @@
719 720 (REFILL_BUFFER_SIZE * i);
720 721 omap_dmm->engines[i].refill_pa = omap_dmm->refill_pa +
721 722 (REFILL_BUFFER_SIZE * i);
722   - init_waitqueue_head(&omap_dmm->engines[i].wait_for_refill);
  723 + init_completion(&omap_dmm->engines[i].compl);
723 724  
724 725 list_add(&omap_dmm->engines[i].idle_node, &omap_dmm->idle_head);
725 726 }