Commit 0c32b8ec authored by Marek Szyprowski's avatar Marek Szyprowski Committed by Mauro Carvalho Chehab

[media] s5p-mfc: Fix race between interrupt routine and device functions

Interrupt routine must wake process waiting for given interrupt AFTER
updating driver's internal structures and contexts. Doing it in-between
is a serious bug. This patch moves all calls to the wake() function to
the end of the interrupt processing block to avoid potential and real
races, especially on multi-core platforms. This also fixes following issue
reported from clock core (clocks were disabled in interrupt after being
unprepared from the other place in the driver, the stack trace however
points to the different place than s5p_mfc driver because of the race):

WARNING: CPU: 1 PID: 18 at drivers/clk/clk.c:544 clk_core_unprepare+0xc8/0x108
Modules linked in:
CPU: 1 PID: 18 Comm: kworker/1:0 Not tainted 4.10.0-next-20170223-00070-g04e18bc99ab9-dirty #2154
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
Workqueue: pm pm_runtime_work
[<c010d8b0>] (unwind_backtrace) from [<c010a534>] (show_stack+0x10/0x14)
[<c010a534>] (show_stack) from [<c033292c>] (dump_stack+0x74/0x94)
[<c033292c>] (dump_stack) from [<c011cef4>] (__warn+0xd4/0x100)
[<c011cef4>] (__warn) from [<c011cf40>] (warn_slowpath_null+0x20/0x28)
[<c011cf40>] (warn_slowpath_null) from [<c0387a84>] (clk_core_unprepare+0xc8/0x108)
[<c0387a84>] (clk_core_unprepare) from [<c0389d84>] (clk_unprepare+0x24/0x2c)
[<c0389d84>] (clk_unprepare) from [<c03d4660>] (exynos_sysmmu_suspend+0x48/0x60)
[<c03d4660>] (exynos_sysmmu_suspend) from [<c042b9b0>] (pm_generic_runtime_suspend+0x2c/0x38)
[<c042b9b0>] (pm_generic_runtime_suspend) from [<c0437580>] (genpd_runtime_suspend+0x94/0x220)
[<c0437580>] (genpd_runtime_suspend) from [<c042e240>] (__rpm_callback+0x134/0x208)
[<c042e240>] (__rpm_callback) from [<c042e334>] (rpm_callback+0x20/0x80)
[<c042e334>] (rpm_callback) from [<c042d3b8>] (rpm_suspend+0xdc/0x458)
[<c042d3b8>] (rpm_suspend) from [<c042ea24>] (pm_runtime_work+0x80/0x90)
[<c042ea24>] (pm_runtime_work) from [<c01322c4>] (process_one_work+0x120/0x318)
[<c01322c4>] (process_one_work) from [<c0132520>] (worker_thread+0x2c/0x4ac)
[<c0132520>] (worker_thread) from [<c0137ab0>] (kthread+0xfc/0x134)
[<c0137ab0>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c)
---[ end trace 1ead49a7bb83f0d8 ]---

Fixes: af935746 ("[media] MFC: Add MFC 5.1 V4L2 driver")
Signed-off-by: default avatarMarek Szyprowski <m.szyprowski@samsung.com>
CC: stable@vger.kernel.org # v4.5+
Reviewed-by: default avatarJavier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: default avatarSylwester Nawrocki <s.nawrocki@samsung.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@s-opensource.com>
parent 615cb49b
...@@ -666,9 +666,9 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv) ...@@ -666,9 +666,9 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)
break; break;
} }
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev); s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
wake_up_ctx(ctx, reason, err);
WARN_ON(test_and_clear_bit(0, &dev->hw_lock) == 0); WARN_ON(test_and_clear_bit(0, &dev->hw_lock) == 0);
s5p_mfc_clock_off(); s5p_mfc_clock_off();
wake_up_ctx(ctx, reason, err);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
} else { } else {
s5p_mfc_handle_frame(ctx, reason, err); s5p_mfc_handle_frame(ctx, reason, err);
...@@ -682,15 +682,11 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv) ...@@ -682,15 +682,11 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)
case S5P_MFC_R2H_CMD_OPEN_INSTANCE_RET: case S5P_MFC_R2H_CMD_OPEN_INSTANCE_RET:
ctx->inst_no = s5p_mfc_hw_call(dev->mfc_ops, get_inst_no, dev); ctx->inst_no = s5p_mfc_hw_call(dev->mfc_ops, get_inst_no, dev);
ctx->state = MFCINST_GOT_INST; ctx->state = MFCINST_GOT_INST;
clear_work_bit(ctx);
wake_up(&ctx->queue);
goto irq_cleanup_hw; goto irq_cleanup_hw;
case S5P_MFC_R2H_CMD_CLOSE_INSTANCE_RET: case S5P_MFC_R2H_CMD_CLOSE_INSTANCE_RET:
clear_work_bit(ctx);
ctx->inst_no = MFC_NO_INSTANCE_SET; ctx->inst_no = MFC_NO_INSTANCE_SET;
ctx->state = MFCINST_FREE; ctx->state = MFCINST_FREE;
wake_up(&ctx->queue);
goto irq_cleanup_hw; goto irq_cleanup_hw;
case S5P_MFC_R2H_CMD_SYS_INIT_RET: case S5P_MFC_R2H_CMD_SYS_INIT_RET:
...@@ -700,9 +696,9 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv) ...@@ -700,9 +696,9 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)
if (ctx) if (ctx)
clear_work_bit(ctx); clear_work_bit(ctx);
s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev); s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev);
wake_up_dev(dev, reason, err);
clear_bit(0, &dev->hw_lock); clear_bit(0, &dev->hw_lock);
clear_bit(0, &dev->enter_suspend); clear_bit(0, &dev->enter_suspend);
wake_up_dev(dev, reason, err);
break; break;
case S5P_MFC_R2H_CMD_INIT_BUFFERS_RET: case S5P_MFC_R2H_CMD_INIT_BUFFERS_RET:
...@@ -717,9 +713,7 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv) ...@@ -717,9 +713,7 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)
break; break;
case S5P_MFC_R2H_CMD_DPB_FLUSH_RET: case S5P_MFC_R2H_CMD_DPB_FLUSH_RET:
clear_work_bit(ctx);
ctx->state = MFCINST_RUNNING; ctx->state = MFCINST_RUNNING;
wake_up(&ctx->queue);
goto irq_cleanup_hw; goto irq_cleanup_hw;
default: default:
...@@ -738,6 +732,8 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv) ...@@ -738,6 +732,8 @@ static irqreturn_t s5p_mfc_irq(int irq, void *priv)
mfc_err("Failed to unlock hw\n"); mfc_err("Failed to unlock hw\n");
s5p_mfc_clock_off(); s5p_mfc_clock_off();
clear_work_bit(ctx);
wake_up(&ctx->queue);
s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); s5p_mfc_hw_call(dev->mfc_ops, try_run, dev);
spin_unlock(&dev->irqlock); spin_unlock(&dev->irqlock);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment