From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 264963ACEE0 for ; Tue, 7 Apr 2026 11:32:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=205.220.168.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775561573; cv=none; b=Uy2RXffSyR5sUTNNdtzI+EPaUUGX7Uqkt8KWzm/2Fpa9V2GERmXjmzcXBZz1ZNHYLZBY9Z6MAsmDa1vb/3dPFSW2s3z+7n112Ok/zdZVJVTNpwbcyvoW9zI5zLnhSekO49Ppm+6ezd85ejswJBJbbK6iwturehWYuIM7Lg7Kfkg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775561573; c=relaxed/simple; bh=GsB/BDBtYHQjLoehOOANUPS3ZltGayGa06lBSGhlTyo=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=fGv8mYv0SIN+hS6zIhqw38ezdh347V+6KLoxgqiqPHYSVV79jGs230a42L74DnzGcpNX0GE9mb1Gi4/mpZQ5Hdd8elnRcqnYZDfn8gfT4m01jimfRH3BQ2mDJCXplfGOR4PHIBO2AGvwVlco77V+JWbNu2A8Jzo/fBsXe/S0zv0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com; spf=pass smtp.mailfrom=oss.qualcomm.com; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b=RpF3BiHu; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b=NVNORW8b; arc=none smtp.client-ip=205.220.168.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b="RpF3BiHu"; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b="NVNORW8b" Received: from pps.filterd (m0279864.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 6376Zk4S2549950 for ; Tue, 7 Apr 2026 11:32:51 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=qcppdkim1; bh= p06D6z7PPjOp305ThiF1ke9VrYyHLovqGljvnynJk4c=; b=RpF3BiHuaelR42Q+ zCx7MlPAJ22VKxkCBvWPnAc+u3RA0E/wJXB8Sl0bXzlLksO3W2GJjU7BrgMNRjp0 339M4HbSXsLnBMJ4vqejEYOaeSBbmMHAoM2cb/97KinP/Z+WzgFlRPCt20L6aZXQ mMoJ3U9CoE6fEmuTmMNJdj1R1TBLR3okEm8yVeMNVEHVmnERPYwJf8ey6IKAB8Ln hP/s4fI853cbrO2IZF1/AeAM6Ds7u+5OWtZP57eVje1yvN5uElrv1wKvfF8wnB7G UjPonWF4HadfZhIJSlvgN2uar1z0I2bsRLew/sg/bKf7yECNTfuDeG9sKFQFOvpy y1wI0w== Received: from mail-pg1-f200.google.com (mail-pg1-f200.google.com [209.85.215.200]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4dcmreah0u-1 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT) for ; Tue, 07 Apr 2026 11:32:51 +0000 (GMT) Received: by mail-pg1-f200.google.com with SMTP id 41be03b00d2f7-c76dbf0fad3so1739898a12.1 for ; Tue, 07 Apr 2026 04:32:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oss.qualcomm.com; s=google; t=1775561571; x=1776166371; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=p06D6z7PPjOp305ThiF1ke9VrYyHLovqGljvnynJk4c=; b=NVNORW8b+NwNGcbr2wGbxsn1hrBWEQPUZCSkqmNdtSoiRU6+1CQTPlz2Nd5pNn/NNl EBDmZFTTtcQLZt7TI/MQ/i6lzF4UA0DF73Nr41qPcnwikF9GYtd39ofe/1enBX8PIGl+ c+FPhxg5/QpMRhJt57AmbkmeDS4o64Nr1gij1tT0YCfug5eBrgxH/1Bab8RATNARax8K M4WRcRWhGacPYSkvhCuLS9O+b6MYKI6/IGJ9EIoG0zrhhIMPywSCQQ8NXk1wXfuuT5Qc 9Id6Dg4XSlB4HX9WpPf4N86fN9G56nGLvectZeBoJjOydNGYcsBY7F2uEmliXIHYWNO/ Jj7A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775561571; x=1776166371; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=p06D6z7PPjOp305ThiF1ke9VrYyHLovqGljvnynJk4c=; b=W5SwIzjzJh3YOt6lkJxWsICCOWETIRJSLxVFSUhYHYYz6vIPc7ofrcWBrYJD6OtGY/ 2cOthWyPJoVXo8Y4xJUa6cv9TF6SOeZy5yK9r9Y05Hf+rW3VTRdBFvBeXzdYqSsrnN7I tkDQCEjsGBew13HjLvdAGxiYib1x1F10nL5MQpgmz3uovRBoZxOCPBneQy0Q/f4dv/s3 Fz7K2EdYZFu4zfEzPjO3Qqd1sR11YFIOzpgkWgrGZ8aAYnX8Pc3ZZBZ2d7zRO7tvBmPc yAD51Bu8DmC8x48BQogOKof6uVr0WU4Bnu0VqXUjpzZWkaEMhHOFnA4Ir0RCNMBIuAc5 vXFQ== X-Forwarded-Encrypted: i=1; AJvYcCVIlmIb8QdqmiX5OAADlUbJA4nBrBtmZljdo5d0QEtznseWy+CN+hQpjMzl0IBHX4oI6WAQJtjknzbO2Jk=@vger.kernel.org X-Gm-Message-State: AOJu0YwPeL50S/F2mJajLOMXsIoZHc3xT9pf1rpOtsgdyRPFvamjIuiU Ir1B3DZ7FJnyTDyAbN/TEbu+mdAEYdvW81AdLfcJZJCKSdj6I2E/EuYZYfLh2iRGEsiY6yhN9Ml MzcoZ/bDUWiBVHtbfIzLEUVMwDfkteH3h3j7AVvfmSGgaxrm+k06RKVayFhMPpBCXKSE= X-Gm-Gg: AeBDietAqFajCAT3lee6u9C8LoA06iDQjFWNHXbRHp+uhdefo2I1DaeurGDVN2rEyRs DRvit2CJqOZGyb3MvobwS5wk4l21WGVQ6zIBwhs5rl1fxshvjV9rST9bir3IOaglEVVgFwL6v/5 1NA1HgD4/2Lus8FTBVOz8EuwevskxnGe9T92Z7IbsUefax48u3z9BmzHq7n6CShCkD35tD/w76l wWagX2WJiE0pro6QU2jUjxlAVQ9n9FS2Blx8KuL/xV+yConDXID3Lkkf+G/lZN460iV56ejl5Su o4nWrwyS7krJZHr6e/gdY+7dqXZV/ha+7Ljl0enNwy7KDd4tFUQL8MGYeW8lsf9Ta/KKUJHzX5c WZzilEvm0wMK4wcSVme7/lLVevLIUlvR8zdj4w8xdnaXUYjIFcSnnwZlN3kJoX2MZzCv4r+4Otb 9guzjy8uUHeWuqzizH X-Received: by 2002:a05:6a00:1805:b0:82a:1380:417d with SMTP id d2e1a72fcca58-82d0dbcfad1mr16233354b3a.52.1775561570683; Tue, 07 Apr 2026 04:32:50 -0700 (PDT) X-Received: by 2002:a05:6a00:1805:b0:82a:1380:417d with SMTP id d2e1a72fcca58-82d0dbcfad1mr16233325b3a.52.1775561570149; Tue, 07 Apr 2026 04:32:50 -0700 (PDT) Received: from [10.133.33.42] (tpe-colo-wan-fw-bordernet.qualcomm.com. [103.229.16.4]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-82cf9c4387esm18126396b3a.31.2026.04.07.04.32.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 07 Apr 2026 04:32:49 -0700 (PDT) Message-ID: <2864ff7a-61a0-4b4a-9e54-389ab47fcc98@oss.qualcomm.com> Date: Tue, 7 Apr 2026 19:32:44 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] cpufreq: governor: Fix race between sysfs store and dbs work handler To: "Rafael J. Wysocki" Cc: viresh.kumar@linaro.org, venkatesh.pallipadi@intel.com, davej@redhat.com, trenn@suse.de, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, zhongqiu.han@oss.qualcomm.com References: <20260406110113.3475920-1-zhongqiu.han@oss.qualcomm.com> Content-Language: en-US From: Zhongqiu Han In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Authority-Analysis: v=2.4 cv=OKEXGyaB c=1 sm=1 tr=0 ts=69d4eb63 cx=c_pps a=oF/VQ+ItUULfLr/lQ2/icg==:117 a=nuhDOHQX5FNHPW3J6Bj6AA==:17 a=IkcTkHD0fZMA:10 a=A5OVakUREuEA:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22 a=u7WPNUs3qKkmUXheDGA7:22 a=DJpcGTmdVt4CTyJn9g5Z:22 a=c92rfblmAAAA:8 a=EUspDBNiAAAA:8 a=sLMbjZCTKKTjWq_z0XMA:9 a=3ZKOabzyN94A:10 a=QEXdDO2ut3YA:10 a=3WC7DwWrALyhR5TkjVHa:22 a=GvGzcOZaWPEFPQC_NcjD:22 X-Proofpoint-GUID: KczKxOcomBsZa5bOXltubS4LzGRPq7vt X-Proofpoint-ORIG-GUID: KczKxOcomBsZa5bOXltubS4LzGRPq7vt X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwNDA3MDEwOCBTYWx0ZWRfX7n1uO1qUPP0B zZ2QKiMSHEzINSGcER5CKy524zL64VDrL/KV4nFFMQ8otrsk8XC2Qp4NooU8m6EZrpWb/uCA+5u VyXoXQmhmNmnoGGCTXfYRte4hxAbHBJyPAYxaa5L5v7pUurfYQSzwjlJ1NeaUkhFTyXW95EAi68 5xFEHvlLd1xJoHGI6EtV3nPg0TM0wPe1S8AR9wvBiF/mU8cacXQnSYikEiW7B2u7cJvXQ4IE5+G YMACfjW4vzyv2ghJE28dW3atGHqDFocQoaq2KIhA0Ks8uKpqWdhctmtd6SQhZXxiSZIWYWMBQiP dAstqi2ZPFK1TTADnN0vIfsE4if4ThcK/2DICRhIWp1eSYLbtD9fMV51+kJPgn0A7fGn2E7zV+A qAFztpJVUdRfiJZMmdScHcB7rCnIqYwJ1vof0BcXxYc5sVF1Ea2LyOsdVdKK6wFvDkEYUDITqtP GOvhSd2CcDUG75ifIZQ== X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1143,Hydra:6.1.51,FMLib:17.12.100.49 definitions=2026-04-07_02,2026-04-07_02,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 priorityscore=1501 spamscore=0 impostorscore=0 lowpriorityscore=0 clxscore=1015 adultscore=0 malwarescore=0 suspectscore=0 phishscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2604010000 definitions=main-2604070108 On 4/6/2026 11:21 PM, Rafael J. Wysocki wrote: > On Mon, Apr 6, 2026 at 1:01 PM Zhongqiu Han > wrote: >> >> gov_update_cpu_data() resets per-CPU prev_cpu_idle and prev_cpu_nice >> for every CPU in the governed domain. It is called from sysfs store >> callbacks (e.g. ignore_nice_load_store) which run under >> attr_set->update_lock, held by the surrounding governor_store(). >> >> Concurrently, dbs_work_handler() calls gov->gov_dbs_update() (which >> calls dbs_update()) under policy_dbs->update_mutex. dbs_update() both >> reads and writes the same prev_cpu_idle / prev_cpu_nice fields. The >> potential race path is: >> >> Path A (sysfs write, holds attr_set->update_lock only): >> >> governor_store() >> mutex_lock(&attr_set->update_lock) >> ignore_nice_load_store() >> dbs_data->ignore_nice_load = input >> gov_update_cpu_data(dbs_data) >> list_for_each_entry(policy_dbs, ...) >> for_each_cpu(j, ...) >> j_cdbs->prev_cpu_idle = get_cpu_idle_time(...) /* write */ >> j_cdbs->prev_cpu_nice = kcpustat_field(...) /* write */ >> mutex_unlock(&attr_set->update_lock) >> >> Path B (work queue, holds policy_dbs->update_mutex only): >> >> dbs_work_handler() >> mutex_lock(&policy_dbs->update_mutex) >> gov->gov_dbs_update(policy) >> dbs_update() >> for_each_cpu(j, policy->cpus) >> idle_time = cur - j_cdbs->prev_cpu_idle /* read */ >> j_cdbs->prev_cpu_idle = cur_idle_time /* write */ >> idle_time += cur_nice - j_cdbs->prev_cpu_nice /* read */ >> j_cdbs->prev_cpu_nice = cur_nice /* write */ >> mutex_unlock(&policy_dbs->update_mutex) >> >> Because attr_set->update_lock and policy_dbs->update_mutex are two >> completely independent locks, the two paths are not mutually exclusive. >> This results in a data race on cpu_dbs_info.prev_cpu_idle and >> cpu_dbs_info.prev_cpu_nice. >> >> Fix this by also acquiring policy_dbs->update_mutex in >> gov_update_cpu_data() for each policy, so that path A participates in >> the mutual exclusion already established by dbs_work_handler(). Also >> update the function comment to accurately reflect the two-level locking >> contract. >> >> The root of this race dates back to the original ondemand/conservative >> governors. Before commit ee88415caf73 ("[CPUFREQ] Cleanup locking in >> conservative governor") and commit 5a75c82828e7 ("[CPUFREQ] Cleanup >> locking in ondemand governor"), all accesses to prev_cpu_idle and >> prev_cpu_nice in cpufreq_governor_dbs() (path X), store_ignore_nice_load() >> (path Y), and do_dbs_timer() (path Z) were serialised by the same >> dbs_mutex, so no race existed. Those two commits switched do_dbs_timer() >> from dbs_mutex to a per-policy/per-cpu timer_mutex to reduce lock >> contention, but left store_ignore_nice_load() still holding dbs_mutex. >> As a result, path Y (store) and path Z (do_dbs_timer) no longer shared a >> common lock, introducing a potential race on prev_cpu_idle/prev_cpu_nice >> between store_ignore_nice_load() and dbs_check_cpu(). >> >> Commit 326c86deaed54a ("[CPUFREQ] Remove unneeded locks") then removed >> dbs_mutex from store_ignore_nice_load() entirely, introducing an >> additional potential race between store_ignore_nice_load() (path Y, now >> lockless) and cpufreq_governor_dbs() (path X, still holding dbs_mutex), >> while the race between path Y and path Z remained. >> >> Fixes: ee88415caf736b ("[CPUFREQ] Cleanup locking in conservative governor") >> Fixes: 5a75c82828e7c0 ("[CPUFREQ] Cleanup locking in ondemand governor") >> Fixes: 326c86deaed54a ("[CPUFREQ] Remove unneeded locks") >> Signed-off-by: Zhongqiu Han >> --- >> drivers/cpufreq/cpufreq_governor.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c >> index 86f35e451914..56ef793362db 100644 >> --- a/drivers/cpufreq/cpufreq_governor.c >> +++ b/drivers/cpufreq/cpufreq_governor.c >> @@ -90,7 +90,8 @@ EXPORT_SYMBOL_GPL(sampling_rate_store); >> * (that may be a single policy or a bunch of them if governor tunables are >> * system-wide). >> * >> - * Call under the @dbs_data mutex. >> + * Call under the @dbs_data->attr_set.update_lock. The per-policy >> + * update_mutex is acquired and released internally for each policy. >> */ >> void gov_update_cpu_data(struct dbs_data *dbs_data) >> { >> @@ -99,6 +100,7 @@ void gov_update_cpu_data(struct dbs_data *dbs_data) >> list_for_each_entry(policy_dbs, &dbs_data->attr_set.policy_list, list) { >> unsigned int j; >> >> + mutex_lock(&policy_dbs->update_mutex); >> for_each_cpu(j, policy_dbs->policy->cpus) { >> struct cpu_dbs_info *j_cdbs = &per_cpu(cpu_dbs, j); >> >> @@ -107,6 +109,7 @@ void gov_update_cpu_data(struct dbs_data *dbs_data) >> if (dbs_data->ignore_nice_load) >> j_cdbs->prev_cpu_nice = kcpustat_field(&kcpustat_cpu(j), CPUTIME_NICE, j); >> } >> + mutex_unlock(&policy_dbs->update_mutex); >> } >> } >> EXPORT_SYMBOL_GPL(gov_update_cpu_data); >> -- > > Please have a look at > > https://sashiko.dev/#/patchset/20260406110113.3475920-1-zhongqiu.han%40oss.qualcomm.com > > and let me know what you think. > > Thanks! Hi Rafael, Thanks for the review, sashiko.dev points out two pre‑existing gaps rather than regressions introduced by this patch. Issue 1: There is a window where sysfs writes can race with the unprotected initialization loop in cpufreq_dbs_governor_start(), which may lead to races. I will address this by protecting the initialization loop with policy_dbs->update_mutex. Issue 2: There is a residual race where dbs_update() may observe a newly updated tunable together with stale counters, leading to an inflated idle delta. I plan to address this by always updating prev_cpu_nice in dbs_update(), regardless of ignore_nice, to avoid stale state. I will double‑check the logic and address this in patch v2. -- Thx and BRs, Zhongqiu Han