Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
From: Abhishek <huntbag@linux.vnet.ibm.com>
To: Thomas Renninger <trenn@suse.de>
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	shuah@kernel.org, "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
Subject: Re: [PATCH] cpupower : Handle set and info subcommands for powerpc
Date: Thu, 12 Sep 2019 15:49:19 +0530
Message-ID: <754e96b7-19da-1a59-25bc-390afc9da00f@linux.vnet.ibm.com> (raw)
In-Reply-To: <12087195.kFesu4gPPu@skinner.arch.suse.de>

Hi Thomas,

Thanks for the review.


On 09/12/2019 03:24 PM, Thomas Renninger wrote:
> Hi Abishek,
>
> On Wednesday, September 11, 2019 11:54:24 AM CEST Abhishek Goel wrote:
>> Cpupower tool has set and info options which are not being used by
>> POWER machines. For powerpc, we will return directly for these two
>> subcommands. This removes the ambiguous error message while using set
>> option in case of power systems.
>>
>> Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
>> ---
>>   tools/power/cpupower/utils/cpupower-info.c | 5 +++++
>>   tools/power/cpupower/utils/cpupower-set.c  | 5 +++++
>>   2 files changed, 10 insertions(+)
>>
>> diff --git a/tools/power/cpupower/utils/cpupower-info.c
>> b/tools/power/cpupower/utils/cpupower-info.c index
>> 4c9d342b70ff..674b707a76af 100644
>> --- a/tools/power/cpupower/utils/cpupower-info.c
>> +++ b/tools/power/cpupower/utils/cpupower-info.c
>> @@ -39,6 +39,11 @@ int cmd_info(int argc, char **argv)
>>   	} params = {};
>>   	int ret = 0;
>>
>> +	#ifdef __powerpc__
>> +	printf(_("Cannot read info as system does not support performance bias
>> setting\n")); +	return 0;
>> +	#endif
>> +
> Please do no do this.
>
> cpupower info
> is designed to show general information related to powersaving features of your CPU.
>
> For examle there has been (see changelog):
> cpupower: Remove mc and smt power aware scheduler info/settings
> These kernel interfaces got removed by:
>
> Unfortunately only -b (perf bias on Intel only) is left right now.
>
> So if you cut this out for Power you do not see anything and the cmd is useless.
> Which is a pity, but for now makes sense.
> Ideally you provide some tag/option which makes sense on power (e.g. whether run
> in OPAL mode and if provide some figures otherwise tell running in VM mode).
> But if this is cut out something like this should do the same and is more flexible:
> - Still allows additional cpupower info features for other CPUs later easily
> - Should also cover AMD or other non-perf bias supporting CPUs to exclude perf_bias
>    setting/info

As I have suggested in here : https://lkml.org/lkml/2019/9/12/159
We should cut out these two options as of now for other architecture
except x86, since the current implementation of both these subcommands
are very intel specific.
As you have already suggested, we can later on maybe change the
documentation of info to be architecture specific and use info based on
arch requirement.

>
> If this one works for you, can you please re-submit with also handling the set cmd
> similar. If it works or you only slightly adjust, feel free to already add:
> Acked-by: Thomas Renninger <trenn@suse.de>

Yeah. Sure.

> Thanks!
>
>         Thomas
>
> --- tools/power/cpupower/utils/cpupower-info.c.orig	2019-09-12 11:45:02.578568335 +0200
> +++ tools/power/cpupower/utils/cpupower-info.c	2019-09-12 11:46:09.618571947 +0200
> @@ -55,8 +55,11 @@
>   		}
>   	};
>
> -	if (!params.params)
> +	if (!params.params) {
>   		params.params = 0x7;
> +		if !(cpupower_cpu_info.caps & CPUPOWER_CAP_PERF_BIAS)
> +			params.perf_bias = 0;
> +	}
>
>   	/* Default is: show output of CPU 0 only */
>   	if (bitmask_isallclear(cpus_chosen))
>
>
>

-- Abhishek


      reply index

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190911095424.49605-1-huntbag@linux.vnet.ibm.com>
2019-09-11 10:11 ` shuah
2019-09-12  9:43   ` Abhishek
2019-09-12 10:16     ` Thomas Renninger
2019-09-12 10:23       ` Abhishek
2019-09-12  9:54 ` Thomas Renninger
2019-09-12 10:19   ` Abhishek [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=754e96b7-19da-1a59-25bc-390afc9da00f@linux.vnet.ibm.com \
    --to=huntbag@linux.vnet.ibm.com \
    --cc=ego@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=shuah@kernel.org \
    --cc=trenn@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org linux-pm@archiver.kernel.org
	public-inbox-index linux-pm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox