]>
Commit | Line | Data |
---|---|---|
dcfe13a2 GKH |
1 | From 0c32b8ec02832df167e16ad659cb11dc148f2ddf Mon Sep 17 00:00:00 2001 |
2 | From: Marek Szyprowski <m.szyprowski@samsung.com> | |
3 | Date: Thu, 23 Feb 2017 08:43:27 -0300 | |
4 | Subject: [media] s5p-mfc: Fix race between interrupt routine and device functions | |
5 | ||
6 | From: Marek Szyprowski <m.szyprowski@samsung.com> | |
7 | ||
8 | commit 0c32b8ec02832df167e16ad659cb11dc148f2ddf upstream. | |
9 | ||
10 | Interrupt routine must wake process waiting for given interrupt AFTER | |
11 | updating driver's internal structures and contexts. Doing it in-between | |
12 | is a serious bug. This patch moves all calls to the wake() function to | |
13 | the end of the interrupt processing block to avoid potential and real | |
14 | races, especially on multi-core platforms. This also fixes following issue | |
15 | reported from clock core (clocks were disabled in interrupt after being | |
16 | unprepared from the other place in the driver, the stack trace however | |
17 | points to the different place than s5p_mfc driver because of the race): | |
18 | ||
19 | WARNING: CPU: 1 PID: 18 at drivers/clk/clk.c:544 clk_core_unprepare+0xc8/0x108 | |
20 | Modules linked in: | |
21 | CPU: 1 PID: 18 Comm: kworker/1:0 Not tainted 4.10.0-next-20170223-00070-g04e18bc99ab9-dirty #2154 | |
22 | Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) | |
23 | Workqueue: pm pm_runtime_work | |
24 | [<c010d8b0>] (unwind_backtrace) from [<c010a534>] (show_stack+0x10/0x14) | |
25 | [<c010a534>] (show_stack) from [<c033292c>] (dump_stack+0x74/0x94) | |
26 | [<c033292c>] (dump_stack) from [<c011cef4>] (__warn+0xd4/0x100) | |
27 | [<c011cef4>] (__warn) from [<c011cf40>] (warn_slowpath_null+0x20/0x28) | |
28 | [<c011cf40>] (warn_slowpath_null) from [<c0387a84>] (clk_core_unprepare+0xc8/0x108) | |
29 | [<c0387a84>] (clk_core_unprepare) from [<c0389d84>] (clk_unprepare+0x24/0x2c) | |
30 | [<c0389d84>] (clk_unprepare) from [<c03d4660>] (exynos_sysmmu_suspend+0x48/0x60) | |
31 | [<c03d4660>] (exynos_sysmmu_suspend) from [<c042b9b0>] (pm_generic_runtime_suspend+0x2c/0x38) | |
32 | [<c042b9b0>] (pm_generic_runtime_suspend) from [<c0437580>] (genpd_runtime_suspend+0x94/0x220) | |
33 | [<c0437580>] (genpd_runtime_suspend) from [<c042e240>] (__rpm_callback+0x134/0x208) | |
34 | [<c042e240>] (__rpm_callback) from [<c042e334>] (rpm_callback+0x20/0x80) | |
35 | [<c042e334>] (rpm_callback) from [<c042d3b8>] (rpm_suspend+0xdc/0x458) | |
36 | [<c042d3b8>] (rpm_suspend) from [<c042ea24>] (pm_runtime_work+0x80/0x90) | |
37 | [<c042ea24>] (pm_runtime_work) from [<c01322c4>] (process_one_work+0x120/0x318) | |
38 | [<c01322c4>] (process_one_work) from [<c0132520>] (worker_thread+0x2c/0x4ac) | |
39 | [<c0132520>] (worker_thread) from [<c0137ab0>] (kthread+0xfc/0x134) | |
40 | [<c0137ab0>] (kthread) from [<c0107978>] (ret_from_fork+0x14/0x3c) | |
41 | ---[ end trace 1ead49a7bb83f0d8 ]--- | |
42 | ||
43 | Fixes: af93574678108 ("[media] MFC: Add MFC 5.1 V4L2 driver") | |
44 | ||
45 | Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> | |
46 | Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com> | |
47 | Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> | |
48 | Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> | |
49 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
50 | ||
51 | --- | |
52 | drivers/media/platform/s5p-mfc/s5p_mfc.c | 12 ++++-------- | |
53 | 1 file changed, 4 insertions(+), 8 deletions(-) | |
54 | ||
55 | --- a/drivers/media/platform/s5p-mfc/s5p_mfc.c | |
56 | +++ b/drivers/media/platform/s5p-mfc/s5p_mfc.c | |
57 | @@ -663,9 +663,9 @@ static irqreturn_t s5p_mfc_irq(int irq, | |
58 | break; | |
59 | } | |
60 | s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev); | |
61 | - wake_up_ctx(ctx, reason, err); | |
62 | WARN_ON(test_and_clear_bit(0, &dev->hw_lock) == 0); | |
63 | s5p_mfc_clock_off(); | |
64 | + wake_up_ctx(ctx, reason, err); | |
65 | s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); | |
66 | } else { | |
67 | s5p_mfc_handle_frame(ctx, reason, err); | |
68 | @@ -679,15 +679,11 @@ static irqreturn_t s5p_mfc_irq(int irq, | |
69 | case S5P_MFC_R2H_CMD_OPEN_INSTANCE_RET: | |
70 | ctx->inst_no = s5p_mfc_hw_call(dev->mfc_ops, get_inst_no, dev); | |
71 | ctx->state = MFCINST_GOT_INST; | |
72 | - clear_work_bit(ctx); | |
73 | - wake_up(&ctx->queue); | |
74 | goto irq_cleanup_hw; | |
75 | ||
76 | case S5P_MFC_R2H_CMD_CLOSE_INSTANCE_RET: | |
77 | - clear_work_bit(ctx); | |
78 | ctx->inst_no = MFC_NO_INSTANCE_SET; | |
79 | ctx->state = MFCINST_FREE; | |
80 | - wake_up(&ctx->queue); | |
81 | goto irq_cleanup_hw; | |
82 | ||
83 | case S5P_MFC_R2H_CMD_SYS_INIT_RET: | |
84 | @@ -697,9 +693,9 @@ static irqreturn_t s5p_mfc_irq(int irq, | |
85 | if (ctx) | |
86 | clear_work_bit(ctx); | |
87 | s5p_mfc_hw_call(dev->mfc_ops, clear_int_flags, dev); | |
88 | - wake_up_dev(dev, reason, err); | |
89 | clear_bit(0, &dev->hw_lock); | |
90 | clear_bit(0, &dev->enter_suspend); | |
91 | + wake_up_dev(dev, reason, err); | |
92 | break; | |
93 | ||
94 | case S5P_MFC_R2H_CMD_INIT_BUFFERS_RET: | |
95 | @@ -714,9 +710,7 @@ static irqreturn_t s5p_mfc_irq(int irq, | |
96 | break; | |
97 | ||
98 | case S5P_MFC_R2H_CMD_DPB_FLUSH_RET: | |
99 | - clear_work_bit(ctx); | |
100 | ctx->state = MFCINST_RUNNING; | |
101 | - wake_up(&ctx->queue); | |
102 | goto irq_cleanup_hw; | |
103 | ||
104 | default: | |
105 | @@ -735,6 +729,8 @@ irq_cleanup_hw: | |
106 | mfc_err("Failed to unlock hw\n"); | |
107 | ||
108 | s5p_mfc_clock_off(); | |
109 | + clear_work_bit(ctx); | |
110 | + wake_up(&ctx->queue); | |
111 | ||
112 | s5p_mfc_hw_call(dev->mfc_ops, try_run, dev); | |
113 | spin_unlock(&dev->irqlock); |