All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools: cpufreq: Fix cpufreq-info print messages for affected[related]_cpus
@ 2013-03-29 14:26 Viresh Kumar
  2013-03-29 21:40 ` Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Viresh Kumar @ 2013-03-29 14:26 UTC (permalink / raw)
  To: rjw, Dominik Brodowski
  Cc: cpufreq, linux-pm, linux-kernel, linaro-kernel, patches,
	arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin, Viresh Kumar

Earlier definitions of affected and related cpus were:
Related_cpus: CPUs which run at the same hardware frequency.
Affected_cpus: CPUs which need to have their frequency coordinated by software.

These definitions were very confusing as they don't communicate the real
difference between them.

Following are the new definitions of these variables:
Related_cpus: All (Online & Offline) CPUs that run at the same hardware frequency.
Affected_cpus: Online CPUs that run at the same hardware frequency.

Above definitions are more consistent with latest cpufreq core code.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 tools/power/cpupower/utils/cpufreq-info.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
index 28953c9..a81d4ec 100644
--- a/tools/power/cpupower/utils/cpufreq-info.c
+++ b/tools/power/cpupower/utils/cpufreq-info.c
@@ -247,7 +247,7 @@ static void debug_output_one(unsigned int cpu)
 
 	cpus = cpufreq_get_related_cpus(cpu);
 	if (cpus) {
-		printf(_("  CPUs which run at the same hardware frequency: "));
+		printf(_("  All (Online & Offline) CPUs that run at the same hardware frequency: "));
 		while (cpus->next) {
 			printf("%d ", cpus->cpu);
 			cpus = cpus->next;
@@ -258,7 +258,7 @@ static void debug_output_one(unsigned int cpu)
 
 	cpus = cpufreq_get_affected_cpus(cpu);
 	if (cpus) {
-		printf(_("  CPUs which need to have their frequency coordinated by software: "));
+		printf(_("  Online CPUs that run at the same hardware frequency: "));
 		while (cpus->next) {
 			printf("%d ", cpus->cpu);
 			cpus = cpus->next;
-- 
1.7.12.rc2.18.g61b472e


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools: cpufreq: Fix cpufreq-info print messages for affected[related]_cpus
  2013-03-29 14:26 [PATCH] tools: cpufreq: Fix cpufreq-info print messages for affected[related]_cpus Viresh Kumar
@ 2013-03-29 21:40 ` Rafael J. Wysocki
  2013-04-02 12:04     ` Thomas Renninger
  0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2013-03-29 21:40 UTC (permalink / raw)
  To: Viresh Kumar, cpufreq, linaro-kernel
  Cc: linux-pm, patches, arvind.chauhan, robin.randhawa,
	Steve.Bannister, Liviu.Dudau, charles.garcia-tobin,
	Thomas Renninger

On Friday, March 29, 2013 07:56:39 PM Viresh Kumar wrote:
> Earlier definitions of affected and related cpus were:
> Related_cpus: CPUs which run at the same hardware frequency.
> Affected_cpus: CPUs which need to have their frequency coordinated by software.
> 
> These definitions were very confusing as they don't communicate the real
> difference between them.
> 
> Following are the new definitions of these variables:
> Related_cpus: All (Online & Offline) CPUs that run at the same hardware frequency.
> Affected_cpus: Online CPUs that run at the same hardware frequency.
> 
> Above definitions are more consistent with latest cpufreq core code.
> 
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

Thomas Renninger is maintaining cpupower nowadays (added to CC).  I won't get
any cpupower changes without his ACK.

Thanks,
Rafael


> ---
>  tools/power/cpupower/utils/cpufreq-info.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/power/cpupower/utils/cpufreq-info.c b/tools/power/cpupower/utils/cpufreq-info.c
> index 28953c9..a81d4ec 100644
> --- a/tools/power/cpupower/utils/cpufreq-info.c
> +++ b/tools/power/cpupower/utils/cpufreq-info.c
> @@ -247,7 +247,7 @@ static void debug_output_one(unsigned int cpu)
>  
>  	cpus = cpufreq_get_related_cpus(cpu);
>  	if (cpus) {
> -		printf(_("  CPUs which run at the same hardware frequency: "));
> +		printf(_("  All (Online & Offline) CPUs that run at the same hardware frequency: "));
>  		while (cpus->next) {
>  			printf("%d ", cpus->cpu);
>  			cpus = cpus->next;
> @@ -258,7 +258,7 @@ static void debug_output_one(unsigned int cpu)
>  
>  	cpus = cpufreq_get_affected_cpus(cpu);
>  	if (cpus) {
> -		printf(_("  CPUs which need to have their frequency coordinated by software: "));
> +		printf(_("  Online CPUs that run at the same hardware frequency: "));
>  		while (cpus->next) {
>  			printf("%d ", cpus->cpu);
>  			cpus = cpus->next;
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools: cpufreq: Fix cpufreq-info print messages for affected[related]_cpus
  2013-03-29 21:40 ` Rafael J. Wysocki
@ 2013-04-02 12:04     ` Thomas Renninger
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Renninger @ 2013-04-02 12:04 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Viresh Kumar, cpufreq, linaro-kernel, linux-pm, patches,
	arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin

On Friday, March 29, 2013 10:40:38 PM Rafael J. Wysocki wrote:
> On Friday, March 29, 2013 07:56:39 PM Viresh Kumar wrote:
> > Earlier definitions of affected and related cpus were:
> > Related_cpus: CPUs which run at the same hardware frequency.
> > Affected_cpus: CPUs which need to have their frequency coordinated by
> > software.
> > 
> > These definitions were very confusing as they don't communicate the real
> > difference between them.
> > 
> > Following are the new definitions of these variables:
> > Related_cpus: All (Online & Offline) CPUs that run at the same hardware
> > frequency. Affected_cpus: Online CPUs that run at the same hardware
> > frequency.
> > 
> > Above definitions are more consistent with latest cpufreq core code.
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> 
> Thomas Renninger is maintaining cpupower nowadays (added to CC).  I won't
> get any cpupower changes without his ACK.
> 
> Thanks,
> Rafael
> 
> > ---
> > 
> >  tools/power/cpupower/utils/cpufreq-info.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/power/cpupower/utils/cpufreq-info.c
> > b/tools/power/cpupower/utils/cpufreq-info.c index 28953c9..a81d4ec 100644
> > --- a/tools/power/cpupower/utils/cpufreq-info.c
> > +++ b/tools/power/cpupower/utils/cpufreq-info.c
> > @@ -247,7 +247,7 @@ static void debug_output_one(unsigned int cpu)
> > 
> >  	cpus = cpufreq_get_related_cpus(cpu);
> >  	if (cpus) {
> > 
> > -		printf(_("  CPUs which run at the same hardware frequency: "));
> > +		printf(_("  All (Online & Offline) CPUs that run at the same hardware frequency: "));
This one is not worth changing IMO, in the end it tells the user more or less the same and
as this stuff is translated, I'd not change it.
> >  		while (cpus->next) {
> >  		
> >  			printf("%d ", cpus->cpu);
> >  			cpus = cpus->next;
> > 
> > @@ -258,7 +258,7 @@ static void debug_output_one(unsigned int cpu)
> > 
> >  	cpus = cpufreq_get_affected_cpus(cpu);
> >  	if (cpus) {
> > 
> > -		printf(_("  CPUs which need to have their frequency coordinated by software: "));
> > +		printf(_("  Online CPUs that run at the same hardware frequency: "));
> >  		while (cpus->next) {
I agree that this message is more developer than user oriented,
but cpupower is more for the end-user. So this message is not perfect.

Checking the manpage which should also get adjusted, I found another bug:

       -a --related-cpus
              Determines which CPUs run at the same hardware frequency.

       -a --affected-cpus
              Determines  which  CPUs need to have their frequency coordinated
              by software.

It must be:
       -r --related-cpus

>From what I can see of current code with patch aa77a52764a92216b61a6c8079b5c01937c046cd
all related_cpus users are gone and related-cpus does not have any meaning at all
anymore?
I haven't gone through your latest changes, but will at least give them a test on
a AMD K10 multi socket machine which iirc where using related_cpus.
I try to catch up with latest cpufreq changes as well, but wow... no idea when this
will happen.

For now I would just leave it (cpupower messages/manpage) as it is, there is
nothing critical which must get fixed immediately.

      Thomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools: cpufreq: Fix cpufreq-info print messages for affected[related]_cpus
@ 2013-04-02 12:04     ` Thomas Renninger
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Renninger @ 2013-04-02 12:04 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Viresh Kumar, cpufreq, linaro-kernel, linux-pm, patches,
	arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin

On Friday, March 29, 2013 10:40:38 PM Rafael J. Wysocki wrote:
> On Friday, March 29, 2013 07:56:39 PM Viresh Kumar wrote:
> > Earlier definitions of affected and related cpus were:
> > Related_cpus: CPUs which run at the same hardware frequency.
> > Affected_cpus: CPUs which need to have their frequency coordinated by
> > software.
> > 
> > These definitions were very confusing as they don't communicate the real
> > difference between them.
> > 
> > Following are the new definitions of these variables:
> > Related_cpus: All (Online & Offline) CPUs that run at the same hardware
> > frequency. Affected_cpus: Online CPUs that run at the same hardware
> > frequency.
> > 
> > Above definitions are more consistent with latest cpufreq core code.
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> 
> Thomas Renninger is maintaining cpupower nowadays (added to CC).  I won't
> get any cpupower changes without his ACK.
> 
> Thanks,
> Rafael
> 
> > ---
> > 
> >  tools/power/cpupower/utils/cpufreq-info.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/power/cpupower/utils/cpufreq-info.c
> > b/tools/power/cpupower/utils/cpufreq-info.c index 28953c9..a81d4ec 100644
> > --- a/tools/power/cpupower/utils/cpufreq-info.c
> > +++ b/tools/power/cpupower/utils/cpufreq-info.c
> > @@ -247,7 +247,7 @@ static void debug_output_one(unsigned int cpu)
> > 
> >  	cpus = cpufreq_get_related_cpus(cpu);
> >  	if (cpus) {
> > 
> > -		printf(_("  CPUs which run at the same hardware frequency: "));
> > +		printf(_("  All (Online & Offline) CPUs that run at the same hardware frequency: "));
This one is not worth changing IMO, in the end it tells the user more or less the same and
as this stuff is translated, I'd not change it.
> >  		while (cpus->next) {
> >  		
> >  			printf("%d ", cpus->cpu);
> >  			cpus = cpus->next;
> > 
> > @@ -258,7 +258,7 @@ static void debug_output_one(unsigned int cpu)
> > 
> >  	cpus = cpufreq_get_affected_cpus(cpu);
> >  	if (cpus) {
> > 
> > -		printf(_("  CPUs which need to have their frequency coordinated by software: "));
> > +		printf(_("  Online CPUs that run at the same hardware frequency: "));
> >  		while (cpus->next) {
I agree that this message is more developer than user oriented,
but cpupower is more for the end-user. So this message is not perfect.

Checking the manpage which should also get adjusted, I found another bug:

       -a --related-cpus
              Determines which CPUs run at the same hardware frequency.

       -a --affected-cpus
              Determines  which  CPUs need to have their frequency coordinated
              by software.

It must be:
       -r --related-cpus

From what I can see of current code with patch aa77a52764a92216b61a6c8079b5c01937c046cd
all related_cpus users are gone and related-cpus does not have any meaning at all
anymore?
I haven't gone through your latest changes, but will at least give them a test on
a AMD K10 multi socket machine which iirc where using related_cpus.
I try to catch up with latest cpufreq changes as well, but wow... no idea when this
will happen.

For now I would just leave it (cpupower messages/manpage) as it is, there is
nothing critical which must get fixed immediately.

      Thomas

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools: cpufreq: Fix cpufreq-info print messages for affected[related]_cpus
  2013-04-02 12:04     ` Thomas Renninger
  (?)
@ 2013-04-02 12:39     ` Rafael J. Wysocki
  -1 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2013-04-02 12:39 UTC (permalink / raw)
  To: Thomas Renninger
  Cc: Viresh Kumar, cpufreq, linaro-kernel, linux-pm, patches,
	arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin

On Tuesday, April 02, 2013 02:04:04 PM Thomas Renninger wrote:
> On Friday, March 29, 2013 10:40:38 PM Rafael J. Wysocki wrote:
> > On Friday, March 29, 2013 07:56:39 PM Viresh Kumar wrote:
> > > Earlier definitions of affected and related cpus were:
> > > Related_cpus: CPUs which run at the same hardware frequency.
> > > Affected_cpus: CPUs which need to have their frequency coordinated by
> > > software.
> > > 
> > > These definitions were very confusing as they don't communicate the real
> > > difference between them.
> > > 
> > > Following are the new definitions of these variables:
> > > Related_cpus: All (Online & Offline) CPUs that run at the same hardware
> > > frequency. Affected_cpus: Online CPUs that run at the same hardware
> > > frequency.
> > > 
> > > Above definitions are more consistent with latest cpufreq core code.
> > > 
> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > 
> > Thomas Renninger is maintaining cpupower nowadays (added to CC).  I won't
> > get any cpupower changes without his ACK.
> > 
> > Thanks,
> > Rafael
> > 
> > > ---
> > > 
> > >  tools/power/cpupower/utils/cpufreq-info.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/power/cpupower/utils/cpufreq-info.c
> > > b/tools/power/cpupower/utils/cpufreq-info.c index 28953c9..a81d4ec 100644
> > > --- a/tools/power/cpupower/utils/cpufreq-info.c
> > > +++ b/tools/power/cpupower/utils/cpufreq-info.c
> > > @@ -247,7 +247,7 @@ static void debug_output_one(unsigned int cpu)
> > > 
> > >  	cpus = cpufreq_get_related_cpus(cpu);
> > >  	if (cpus) {
> > > 
> > > -		printf(_("  CPUs which run at the same hardware frequency: "));
> > > +		printf(_("  All (Online & Offline) CPUs that run at the same hardware frequency: "));
> This one is not worth changing IMO, in the end it tells the user more or less the same and
> as this stuff is translated, I'd not change it.
> > >  		while (cpus->next) {
> > >  		
> > >  			printf("%d ", cpus->cpu);
> > >  			cpus = cpus->next;
> > > 
> > > @@ -258,7 +258,7 @@ static void debug_output_one(unsigned int cpu)
> > > 
> > >  	cpus = cpufreq_get_affected_cpus(cpu);
> > >  	if (cpus) {
> > > 
> > > -		printf(_("  CPUs which need to have their frequency coordinated by software: "));
> > > +		printf(_("  Online CPUs that run at the same hardware frequency: "));
> > >  		while (cpus->next) {
> I agree that this message is more developer than user oriented,
> but cpupower is more for the end-user. So this message is not perfect.
> 
> Checking the manpage which should also get adjusted, I found another bug:
> 
>        -a --related-cpus
>               Determines which CPUs run at the same hardware frequency.
> 
>        -a --affected-cpus
>               Determines  which  CPUs need to have their frequency coordinated
>               by software.
> 
> It must be:
>        -r --related-cpus
> 
> From what I can see of current code with patch aa77a52764a92216b61a6c8079b5c01937c046cd
> all related_cpus users are gone and related-cpus does not have any meaning at all
> anymore?
> I haven't gone through your latest changes, but will at least give them a test on
> a AMD K10 multi socket machine which iirc where using related_cpus.
> I try to catch up with latest cpufreq changes as well, but wow... no idea when this
> will happen.
> 
> For now I would just leave it (cpupower messages/manpage) as it is, there is
> nothing critical which must get fixed immediately.

OK

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] tools: cpufreq: Fix cpufreq-info print messages for affected[related]_cpus
  2013-04-02 12:04     ` Thomas Renninger
  (?)
  (?)
@ 2013-04-02 13:08     ` Viresh Kumar
  -1 siblings, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2013-04-02 13:08 UTC (permalink / raw)
  To: Thomas Renninger
  Cc: Rafael J. Wysocki, cpufreq, linaro-kernel, linux-pm, patches,
	arvind.chauhan, robin.randhawa, Steve.Bannister, Liviu.Dudau,
	charles.garcia-tobin

On 2 April 2013 17:34, Thomas Renninger <trenn@suse.de> wrote:
> On Friday, March 29, 2013 10:40:38 PM Rafael J. Wysocki wrote:
>> On Friday, March 29, 2013 07:56:39 PM Viresh Kumar wrote:
>> > -           printf(_("  CPUs which run at the same hardware frequency: "));
>> > +           printf(_("  All (Online & Offline) CPUs that run at the same hardware frequency: "));
> This one is not worth changing IMO, in the end it tells the user more or less the same and
> as this stuff is translated, I'd not change it.

Okay.

>> > -           printf(_("  CPUs which need to have their frequency coordinated by software: "));
>> > +           printf(_("  Online CPUs that run at the same hardware frequency: "));
>> >             while (cpus->next) {
> I agree that this message is more developer than user oriented,
> but cpupower is more for the end-user. So this message is not perfect.

> From what I can see of current code with patch aa77a52764a92216b61a6c8079b5c01937c046cd
> all related_cpus users are gone and related-cpus does not have any meaning at all
> anymore?

No, that's wrong. We need to set policy->cpus correctly (with online +
offline cpus) and cpufreq core will take care of setting related_cpus
with everything
from policy->cpus and policy->cpus will be modified to keep only online cpus.

> I haven't gone through your latest changes, but will at least give them a test on
> a AMD K10 multi socket machine which iirc where using related_cpus.
> I try to catch up with latest cpufreq changes as well, but wow... no idea when this
> will happen.

:)

> For now I would just leave it (cpupower messages/manpage) as it is, there is
> nothing critical which must get fixed immediately.

Yes its not really critical but it must be fixed to reflect the right stuff.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-04-02 13:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-29 14:26 [PATCH] tools: cpufreq: Fix cpufreq-info print messages for affected[related]_cpus Viresh Kumar
2013-03-29 21:40 ` Rafael J. Wysocki
2013-04-02 12:04   ` Thomas Renninger
2013-04-02 12:04     ` Thomas Renninger
2013-04-02 12:39     ` Rafael J. Wysocki
2013-04-02 13:08     ` Viresh Kumar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.