Re: [Powertop] [PATCH] abstract_cpu: do not call total_pstate_time() in validate()

Re: [Powertop] [PATCH] abstract_cpu: do not call total_pstate_time() in validate()

[Arjan van de Ven]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]

On 9/14/2012 3:00 AM, Paul Menzel wrote:
> Dear Sergey,
> 
> 
> Am Donnerstag, den 13.09.2012, 15:41 +0300 schrieb Sergey Senozhatsky:
>> abstract_cpu: do not call total_pstate_time() in validate()
> 
> copying the commit summary into the commit message body is not useful.

you do need to do that... it needs to be part of the git commit

> 
> Maybe also describe why this was a problem.

having more than the summary is often useful yes ;-)

Re: [Powertop] [PATCH] abstract_cpu: do not call total_pstate_time() in validate()

[Sergey Senozhatsky]

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

On (09/14/12 12:00), Paul Menzel wrote:
> 
> Dear Sergey,
> 
> Am Donnerstag, den 13.09.2012, 15:41 +0300 schrieb Sergey Senozhatsky:
> > abstract_cpu: do not call total_pstate_time() in validate()
> 
> copying the commit summary into the commit message body is not useful.
> 
> Maybe also describe why this was a problem.
>

while I agree, this commit was not fixing any real problem (perhaps, meager
performance impact), but just cleaned up abstract_cpu part after
f77ea5fe2ad46b3c5de46b8447d23bd12958d2c8 commit:

-                       if (my_time != children[i]->total_pstate_time())
-                               printf("My (%i) time %llu is not the same as child (%i) time %llu\n",
-                                       first_cpu,
-                                       (unsigned long long)my_time,
-                                       children[i]->number,
-                                       (unsigned long long)children[i]->total_pstate_time());

for such trivial commits I usually cut corners since commit does what the summary says.
may be I should be more accurate.


	-ss

Re: [Powertop] [PATCH] abstract_cpu: do not call total_pstate_time() in validate()

[Paul Menzel]

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]

Dear Sergey,


Am Donnerstag, den 13.09.2012, 15:41 +0300 schrieb Sergey Senozhatsky:
> abstract_cpu: do not call total_pstate_time() in validate()

copying the commit summary into the commit message body is not useful.

Maybe also describe why this was a problem.

> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>
> 
> ---
> 
>  src/cpu/abstract_cpu.cpp | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)

[…]


Thanks,

Paul

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [Powertop] [PATCH] abstract_cpu: do not call total_pstate_time() in validate()

[Chris Ferron]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On 09/13/2012 05:41 AM, Sergey Senozhatsky wrote:
>   abstract_cpu: do not call total_pstate_time() in validate()
>
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>
>
> ---
>
>   src/cpu/abstract_cpu.cpp | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
> index ebdc510..386d981 100644
> --- a/src/cpu/abstract_cpu.cpp
> +++ b/src/cpu/abstract_cpu.cpp
> @@ -414,15 +414,10 @@ uint64_t abstract_cpu::total_pstate_time(void)
>   void abstract_cpu::validate(void)
>   {
>   	unsigned int i;
> -	uint64_t my_time;
> -
> -	my_time = total_pstate_time();
>   
>   	for (i = 0; i < children.size(); i++) {
> -
> -		if (children[i]) {
> +		if (children[i])
>   			children[i]->validate();
> -		}
>   	}
>   }
>   
Your patch has been applied.
Thank You
-C

[Powertop] [PATCH] abstract_cpu: do not call total_pstate_time() in validate()

[Sergey Senozhatsky]

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

 abstract_cpu: do not call total_pstate_time() in validate()

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky(a)gmail.com>

---

 src/cpu/abstract_cpu.cpp | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/cpu/abstract_cpu.cpp b/src/cpu/abstract_cpu.cpp
index ebdc510..386d981 100644
--- a/src/cpu/abstract_cpu.cpp
+++ b/src/cpu/abstract_cpu.cpp
@@ -414,15 +414,10 @@ uint64_t abstract_cpu::total_pstate_time(void)
 void abstract_cpu::validate(void)
 {
 	unsigned int i;
-	uint64_t my_time;
-
-	my_time = total_pstate_time();
 
 	for (i = 0; i < children.size(); i++) {
-
-		if (children[i]) {
+		if (children[i])
 			children[i]->validate();
-		}
 	}
 }