Bug 18524

Summary: The current kernel doesn't keep stats on a per cpu basis
Product: Base System Reporter: adsharma <adsharma>
Component: kernAssignee: freebsd-bugs (Nobody) <bugs>
Status: Closed FIXED    
Severity: Affects Only Me    
Priority: Normal    
Version: 4.0-STABLE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
file.diff none

Description adsharma 2000-05-13 07:00:04 UTC 
	The current kernel doesn't keep stats on a per cpu basis

Fix: The attached patch creates the right variables within the kernel.
	The kld at http://sharmas.dhs.org/~adsharma/projects/freebsd/sysctl.tar.gz 
        extracts them using a bunch of dynamic sysctls. 
 
Implementation details:

1. On UP,
 
        sys_time is a global and contains the system wide stats
        cpu_time is a global and is essentially the same as sys_time.
 
2. On SMP
 
        sys_time contains the system wide stats
        cpu_time has been changed to a pointer in the per-cpu space.
        On BSP, this pointer points to a static array cpu0_cpu_time
        On APs, this space is kmem_alloc'ed
 
        Perhaps I should wrap cpu_time in a structure (cpu_info ?), which
        could be the right place to store all per CPU info.
 
3. I've taken the liberty of changing CP_* to CPU_*. I hope the new names
   better convey the meaning of the variables and are acceptable.   

4. The machine specific code for Alpha will need some changes - which I
   can implement, but have no way of compiling or testing.
 
5. All the existing utilties which depended on peeking at cp_time will
   break (which is a good thing, IMO - so that they can be changed to use 
   the new sysctl :-) 

6. After the above kld is loaded, the sysctl's will be at kern.stats.*

How-To-Repeat: 
 	Run 5.0-current.
Comment 1 Jens Schweikhardt freebsd_committer freebsd_triage 2002-08-13 22:01:23 UTC 
State Changed
From-To: open->feedback

A lot of work has been going on in -current since this PR 
was opened. Does this problem still persist?
Comment 2 Jens Schweikhardt freebsd_committer freebsd_triage 2002-08-14 21:37:48 UTC 
State Changed
From-To: feedback->closed

Feedback request bounced. If still an issue, let us know.
Comment 3 Arun Sharma 2003-02-06 07:33:14 UTC 
I'd like to reopen this PR:

http://www.freebsd.org/cgi/query-pr.cgi?pr=18524

and submit a new patch:

http://www.sharma-home.net/~adsharma/misc/pcpu-cptime.patch

$ sysctl kern.smp.cpu kern.cp_time
kern.smp.cpu.0.cp_time: 1196 1 2900 351 74120
kern.smp.cpu.1.cp_time: 1248 3 2837 329 74110
kern.cp_time: 2444 4 5737 680 148230

The ugliest part of this patch is calling mi_cpu_start(cpu). I tried 
making the calls in i386/mp_machdep.c, but ran into panics no matter 
where I placed the call (mi_cpu_start ends up calling malloc, which may 
block, do TLB shoot downs etc).

So if someone has a better idea about how to arrange for a machine 
independent cpu start/stop hook that could make blocking calls, I'd like 
to hear about it.

	-Arun
Comment 4 John Baldwin freebsd_committer freebsd_triage 2003-02-06 16:13:15 UTC 
On 06-Feb-2003 Arun Sharma wrote:
> 
> I'd like to reopen this PR:
> 
> http://www.freebsd.org/cgi/query-pr.cgi?pr=18524
> 
> and submit a new patch:
> 
> http://www.sharma-home.net/~adsharma/misc/pcpu-cptime.patch
> 
> $ sysctl kern.smp.cpu kern.cp_time
> kern.smp.cpu.0.cp_time: 1196 1 2900 351 74120
> kern.smp.cpu.1.cp_time: 1248 3 2837 329 74110
> kern.cp_time: 2444 4 5737 680 148230
> 
> The ugliest part of this patch is calling mi_cpu_start(cpu). I tried 
> making the calls in i386/mp_machdep.c, but ran into panics no matter 
> where I placed the call (mi_cpu_start ends up calling malloc, which may 
> block, do TLB shoot downs etc).
> 
> So if someone has a better idea about how to arrange for a machine 
> independent cpu start/stop hook that could make blocking calls, I'd like 
> to hear about it.

Why not stick the cp_time stuff in struct pcpu instead of using an
array?

-- 

John Baldwin <jhb@FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/
Comment 5 Arun Sharma 2003-02-06 16:50:49 UTC 
On Thu, Feb 06, 2003 at 11:13:15AM -0500, John Baldwin wrote:
> > and submit a new patch:
> > 
> > http://www.sharma-home.net/~adsharma/misc/pcpu-cptime.patch
> 
> [...]
>
> Why not stick the cp_time stuff in struct pcpu instead of using an
> array?

The new patch _is_ putting cp_time in struct pcpu. The old patch in the PR
predates struct pcpu.

I also chose to leave the existing cp_time alone. One could argue that a user
level tool could sum up the pcpu cp_times to derive the cp_time and
the kernel can avoid dirtying an extra cache line. If people feel
strongly about it, I can skip touching cp_time in the SMP case. It's a
choice between compatibility with UP vs performance.

	-Arun