[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a3255da4-b2af-4403-af68-3067a5fd49bf@linaro.org>
Date: Thu, 12 Oct 2023 17:39:08 +0200
From: Daniel Lezcano <daniel.lezcano@...aro.org>
To: Bo Ye <bo.ye@...iatek.com>,
"Rafael J. Wysocki" <rafael@...nel.org>,
Amit Kucheria <amitk@...nel.org>,
Zhang Rui <rui.zhang@...el.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
Cc: yugang.wang@...iatek.com, yongdong.zhang@...iatek.com,
browse.zhang@...iatek.com, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH] Subject: thermal: Fix potential race condition in
suspend/resume
On 16/09/2023 13:33, Bo Ye wrote:
> From: "yugang.wang" <yugang.wang@...iatek.com>
>
> Body:
> This patch fixes a race condition during system resume. It occurs if
> the system is exiting a suspend state and a user is trying to
> register/unregister a thermal zone concurrently. The root cause is
> that both actions access the `thermal_tz_list`.
I'm not sure the tasks are already thawed during POST_RESTORE, so no
user can unload a driver and then reaching the race window.
Is that an observed issue?
> In detail:
>
> 1. At PM_POST_SUSPEND during the resume, the system reads all thermal
> zones in `thermal_tz_list`, then resets and updates their
> temperatures.
> 2. When registering/unregistering a thermal zone, the
> `thermal_tz_list` gets manipulated.
>
> These two actions might occur concurrently, causing a race condition.
> To solve this issue, we introduce a mutex lock to protect
> `thermal_tz_list` from being modified while it's being read and
> updated during the resume from suspend.
>
> Kernel oops excerpt related to this fix:
>
> [ 5201.869845] [T316822] pc: [0xffffffeb7d4876f0] mutex_lock+0x34/0x170
> [ 5201.869856] [T316822] lr: [0xffffffeb7ca98a84] thermal_pm_notify+0xd4/0x26c
> [... cut for brevity ...]
> [ 5201.871061] [T316822] suspend_prepare+0x150/0x470
> [ 5201.871067] [T316822] enter_state+0x84/0x6f4
> [ 5201.871076] [T316822] state_store+0x15c/0x1e8
>
> Change-Id: Ifdbdecba17093f91eab7e36ce04b46d311ca6568
> Signed-off-by: yugang.wang <yugang.wang@...iatek.com>
> Signed-off-by: Bo Ye <bo.ye@...iatek.com>
> ---
> drivers/thermal/thermal_core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 8717a3343512..a7a18ed57b6d 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1529,12 +1529,14 @@ static int thermal_pm_notify(struct notifier_block *nb,
> case PM_POST_HIBERNATION:
> case PM_POST_RESTORE:
> case PM_POST_SUSPEND:
> + mutex_lock(&thermal_list_lock);
> atomic_set(&in_suspend, 0);
> list_for_each_entry(tz, &thermal_tz_list, node) {
> thermal_zone_device_init(tz);
> thermal_zone_device_update(tz,
> THERMAL_EVENT_UNSPECIFIED);
> }
> + mutex_unlock(&thermal_list_lock);
> break;
> default:
> break;
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
Powered by blists - more mailing lists