public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Eric Dumazet <dada1@cosmosbay.com>
To: Maxim Uvarov <muvarov@ru.mvista.com>
Cc: linux-kernel@vger.kernel.org, davidsen@tmr.com,
	randy.dunlap@oracle.com, Valdis.Kletnieks@vt.edu,
	jesper.juhl@gmail.com
Subject: Re: Performance Stats: Kernel patch
Date: Wed, 11 Apr 2007 21:22:46 +0200	[thread overview]
Message-ID: <461D3586.30003@cosmosbay.com> (raw)
In-Reply-To: <461D047B.90800@ru.mvista.com>

Maxim Uvarov a écrit :
> Eric Dumazet wrote:
> 
>  >Please check kernel/sys.c:k_getrusage() to see how getrusage() has to 
> sum *lot* of individual fields to get precise process numbers (even 
> counting stats for dead threads)
> 
> 
> 
> Thanks for helping me and for this link. But it is not enough clear for 
> me what do you mean at this time.  Inside of patch I am using 2 default 
> counters
> task_struct->nivcsw and task_struct->nvcsw. And also one new syscall 
> counter. And there is only one way to increment this counter, it is from 
> entry.S.
> 
> If you are speaking about locks,  in my point of view, they are not 
> needed in this code. Because increment syscall counter is atomic for X86 
> (just one assembly instruction) and in case with PPC (3 instructions) 
> there 1) nothing wrong will not happen in any case 2) only own thread 
> can increase it's syscall counter. So here should be not any race 
> conditions.

I was not speaking about locks.

> 
> I've tested this patch on x86,x86_64,and ppc_32. And I should work now 
> with ppc_64 (I didn't check).
> And  also updated description.
> 
> Best regards,
> Maxim Uvarov.
> 
> 
> ------------------------------------------------------------------------
> 
> Patch adds Process Performance Statistics.
> It make available to the user the following 
> thread performance statistics:
>    * Involuntary Context Switches (task_struct->nivcsw)
>    * Voluntary Context Switches (task_struct->nvcsw)
>    * Number of system calls (added new counter 
>      thread_info->sysc_cnt)
> 
> Statistics information is available from
> /proc/PID/status
>    
> This data is useful for detecting hyperactivity 
> patterns between processes.
> 

What I meant is : You falsely speak of 'PROCESS performance statistics'.

Your implementation only cares about threads, not processes.
There is a slight difference, that getrusage() can do.

So if you do "cat /proc/PID/status", you'll get counters not for the PROCESS, 
only the main thread of the process.

If you want an analogy, imagine a "ps aux" that doesnt show the cpu time of 
all threads of a process, but only the cpu time of the main thread. Quite 
meaningless isnt it ?

So either :

1) You change all your description to mention 'thread' instead of 'process'.

2) You change your implementation to match your claim.


  reply	other threads:[~2007-04-11 19:23 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-09 14:22 Performance Stats: Kernel patch Maxim Uvarov
2007-04-10  8:21 ` Eric Dumazet
2007-04-11 11:59   ` Maxim Uvarov
2007-04-11 12:26     ` Eric Dumazet
2007-04-11 13:15       ` Maxim Uvarov
2007-04-11 14:15     ` Eric Dumazet
2007-04-11 15:33       ` Bill Davidsen
2007-04-11 15:57         ` Maxim Uvarov
2007-04-11 15:53       ` Maxim Uvarov
2007-04-11 19:22         ` Eric Dumazet [this message]
2007-04-12 13:46           ` Maxim Uvarov
2007-04-15  9:47 ` Pavel Machek
2007-04-15 10:21   ` William Lee Irwin III
2007-04-15 20:10     ` Pavel Machek
2007-04-16  1:04       ` William Lee Irwin III
2007-04-16  9:24         ` Maxim Uvarov
  -- strict thread matches above, loose matches on Subject: below --
2007-04-03 12:54 Maxim Uvarov
2007-04-03 23:01 ` Valdis.Kletnieks
2007-04-04 13:15   ` Maxim Uvarov
2007-04-04 13:46     ` Eric Dumazet
2007-04-04 16:52       ` Maxim Uvarov
2007-04-04 18:04         ` Eric Dumazet
2007-04-04 21:54       ` Valdis.Kletnieks
2007-04-04 13:59     ` Jesper Juhl
2007-04-04 21:50     ` Valdis.Kletnieks
2007-04-04 22:03       ` Randy Dunlap
2007-04-06 21:50       ` Bill Davidsen
2007-04-08 16:58     ` Pavel Machek

Reply instructions:

You may reply publicly 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=461D3586.30003@cosmosbay.com \
    --to=dada1@cosmosbay.com \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=davidsen@tmr.com \
    --cc=jesper.juhl@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=muvarov@ru.mvista.com \
    --cc=randy.dunlap@oracle.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox