2023-12-04 11:30:58

by Geert Uytterhoeven

[permalink] [raw]
Subject: [PATCH v2] mmc: core: Cancel delayed work before releasing host

On RZ/Five SMARC EVK, where probing of SDHI is deferred due to probe
deferral of the vqmmc-supply regulator:

------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at kernel/time/timer.c:1738 __run_timers.part.0+0x1d0/0x1e8
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 6.7.0-rc4 #101
Hardware name: Renesas SMARC EVK based on r9a07g043f01 (DT)
epc : __run_timers.part.0+0x1d0/0x1e8
ra : __run_timers.part.0+0x134/0x1e8
epc : ffffffff800771a4 ra : ffffffff80077108 sp : ffffffc800003e60
gp : ffffffff814f5028 tp : ffffffff8140c5c0 t0 : ffffffc800000000
t1 : 0000000000000001 t2 : ffffffff81201300 s0 : ffffffc800003f20
s1 : ffffffd8023bc4a0 a0 : 00000000fffee6b0 a1 : 0004010000400000
a2 : ffffffffc0000016 a3 : ffffffff81488640 a4 : ffffffc800003e60
a5 : 0000000000000000 a6 : 0000000004000000 a7 : ffffffc800003e68
s2 : 0000000000000122 s3 : 0000000000200000 s4 : 0000000000000000
s5 : ffffffffffffffff s6 : ffffffff81488678 s7 : ffffffff814886c0
s8 : ffffffff814f49c0 s9 : ffffffff81488640 s10: 0000000000000000
s11: ffffffc800003e60 t3 : 0000000000000240 t4 : 0000000000000a52
t5 : ffffffd8024ae018 t6 : ffffffd8024ae038
status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
[<ffffffff800771a4>] __run_timers.part.0+0x1d0/0x1e8
[<ffffffff800771e0>] run_timer_softirq+0x24/0x4a
[<ffffffff80809092>] __do_softirq+0xc6/0x1fa
[<ffffffff80028e4c>] irq_exit_rcu+0x66/0x84
[<ffffffff80800f7a>] handle_riscv_irq+0x40/0x4e
[<ffffffff80808f48>] call_on_irq_stack+0x1c/0x28
---[ end trace 0000000000000000 ]---

What happens?

renesas_sdhi_probe()
{
tmio_mmc_host_alloc()
mmc_alloc_host()
INIT_DELAYED_WORK(&host->detect, mmc_rescan);

devm_request_irq(tmio_mmc_irq);

/*
* After this, the interrupt handler may be invoked at any time
*
* tmio_mmc_irq()
* {
* __tmio_mmc_card_detect_irq()
* mmc_detect_change()
* _mmc_detect_change()
* mmc_schedule_delayed_work(&host->detect, delay);
* }
*/

tmio_mmc_host_probe()
tmio_mmc_init_ocr()
-EPROBE_DEFER

tmio_mmc_host_free()
mmc_free_host()
}

When expire_timers() runs later, it warns because the MMC host structure
containing the delayed work was freed, and now contains an invalid work
function pointer.

Fix this by cancelling any pending delayed work before releasing the
MMC host structure.

Signed-off-by: Geert Uytterhoeven <[email protected]>
---
This is v2 of "[RFC] mmc: tmio: Cancel delayed work before freeing
host".

v2:
- Move cancel_delayed_work_sync() call from tmio_mmc_host_free() to
mmc_free_host(),
- Correct explanation from missing pin control to vqmmc-supply probe
deferral,
- Update backtrace.
---
drivers/mmc/core/host.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 096093f7be006353..2f51db4df1a8571b 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -692,6 +692,7 @@ EXPORT_SYMBOL(mmc_remove_host);
*/
void mmc_free_host(struct mmc_host *host)
{
+ cancel_delayed_work_sync(&host->detect);
mmc_pwrseq_free(host);
put_device(&host->class_dev);
}
--
2.34.1


2023-12-04 13:00:39

by Prabhakar

[permalink] [raw]
Subject: Re: [PATCH v2] mmc: core: Cancel delayed work before releasing host

On Mon, Dec 4, 2023 at 11:30 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> On RZ/Five SMARC EVK, where probing of SDHI is deferred due to probe
> deferral of the vqmmc-supply regulator:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at kernel/time/timer.c:1738 __run_timers.part.0+0x1d0/0x1e8
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper Not tainted 6.7.0-rc4 #101
> Hardware name: Renesas SMARC EVK based on r9a07g043f01 (DT)
> epc : __run_timers.part.0+0x1d0/0x1e8
> ra : __run_timers.part.0+0x134/0x1e8
> epc : ffffffff800771a4 ra : ffffffff80077108 sp : ffffffc800003e60
> gp : ffffffff814f5028 tp : ffffffff8140c5c0 t0 : ffffffc800000000
> t1 : 0000000000000001 t2 : ffffffff81201300 s0 : ffffffc800003f20
> s1 : ffffffd8023bc4a0 a0 : 00000000fffee6b0 a1 : 0004010000400000
> a2 : ffffffffc0000016 a3 : ffffffff81488640 a4 : ffffffc800003e60
> a5 : 0000000000000000 a6 : 0000000004000000 a7 : ffffffc800003e68
> s2 : 0000000000000122 s3 : 0000000000200000 s4 : 0000000000000000
> s5 : ffffffffffffffff s6 : ffffffff81488678 s7 : ffffffff814886c0
> s8 : ffffffff814f49c0 s9 : ffffffff81488640 s10: 0000000000000000
> s11: ffffffc800003e60 t3 : 0000000000000240 t4 : 0000000000000a52
> t5 : ffffffd8024ae018 t6 : ffffffd8024ae038
> status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
> [<ffffffff800771a4>] __run_timers.part.0+0x1d0/0x1e8
> [<ffffffff800771e0>] run_timer_softirq+0x24/0x4a
> [<ffffffff80809092>] __do_softirq+0xc6/0x1fa
> [<ffffffff80028e4c>] irq_exit_rcu+0x66/0x84
> [<ffffffff80800f7a>] handle_riscv_irq+0x40/0x4e
> [<ffffffff80808f48>] call_on_irq_stack+0x1c/0x28
> ---[ end trace 0000000000000000 ]---
>
> What happens?
>
> renesas_sdhi_probe()
> {
> tmio_mmc_host_alloc()
> mmc_alloc_host()
> INIT_DELAYED_WORK(&host->detect, mmc_rescan);
>
> devm_request_irq(tmio_mmc_irq);
>
> /*
> * After this, the interrupt handler may be invoked at any time
> *
> * tmio_mmc_irq()
> * {
> * __tmio_mmc_card_detect_irq()
> * mmc_detect_change()
> * _mmc_detect_change()
> * mmc_schedule_delayed_work(&host->detect, delay);
> * }
> */
>
> tmio_mmc_host_probe()
> tmio_mmc_init_ocr()
> -EPROBE_DEFER
>
> tmio_mmc_host_free()
> mmc_free_host()
> }
>
> When expire_timers() runs later, it warns because the MMC host structure
> containing the delayed work was freed, and now contains an invalid work
> function pointer.
>
> Fix this by cancelling any pending delayed work before releasing the
> MMC host structure.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>
> ---
> This is v2 of "[RFC] mmc: tmio: Cancel delayed work before freeing
> host".
>
> v2:
> - Move cancel_delayed_work_sync() call from tmio_mmc_host_free() to
> mmc_free_host(),
> - Correct explanation from missing pin control to vqmmc-supply probe
> deferral,
> - Update backtrace.
> ---
> drivers/mmc/core/host.c | 1 +
> 1 file changed, 1 insertion(+)
>
Tested-by: Lad Prabhakar <[email protected]>

Cheers,
Prabhakar

> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 096093f7be006353..2f51db4df1a8571b 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -692,6 +692,7 @@ EXPORT_SYMBOL(mmc_remove_host);
> */
> void mmc_free_host(struct mmc_host *host)
> {
> + cancel_delayed_work_sync(&host->detect);
> mmc_pwrseq_free(host);
> put_device(&host->class_dev);
> }
> --
> 2.34.1
>
>

2023-12-07 14:18:25

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH v2] mmc: core: Cancel delayed work before releasing host

On Mon, 4 Dec 2023 at 12:30, Geert Uytterhoeven <[email protected]> wrote:
>
> On RZ/Five SMARC EVK, where probing of SDHI is deferred due to probe
> deferral of the vqmmc-supply regulator:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at kernel/time/timer.c:1738 __run_timers.part.0+0x1d0/0x1e8
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper Not tainted 6.7.0-rc4 #101
> Hardware name: Renesas SMARC EVK based on r9a07g043f01 (DT)
> epc : __run_timers.part.0+0x1d0/0x1e8
> ra : __run_timers.part.0+0x134/0x1e8
> epc : ffffffff800771a4 ra : ffffffff80077108 sp : ffffffc800003e60
> gp : ffffffff814f5028 tp : ffffffff8140c5c0 t0 : ffffffc800000000
> t1 : 0000000000000001 t2 : ffffffff81201300 s0 : ffffffc800003f20
> s1 : ffffffd8023bc4a0 a0 : 00000000fffee6b0 a1 : 0004010000400000
> a2 : ffffffffc0000016 a3 : ffffffff81488640 a4 : ffffffc800003e60
> a5 : 0000000000000000 a6 : 0000000004000000 a7 : ffffffc800003e68
> s2 : 0000000000000122 s3 : 0000000000200000 s4 : 0000000000000000
> s5 : ffffffffffffffff s6 : ffffffff81488678 s7 : ffffffff814886c0
> s8 : ffffffff814f49c0 s9 : ffffffff81488640 s10: 0000000000000000
> s11: ffffffc800003e60 t3 : 0000000000000240 t4 : 0000000000000a52
> t5 : ffffffd8024ae018 t6 : ffffffd8024ae038
> status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
> [<ffffffff800771a4>] __run_timers.part.0+0x1d0/0x1e8
> [<ffffffff800771e0>] run_timer_softirq+0x24/0x4a
> [<ffffffff80809092>] __do_softirq+0xc6/0x1fa
> [<ffffffff80028e4c>] irq_exit_rcu+0x66/0x84
> [<ffffffff80800f7a>] handle_riscv_irq+0x40/0x4e
> [<ffffffff80808f48>] call_on_irq_stack+0x1c/0x28
> ---[ end trace 0000000000000000 ]---
>
> What happens?
>
> renesas_sdhi_probe()
> {
> tmio_mmc_host_alloc()
> mmc_alloc_host()
> INIT_DELAYED_WORK(&host->detect, mmc_rescan);
>
> devm_request_irq(tmio_mmc_irq);
>
> /*
> * After this, the interrupt handler may be invoked at any time
> *
> * tmio_mmc_irq()
> * {
> * __tmio_mmc_card_detect_irq()
> * mmc_detect_change()
> * _mmc_detect_change()
> * mmc_schedule_delayed_work(&host->detect, delay);
> * }
> */
>
> tmio_mmc_host_probe()
> tmio_mmc_init_ocr()
> -EPROBE_DEFER
>
> tmio_mmc_host_free()
> mmc_free_host()
> }
>
> When expire_timers() runs later, it warns because the MMC host structure
> containing the delayed work was freed, and now contains an invalid work
> function pointer.
>
> Fix this by cancelling any pending delayed work before releasing the
> MMC host structure.
>
> Signed-off-by: Geert Uytterhoeven <[email protected]>

Applied for next and by adding a stable tag, thanks!

Kind regards
Uffe

> ---
> This is v2 of "[RFC] mmc: tmio: Cancel delayed work before freeing
> host".
>
> v2:
> - Move cancel_delayed_work_sync() call from tmio_mmc_host_free() to
> mmc_free_host(),
> - Correct explanation from missing pin control to vqmmc-supply probe
> deferral,
> - Update backtrace.
> ---
> drivers/mmc/core/host.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
> index 096093f7be006353..2f51db4df1a8571b 100644
> --- a/drivers/mmc/core/host.c
> +++ b/drivers/mmc/core/host.c
> @@ -692,6 +692,7 @@ EXPORT_SYMBOL(mmc_remove_host);
> */
> void mmc_free_host(struct mmc_host *host)
> {
> + cancel_delayed_work_sync(&host->detect);
> mmc_pwrseq_free(host);
> put_device(&host->class_dev);
> }
> --
> 2.34.1
>