From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id DBF674A04; Sun, 31 Aug 2025 21:30:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756675818; cv=none; b=mknPSVR1O265mDyCKcEttjfLU0et/pPWJ6P2ygehNYlGSVQ/GK0UfK32U2EQj/NC/xAxEburIeMtcmiZwi3VuVDevwFFEi/YAPMicvvj5c6C5g9kKWJRn3YbE2pOQMTQpLxFTfqf6rNTAM7XTjOoOh3mKbaeJ2mzZ36FOz5hutQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1756675818; c=relaxed/simple; bh=oIxuXkfSseOKb2DSBy4MWIrocNC2sTkpyvkWCsKkAH4=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=r1eXYS+x4qQOS1NEZscvSUX1Bzm6ZeANqB8euMsWiBK6GGTFYqEkIcso2gSm2pUpeltwwRzkE6BGJg0swVq8DwL/SBKpnlpHwXAZFHrV85CckETQS6dDhy5iuWAG9iqXsc0eqZCF5Eks80pWYcTswiHtqV8rfxEI4XeRG/th/WY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0F5FB1D13; Sun, 31 Aug 2025 14:30:00 -0700 (PDT) Received: from [10.57.78.139] (unknown [10.57.78.139]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 886723F694; Sun, 31 Aug 2025 14:30:07 -0700 (PDT) Message-ID: Date: Sun, 31 Aug 2025 22:30:05 +0100 Precedence: bulk X-Mailing-List: linux-pm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1] cpuidle: governors: teo: Special-case nohz_full CPUs To: "Rafael J. Wysocki" , Linux PM Cc: Frederic Weisbecker , LKML , Peter Zijlstra References: <2804546.mvXUDI8C0e@rafael.j.wysocki> <5939372.DvuYhMxLoT@rafael.j.wysocki> Content-Language: en-US From: Christian Loehle In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 8/29/25 20:37, Rafael J. Wysocki wrote: > On Thu, Aug 28, 2025 at 10:16 PM Rafael J. Wysocki wrote: >> >> From: Rafael J. Wysocki >> >> This change follows an analogous modification of the menu governor [1]. >> >> Namely, when the governor runs on a nohz_full CPU and there are no user >> space timers in the workload on that CPU, it ends up selecting idle >> states with target residency values above TICK_NSEC, or the deepest >> enabled idle state in the absence of any of those, all the time due to >> a tick_nohz_tick_stopped() check designed for running on CPUs where the >> tick is not permanently disabled. In that case, the fact that the tick >> has been stopped means that the CPU was expected to be idle sufficiently >> long previously, so it is not unreasonable to expect it to be idle >> sufficiently long again, but this inference does not apply to nohz_full >> CPUs. >> >> In some cases, latency in the workload grows undesirably as a result of >> selecting overly deep idle states, and the workload may also consume >> more energy than necessary if the CPU does not spend enough time in the >> selected deep idle state. >> >> Address this by amending the tick_nohz_tick_stopped() check in question >> with a tick_nohz_full_cpu() one to avoid effectively ignoring all >> shallow idle states on nohz_full CPUs. While doing so introduces a risk >> of getting stuck in a shallow idle state for a long time, that only >> affects energy efficiently, but the current behavior potentially hurts >> both energy efficiency and performance that is arguably the priority for >> nohz_full CPUs. > > This change is likely to break the use case in which CPU isolation is > used for power management reasons, to prevent CPUs from running any > code and so to save energy. > > In that case, going into the deepest state every time on nohz_full > CPUs is a feature, so it can't be changed unconditionally. > > For this reason, I'm not going to apply this patch and I'm going to > drop the menu governor one below. > > The only way to allow everyone to do what they want/need I can see > would be to add a control knob for adjusting the behavior of cpuidle > governors regarding the handling of nohz_full CPUs. But then what's the advantage instead of just using /sys/devices/system/cpu/cpuX/power/latency for the nohz_full CPUs (if you don't want the current 'over-eagerly selecting deepest state on nohz_full')?