Message ID | 12138067.O9o76ZdvQC@kreacher |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp1556985wrt; Tue, 27 Dec 2022 12:00:50 -0800 (PST) X-Google-Smtp-Source: AMrXdXvsf0BTi2aVDiE91Xz4F4XOGx6AAvWacWhcf7/kp9SrEamMNCn7XPDbq4t/RuDutztOi3s3 X-Received: by 2002:a17:902:d303:b0:189:b3bf:c0b5 with SMTP id b3-20020a170902d30300b00189b3bfc0b5mr24952491plc.34.1672171250603; Tue, 27 Dec 2022 12:00:50 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1672171250; cv=none; d=google.com; s=arc-20160816; b=RDyw+wjTAAav8eE1vgAiponS+zS7gldjZXYY2OBcmT+9fx2uCXXYKJGvu8BC7GBNZ9 NOJRaDA9on9+wCaXd2KamC2RcoM8xxeARrFcsIE7MEzA2tGUmTtlbiVndPw2AQKe3uvR b1OSym2S9hxHjkHzEWFLLjSqU2J96kUnM7vZaHIklu1efeqeA5RJNvS7eNDVBiN+xDjH SHA/Yvkg+dcDpN8LWEHT7oibZ5/OsryCnt837zh1IS9CcmM7c3/ZlAYwfNMzfuC1uDfi DhjRivZbEAYsWECk/cLBJRAArgIDkWjpfTqQg8+OMJCmxrNItWM0CCxTfh8v94z/yfki eYNg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=jbobXY0IufRhIjMjqgH6VvroHlzOP8CSLt/5noAp9bs=; b=ZCtamjWCgTIGrD4yjnFgED2FavzB+k8T40ZHpfRfmiKoDSgdyG+b6pHnQXRslcS6e+ GB7cAtPspERjKY8iSGeOxAsN3JdratK3s7Lc+O5UhWv9gHyms4fpZwAANS3zZtIJHv/A oP3ndlJyOJukwsVqWO+mCWaTzMYeCnMYNapFWU9jWYMFWb3gKjaIYpMDKmRCdfqpninV Qq86fjB7C/pgn/XFo+4u92QGz/O8K4AbbEBYPpNoLFnX/g6qHjLeBvpnu8ShfRjJYTcF +9AYcHJ1ZAHUfmPMFPxtarjxQtBZf2kqHQmqEOJxNY3FDx+CdSC2KpYbZ5kEEHOayTd/ Wv4A== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id c3-20020a170903234300b0018659963cdcsi15718760plh.514.2022.12.27.12.00.38; Tue, 27 Dec 2022 12:00:50 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231749AbiL0Tx6 (ORCPT <rfc822;eddaouddi.ayoub@gmail.com> + 99 others); Tue, 27 Dec 2022 14:53:58 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55510 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229788AbiL0Tx5 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 27 Dec 2022 14:53:57 -0500 Received: from cloudserver094114.home.pl (cloudserver094114.home.pl [79.96.170.134]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A23E51131; Tue, 27 Dec 2022 11:53:55 -0800 (PST) Received: from localhost (127.0.0.1) (HELO v370.home.net.pl) by /usr/run/smtp (/usr/run/postfix/private/idea_relay_lmtp) via UNIX with SMTP (IdeaSmtpServer 5.1.0) id f43143bd2e72e6b3; Tue, 27 Dec 2022 20:53:53 +0100 Received: from kreacher.localnet (unknown [213.134.183.5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by v370.home.net.pl (Postfix) with ESMTPSA id 3FA3E28A34C8; Tue, 27 Dec 2022 20:53:53 +0100 (CET) Authentication-Results: v370.home.net.pl; dmarc=none (p=none dis=none) header.from=rjwysocki.net Authentication-Results: v370.home.net.pl; spf=fail smtp.mailfrom=rjwysocki.net From: "Rafael J. Wysocki" <rjw@rjwysocki.net> To: Linux PM <linux-pm@vger.kernel.org> Cc: Pratyush Yadav <ptyadav@amazon.de>, LKML <linux-kernel@vger.kernel.org>, Linux ACPI <linux-acpi@vger.kernel.org>, Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> Subject: [PATCH v1 1/2] ACPI: processor: perflib: Use the "no limit" frequency QoS Date: Tue, 27 Dec 2022 20:51:43 +0100 Message-ID: <12138067.O9o76ZdvQC@kreacher> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="UTF-8" X-CLIENT-IP: 213.134.183.5 X-CLIENT-HOSTNAME: 213.134.183.5 X-VADE-SPAMSTATE: clean X-VADE-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedvhedriedtgddufeduucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecujffqoffgrffnpdggtffipffknecuuegrihhlohhuthemucduhedtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjughrpefhvfevufffkfgggfgtsehtufertddttdejnecuhfhrohhmpedftfgrfhgrvghlucflrdcuhgihshhotghkihdfuceorhhjfiesrhhjfiihshhotghkihdrnhgvtheqnecuggftrfgrthhtvghrnhepffffffekgfehheffleetieevfeefvefhleetjedvvdeijeejledvieehueevueffnecukfhppedvudefrddufeegrddukeefrdehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepvddufedrudefgedrudekfedrhedphhgvlhhopehkrhgvrggthhgvrhdrlhhotggrlhhnvghtpdhmrghilhhfrhhomhepfdftrghfrggvlhculfdrucghhihsohgtkhhifdcuoehrjhifsehrjhifhihsohgtkhhirdhnvghtqedpnhgspghrtghpthhtohephedprhgtphhtthhopehlihhnuhigqdhpmhesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehpthihrggurghvsegrmhgriihonhdruggvpdhrtghpthhtoheplhhinhhugidqkhgvrhhnvghlsehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtoheplhhinhhugidqrggtphhisehvghgvrhdrkhgvrhhnvghlrdhorhhgpdhrtghpthhtohepshhrihhnihhv rghsrdhprghnughruhhvrggurgeslhhinhhugidrihhnthgvlhdrtghomh X-DCC--Metrics: v370.home.net.pl 1024; Body=5 Fuz1=5 Fuz2=5 X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1753398641352408968?= X-GMAIL-MSGID: =?utf-8?q?1753398641352408968?= |
Series |
[v1,1/2] ACPI: processor: perflib: Use the "no limit" frequency QoS
|
|
Commit Message
Rafael J. Wysocki
Dec. 27, 2022, 7:51 p.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> When _PPC returns 0, it means that the CPU frequency is not limited by the platform firmware, so make acpi_processor_get_platform_limit() update the frequency QoS request used by it to "no limit" in that case and avoid updating the QoS request when the _PPC return value has not changed. This addresses a problem with limiting CPU frequency artificially on some systems after CPU offline/online to the frequency that corresponds to the first entry in the _PSS return package. While at it, move the _PPC return value check against the state count earlier to avoid setting performance_platform_limit to an invalid value. Reported-by: Pratyush Yadav <ptyadav@amazon.de> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/acpi/processor_perflib.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-)
Comments
On Tue, 2022-12-27 at 20:51 +0100, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > When _PPC returns 0, it means that the CPU frequency is not limited > by > the platform firmware, so make acpi_processor_get_platform_limit() > update the frequency QoS request used by it to "no limit" in that > case > and avoid updating the QoS request when the _PPC return value has not > changed. > > This addresses a problem with limiting CPU frequency artificially on > some systems after CPU offline/online to the frequency that > corresponds > to the first entry in the _PSS return package. > > While at it, move the _PPC return value check against the state count > earlier to avoid setting performance_platform_limit to an invalid > value. > > Reported-by: Pratyush Yadav <ptyadav@amazon.de> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/processor_perflib.c | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > Index: linux-pm/drivers/acpi/processor_perflib.c > =================================================================== > --- linux-pm.orig/drivers/acpi/processor_perflib.c > +++ linux-pm/drivers/acpi/processor_perflib.c > @@ -53,6 +53,8 @@ static int acpi_processor_get_platform_l > { > acpi_status status = 0; > unsigned long long ppc = 0; > + s32 qos_value; > + int index; > int ret; > > if (!pr) > @@ -72,17 +74,30 @@ static int acpi_processor_get_platform_l > } > } > > + index = ppc; > + > + if (pr->performance_platform_limit == index || > + ppc >= pr->performance->state_count) > + return 0; Do we need to re initialize pr->performance_platform_limit to 0 in acpi_processor_unregister_performance()? If PPC was 1 before the offline and after online the above check will cause it to return as the pr->performance_platform_limit is not changed. Not sure if the PM QOS state is preserved after offline and online. This is stored in a per CPU variable, not in dynamically allocated memory which will be reallocated during online again. Thanks, Srinivas > + > pr_debug("CPU %d: _PPC is %d - frequency %s limited\n", pr- > >id, > - (int)ppc, ppc ? "" : "not"); > + index, index ? "is" : "is not"); > > - pr->performance_platform_limit = (int)ppc; > + pr->performance_platform_limit = index; > > - if (ppc >= pr->performance->state_count || > - unlikely(!freq_qos_request_active(&pr->perflib_req))) > + if (unlikely(!freq_qos_request_active(&pr->perflib_req))) > return 0; > > - ret = freq_qos_update_request(&pr->perflib_req, > - pr->performance->states[ppc].core_frequency * > 1000); > + /* > + * If _PPC returns 0, it means that all of the available > states can be > + * used ("no limit"). > + */ > + if (index == 0) > + qos_value = FREQ_QOS_MAX_DEFAULT_VALUE; > + else > + qos_value = pr->performance- > >states[index].core_frequency * 1000; > + > + ret = freq_qos_update_request(&pr->perflib_req, qos_value); > if (ret < 0) { > pr_warn("Failed to update perflib freq constraint: > CPU%d (%d)\n", > pr->id, ret); > > >
On Tue, Dec 27 2022, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > When _PPC returns 0, it means that the CPU frequency is not limited by > the platform firmware, so make acpi_processor_get_platform_limit() > update the frequency QoS request used by it to "no limit" in that case > and avoid updating the QoS request when the _PPC return value has not > changed. > > This addresses a problem with limiting CPU frequency artificially on > some systems after CPU offline/online to the frequency that corresponds > to the first entry in the _PSS return package. > > While at it, move the _PPC return value check against the state count > earlier to avoid setting performance_platform_limit to an invalid value. > > Reported-by: Pratyush Yadav <ptyadav@amazon.de> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Tested-by: Pratyush Yadav <ptyadav@amazon.de>
On Tue, Dec 27, 2022 at 9:55 PM srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote: > > On Tue, 2022-12-27 at 20:51 +0100, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > > > When _PPC returns 0, it means that the CPU frequency is not limited > > by > > the platform firmware, so make acpi_processor_get_platform_limit() > > update the frequency QoS request used by it to "no limit" in that > > case > > and avoid updating the QoS request when the _PPC return value has not > > changed. > > > > This addresses a problem with limiting CPU frequency artificially on > > some systems after CPU offline/online to the frequency that > > corresponds > > to the first entry in the _PSS return package. > > > > While at it, move the _PPC return value check against the state count > > earlier to avoid setting performance_platform_limit to an invalid > > value. > > > > Reported-by: Pratyush Yadav <ptyadav@amazon.de> > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > --- > > drivers/acpi/processor_perflib.c | 27 +++++++++++++++++++++------ > > 1 file changed, 21 insertions(+), 6 deletions(-) > > > > Index: linux-pm/drivers/acpi/processor_perflib.c > > =================================================================== > > --- linux-pm.orig/drivers/acpi/processor_perflib.c > > +++ linux-pm/drivers/acpi/processor_perflib.c > > @@ -53,6 +53,8 @@ static int acpi_processor_get_platform_l > > { > > acpi_status status = 0; > > unsigned long long ppc = 0; > > + s32 qos_value; > > + int index; > > int ret; > > > > if (!pr) > > @@ -72,17 +74,30 @@ static int acpi_processor_get_platform_l > > } > > } > > > > + index = ppc; > > + > > + if (pr->performance_platform_limit == index || > > + ppc >= pr->performance->state_count) > > + return 0; > > Do we need to re initialize pr->performance_platform_limit to 0 in > acpi_processor_unregister_performance()? > > If PPC was 1 before the offline and after online the above check will > cause it to return as the pr->performance_platform_limit is not > changed. Not sure if the PM QOS state is preserved after offline and > online. This is stored in a per CPU variable, not in dynamically > allocated memory which will be reallocated during online again. Good point in general, but the QoS request is tied to the cpufreq policy, so it is not freed on offline. However, if the policy goes away and is created again for the same CPU (like when the intel_pstate mode is changed via its 'status' attribute in sysfs), there may be a stale value in performance_platform_limit, so it needs to be cleared in acpi_processor_ppc_init() when the QoS request is first set to "no limit". I'll update the patch accordingly. I think I'll also split it in two to avoid making too many changes in one go.
Index: linux-pm/drivers/acpi/processor_perflib.c =================================================================== --- linux-pm.orig/drivers/acpi/processor_perflib.c +++ linux-pm/drivers/acpi/processor_perflib.c @@ -53,6 +53,8 @@ static int acpi_processor_get_platform_l { acpi_status status = 0; unsigned long long ppc = 0; + s32 qos_value; + int index; int ret; if (!pr) @@ -72,17 +74,30 @@ static int acpi_processor_get_platform_l } } + index = ppc; + + if (pr->performance_platform_limit == index || + ppc >= pr->performance->state_count) + return 0; + pr_debug("CPU %d: _PPC is %d - frequency %s limited\n", pr->id, - (int)ppc, ppc ? "" : "not"); + index, index ? "is" : "is not"); - pr->performance_platform_limit = (int)ppc; + pr->performance_platform_limit = index; - if (ppc >= pr->performance->state_count || - unlikely(!freq_qos_request_active(&pr->perflib_req))) + if (unlikely(!freq_qos_request_active(&pr->perflib_req))) return 0; - ret = freq_qos_update_request(&pr->perflib_req, - pr->performance->states[ppc].core_frequency * 1000); + /* + * If _PPC returns 0, it means that all of the available states can be + * used ("no limit"). + */ + if (index == 0) + qos_value = FREQ_QOS_MAX_DEFAULT_VALUE; + else + qos_value = pr->performance->states[index].core_frequency * 1000; + + ret = freq_qos_update_request(&pr->perflib_req, qos_value); if (ret < 0) { pr_warn("Failed to update perflib freq constraint: CPU%d (%d)\n", pr->id, ret);