From ae1efd457bdf513bc37586d703666f3aab985649 Mon Sep 17 00:00:00 2001 From: Abhishek Shah Date: Mon, 16 Aug 2021 16:47:52 +0530 Subject: [PATCH] devfreq: governor_bw_hwmon: fix deadlock warning due to state_lock usage lockdep is detecting possible circular locking dependency due to state_lock mutex as shown below: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(devfreq_list_lock); lock(state_lock#2); lock(devfreq_list_lock); lock(state_lock#2); *** DEADLOCK *** Below is partial call stacks (in reverse order) showing relevant locking paths: Call stack for CPU0: devfreq_bw_hwmon_ev_handler+0x4c/0x5e0 [may acquire &state_lock] devfreq_add_device+0x418/0x538 [may acquire &devfreq_list_lock] devfreq_add_icc+0x41c/0x528 devfreq_icc_probe+0x20/0x30 Call stack for CPU1: devfreq_add_governor+0x3c/0x260 [may acquire &devfreq_list_lock] register_bw_hwmon+0x1e8/0x248 [may acquire &state_lock] bimc_bwmon_driver_probe+0x310/0x408 Practically, this race is not possible, since devfreq_add_device first tries to find the governor, and only if it is succeeds, devfreq_bw_hwmon_ev_handler is called. And the governor would be found only if devfreq_add_governor has added it priorly. We have mechanism(initcall_level) in place to make sure that governor is added before devfreq_add_device happens. In attempt to quiet the lockdep warning, below fix is adopted: Since state_lock mutex has different purpose, introduce a new event_handle_lock mutex for devfreq_bw_hwmon_ev_handler to avoid this warning. Change-Id: I53c4514aa2357bf39e9405683f5e73ada0160ba5 Signed-off-by: Abhishek Shah --- drivers/devfreq/governor_bw_hwmon.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/devfreq/governor_bw_hwmon.c b/drivers/devfreq/governor_bw_hwmon.c index 2acdb0a3fe5a..261b40843ff3 100644 --- a/drivers/devfreq/governor_bw_hwmon.c +++ b/drivers/devfreq/governor_bw_hwmon.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2013-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2013-2021, The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt) "bw-hwmon: " fmt @@ -80,6 +80,8 @@ static DEFINE_MUTEX(list_lock); static int use_cnt; static DEFINE_MUTEX(state_lock); +static DEFINE_MUTEX(event_handle_lock); + #define show_attr(name) \ static ssize_t name##_show(struct device *dev, \ struct device_attribute *attr, char *buf) \ @@ -865,7 +867,7 @@ static int devfreq_bw_hwmon_ev_handler(struct devfreq *df, struct hwmon_node *node; struct bw_hwmon *hw; - mutex_lock(&state_lock); + mutex_lock(&event_handle_lock); switch (event) { case DEVFREQ_GOV_START: @@ -939,7 +941,7 @@ static int devfreq_bw_hwmon_ev_handler(struct devfreq *df, } out: - mutex_unlock(&state_lock); + mutex_unlock(&event_handle_lock); return ret; }