Re: MAXCPU preparations

2010-09-29 Thread John Baldwin
On Tuesday, September 28, 2010 6:24:32 pm Robert N. M. Watson wrote:
 
 On 28 Sep 2010, at 19:40, Sean Bruno wrote:
 
  If you go fully dynamic you should use mp_maxid + 1 rather than maxcpus.
  
  I assume that mp_maxid is the new kern.smp.maxcpus?  Can you inject some
  history here so I can understand why one is better than the other?
 
 So, unlike maxcpus, mp_maxid is in theory susceptible to races in a brave new 
 world in which we support hotplug CPUs -- a brave new world we're 
not yet ready for, however. If you do use mp_maxid, be aware you need to add 
one to get the size of the array -- maxcpus is the number of CPUs that 
can be used, whereas mp_maxid is the highest CPU ID (counting from 0) that is 
used.

Hmm, I'm not sure that is true.  My feeling on mp_maxid is that it is the
largest supported CPUID.  Platforms that support hotplug would need to set
mp_maxid such that it has room for hotpluggable CPUs.  You don't want to
go reallocate the UMA datastructures for every zone when a CPU is hotplugged
for example.

-- 
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-29 Thread Robert N. M. Watson

On 29 Sep 2010, at 12:49, John Baldwin wrote:

 On Tuesday, September 28, 2010 6:24:32 pm Robert N. M. Watson wrote:
 
 On 28 Sep 2010, at 19:40, Sean Bruno wrote:
 
 If you go fully dynamic you should use mp_maxid + 1 rather than maxcpus.
 
 I assume that mp_maxid is the new kern.smp.maxcpus?  Can you inject some
 history here so I can understand why one is better than the other?
 
 So, unlike maxcpus, mp_maxid is in theory susceptible to races in a brave 
 new world in which we support hotplug CPUs -- a brave new world we're 
 not yet ready for, however. If you do use mp_maxid, be aware you need to add 
 one to get the size of the array -- maxcpus is the number of CPUs that 
 can be used, whereas mp_maxid is the highest CPU ID (counting from 0) that is 
 used.
 
 Hmm, I'm not sure that is true.  My feeling on mp_maxid is that it is the
 largest supported CPUID.  Platforms that support hotplug would need to set
 mp_maxid such that it has room for hotpluggable CPUs.  You don't want to
 go reallocate the UMA datastructures for every zone when a CPU is hotplugged
 for example.

Yes, we'll have to break one (or even both) of two current assumptions with the 
move to hotplug: contiguous in-use CPU IDs and mp_maxid representing the 
greatest possible CPU ID as a constant value. The former is guaranteed, but I 
wonder about the latter. On face value, you might say oh, we know how many 
sockets there are, but if course, we don't know how many threads will arrive 
when a package is inserted.  For many subsystems, DPCPU will present a more 
than adequate answer for avoiding resizing, although not for low-level systems 
(perhaps such as UMA?). Likewise, on virtual machine platforms where hotplug 
actually might reflect a longer-term scheduling choice by the admin/hypervisor 
(i.e., resource partitioning), it may be harder to reason about what the future 
holds.

Robert___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-29 Thread Matthew Fleming
On Wed, Sep 29, 2010 at 1:54 PM, Robert N. M. Watson
rwat...@freebsd.org wrote:

 On 29 Sep 2010, at 12:49, John Baldwin wrote:

 On Tuesday, September 28, 2010 6:24:32 pm Robert N. M. Watson wrote:

 On 28 Sep 2010, at 19:40, Sean Bruno wrote:

 If you go fully dynamic you should use mp_maxid + 1 rather than maxcpus.

 I assume that mp_maxid is the new kern.smp.maxcpus?  Can you inject some
 history here so I can understand why one is better than the other?

 So, unlike maxcpus, mp_maxid is in theory susceptible to races in a brave 
 new world in which we support hotplug CPUs -- a brave new world we're
 not yet ready for, however. If you do use mp_maxid, be aware you need to add 
 one to get the size of the array -- maxcpus is the number of CPUs that
 can be used, whereas mp_maxid is the highest CPU ID (counting from 0) that 
 is used.

 Hmm, I'm not sure that is true.  My feeling on mp_maxid is that it is the
 largest supported CPUID.  Platforms that support hotplug would need to set
 mp_maxid such that it has room for hotpluggable CPUs.  You don't want to
 go reallocate the UMA datastructures for every zone when a CPU is hotplugged
 for example.

 Yes, we'll have to break one (or even both) of two current assumptions with 
 the move to hotplug: contiguous in-use CPU IDs and mp_maxid representing the 
 greatest possible CPU ID as a constant value. The former is guaranteed, but I 
 wonder about the latter. On face value, you might say oh, we know how many 
 sockets there are, but if course, we don't know how many threads will arrive 
 when a package is inserted.  For many subsystems, DPCPU will present a more 
 than adequate answer for avoiding resizing, although not for low-level 
 systems (perhaps such as UMA?). Likewise, on virtual machine platforms where 
 hotplug actually might reflect a longer-term scheduling choice by the 
 admin/hypervisor (i.e., resource partitioning), it may be harder to reason 
 about what the future holds.


As a historical note, when AIX added hotplug CPU support, we kept the
MAXCPU define as the largest number of CPUs supported by the OS image.
At the time it was 256; as it shifted to 1024 there was a large
cleanup effort to eliminate as many statically sized arrays as
possible.

AIX also has an mp_maxid equivalent which changed when new higher core
numbers were used.  For various reasons, new CPUs were added only
while a single CPU was running, so any loop that looked like (for i =
0; i = mp_maxid; i++) could get interrupted by an IPI (the handler
spun until the core was added by the coordinating CPU) and find that
it had a stale mp_maxid value.  This was not a bug in 99% of the uses
of the code, since whatever it was doing with each CPU was either not
atomic (e.g. summing per-cpu counters) or was some kind of
initializing work which was also done by the new CPU before it was
fully awake.

I don't necessarily recommend this as an architected plan for adding
CPUs, but I wanted to offer the historical note that it could work.

Also, CPU id's may not be contiguous with hotplug.  Even if they are
contiguous on boot, there is no reason to assume CPUs are offlined
from highest ID down.  For reasons of cache sharing, the best
performance may be obtained by picking a specific CPU to offline.  It
also may be that it makes more sense to reserve CPU ids so that e.g.
CPUs N*T through N*(2T-1) are all HMT threads on the same core, (for
T-way HMT).  In this case CPU IDs become sparse if HMT is completely
disabled, and the best performance for the box overall is probably
obtained by offlining T3 and T2 for each core while keeping T0 and T1
active.

But discontiguous IDs shouldn't be a problem as all the code I've seen
checks CPU_ABSENT(i) in the loop.

Cheers,
matthew
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-28 Thread Robert Watson


On Mon, 27 Sep 2010, Joshua Neal wrote:

I hit this bug at one point, and had to bump MEMSTAT_MAXCPU.  It's already 
asking the kernel for the max number and throwing an error if it doesn't 
agree:


Yes, it looks like MAXCPU was bumped in the kernel without bumping the limit 
in libmemstat.  The bug could be in not having a comment by the definition of 
MAXCPU saying that MEMSTAT_MAXCPU needs to be modified as well.


I was thinking a more future-proof fix would be to get rid of the static 
allocations and allocate the library's internal structures based on the 
value of kern.smp.maxcpus.


Agreed.  I'm fairly preoccupied currently, but would be happy to accept 
patches :-).


Robert



- Joshua

On Mon, Sep 27, 2010 at 2:42 PM, Robert Watson rwat...@freebsd.org wrote:


On Mon, 27 Sep 2010, John Baldwin wrote:


Also, I think we should either fix MAXCPU to export the SMP value to
userland, or hide it from userland completely.  Exporting the UP value is
Just Wrong (tm).


Well, it's useful in the sense that it tells you what the maximum number of
CPUs a kernel can support is, which is helpful, especially if you're futzing
with MAXCPU as a kernel option :-).

But, more generally, many things that use MAXCPU should probably use either
mp_maxid or DPCPU.  Not everything, but most things.

Robert
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

Re: MAXCPU preparations

2010-09-28 Thread Sean Bruno
On Tue, 2010-09-28 at 02:48 -0500, Robert Watson wrote:
 On Mon, 27 Sep 2010, Joshua Neal wrote:
 
  I hit this bug at one point, and had to bump MEMSTAT_MAXCPU.  It's already 
  asking the kernel for the max number and throwing an error if it doesn't 
  agree:
 
 Yes, it looks like MAXCPU was bumped in the kernel without bumping the limit 
 in libmemstat.  The bug could be in not having a comment by the definition of 
 MAXCPU saying that MEMSTAT_MAXCPU needs to be modified as well.
 
  I was thinking a more future-proof fix would be to get rid of the static 
  allocations and allocate the library's internal structures based on the 
  value of kern.smp.maxcpus.
 
 Agreed.  I'm fairly preoccupied currently, but would be happy to accept 
 patches :-).
 
 Robert

Working on a dynamic version today.  I'll spam it over to you for review
later.  

I'm moving the percpu struct definitions outside of struct memory_type,
allocating quantity kern.smp.maxcpus, removing the boundary checks based
on MEMSTAT_MAXCPU and then removing MEMSTAT_MAXCPU all together.

Sean

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-28 Thread Robert N. M. Watson

On 28 Sep 2010, at 17:45, Sean Bruno wrote:

 Working on a dynamic version today.  I'll spam it over to you for review
 later.  
 
 I'm moving the percpu struct definitions outside of struct memory_type,
 allocating quantity kern.smp.maxcpus, removing the boundary checks based
 on MEMSTAT_MAXCPU and then removing MEMSTAT_MAXCPU all together.

Sounds like the right answer. Make sure to also keep the adapt the use of 
maxcpus from crashdumps so that we can also size the data structure when 
analyzing cores (or /dev/kmem or firewire).

Robert___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-28 Thread John Baldwin
On Tuesday, September 28, 2010 12:45:11 pm Sean Bruno wrote:
 On Tue, 2010-09-28 at 02:48 -0500, Robert Watson wrote:
  On Mon, 27 Sep 2010, Joshua Neal wrote:
  
   I hit this bug at one point, and had to bump MEMSTAT_MAXCPU.  It's 
   already 
   asking the kernel for the max number and throwing an error if it doesn't 
   agree:
  
  Yes, it looks like MAXCPU was bumped in the kernel without bumping the 
  limit 
  in libmemstat.  The bug could be in not having a comment by the definition 
  of 
  MAXCPU saying that MEMSTAT_MAXCPU needs to be modified as well.
  
   I was thinking a more future-proof fix would be to get rid of the static 
   allocations and allocate the library's internal structures based on the 
   value of kern.smp.maxcpus.
  
  Agreed.  I'm fairly preoccupied currently, but would be happy to accept 
  patches :-).
  
  Robert
 
 Working on a dynamic version today.  I'll spam it over to you for review
 later.  
 
 I'm moving the percpu struct definitions outside of struct memory_type,
 allocating quantity kern.smp.maxcpus, removing the boundary checks based
 on MEMSTAT_MAXCPU and then removing MEMSTAT_MAXCPU all together.

If you go fully dynamic you should use mp_maxid + 1 rather than maxcpus.

-- 
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-28 Thread Sean Bruno
On Tue, 2010-09-28 at 13:29 -0500, John Baldwin wrote:
 On Tuesday, September 28, 2010 12:45:11 pm Sean Bruno wrote:
  On Tue, 2010-09-28 at 02:48 -0500, Robert Watson wrote:
   On Mon, 27 Sep 2010, Joshua Neal wrote:
   
I hit this bug at one point, and had to bump MEMSTAT_MAXCPU.  It's 
already 
asking the kernel for the max number and throwing an error if it 
doesn't 
agree:
   
   Yes, it looks like MAXCPU was bumped in the kernel without bumping the 
   limit 
   in libmemstat.  The bug could be in not having a comment by the 
   definition of 
   MAXCPU saying that MEMSTAT_MAXCPU needs to be modified as well.
   
I was thinking a more future-proof fix would be to get rid of the 
static 
allocations and allocate the library's internal structures based on the 
value of kern.smp.maxcpus.
   
   Agreed.  I'm fairly preoccupied currently, but would be happy to accept 
   patches :-).
   
   Robert
  
  Working on a dynamic version today.  I'll spam it over to you for review
  later.  
  
  I'm moving the percpu struct definitions outside of struct memory_type,
  allocating quantity kern.smp.maxcpus, removing the boundary checks based
  on MEMSTAT_MAXCPU and then removing MEMSTAT_MAXCPU all together.
 
 If you go fully dynamic you should use mp_maxid + 1 rather than maxcpus.
 

I assume that mp_maxid is the new kern.smp.maxcpus?  Can you inject some
history here so I can understand why one is better than the other?

Sean

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-28 Thread John Baldwin
On Tuesday, September 28, 2010 2:40:44 pm Sean Bruno wrote:
 On Tue, 2010-09-28 at 13:29 -0500, John Baldwin wrote:
  On Tuesday, September 28, 2010 12:45:11 pm Sean Bruno wrote:
   On Tue, 2010-09-28 at 02:48 -0500, Robert Watson wrote:
On Mon, 27 Sep 2010, Joshua Neal wrote:

 I hit this bug at one point, and had to bump MEMSTAT_MAXCPU.  It's 
 already 
 asking the kernel for the max number and throwing an error if it 
 doesn't 
 agree:

Yes, it looks like MAXCPU was bumped in the kernel without bumping the 
limit 
in libmemstat.  The bug could be in not having a comment by the 
definition of 
MAXCPU saying that MEMSTAT_MAXCPU needs to be modified as well.

 I was thinking a more future-proof fix would be to get rid of the 
 static 
 allocations and allocate the library's internal structures based on 
 the 
 value of kern.smp.maxcpus.

Agreed.  I'm fairly preoccupied currently, but would be happy to accept 
patches :-).

Robert
   
   Working on a dynamic version today.  I'll spam it over to you for review
   later.  
   
   I'm moving the percpu struct definitions outside of struct memory_type,
   allocating quantity kern.smp.maxcpus, removing the boundary checks based
   on MEMSTAT_MAXCPU and then removing MEMSTAT_MAXCPU all together.
  
  If you go fully dynamic you should use mp_maxid + 1 rather than maxcpus.
  
 
 I assume that mp_maxid is the new kern.smp.maxcpus?  Can you inject some
 history here so I can understand why one is better than the other?

mp_maxid is the variable in the kernel of the maximum possible CPU ID in the
running kernel.  It is what all kernel code uses for dynamically sized per-CPU
arrays such as UMA.  It can be smaller than MAXCPU if the platform does not
support hotplug CPUs (true for all of our current platforms).

maxcpus was only added to export the value of MAXCPU so that user code that
uses libkvm to read kernel memory directly can work with datastructures that
use statically sized arrays (foo[MAXCPU]) rather than dynamically sized
arrays.  Aside from that one specific use case, maxcpus should not be used.

-- 
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-28 Thread Sean Bruno
On Tue, 2010-09-28 at 14:06 -0500, John Baldwin wrote:
 On Tuesday, September 28, 2010 2:40:44 pm Sean Bruno wrote:
  On Tue, 2010-09-28 at 13:29 -0500, John Baldwin wrote:
   On Tuesday, September 28, 2010 12:45:11 pm Sean Bruno wrote:
On Tue, 2010-09-28 at 02:48 -0500, Robert Watson wrote:
 On Mon, 27 Sep 2010, Joshua Neal wrote:
 
  I hit this bug at one point, and had to bump MEMSTAT_MAXCPU.  It's 
  already 
  asking the kernel for the max number and throwing an error if it 
  doesn't 
  agree:
 
 Yes, it looks like MAXCPU was bumped in the kernel without bumping 
 the limit 
 in libmemstat.  The bug could be in not having a comment by the 
 definition of 
 MAXCPU saying that MEMSTAT_MAXCPU needs to be modified as well.
 
  I was thinking a more future-proof fix would be to get rid of the 
  static 
  allocations and allocate the library's internal structures based on 
  the 
  value of kern.smp.maxcpus.
 
 Agreed.  I'm fairly preoccupied currently, but would be happy to 
 accept 
 patches :-).
 
 Robert

Working on a dynamic version today.  I'll spam it over to you for review
later.  

I'm moving the percpu struct definitions outside of struct memory_type,
allocating quantity kern.smp.maxcpus, removing the boundary checks based
on MEMSTAT_MAXCPU and then removing MEMSTAT_MAXCPU all together.
   
   If you go fully dynamic you should use mp_maxid + 1 rather than maxcpus.
   
  
  I assume that mp_maxid is the new kern.smp.maxcpus?  Can you inject some
  history here so I can understand why one is better than the other?
 
 mp_maxid is the variable in the kernel of the maximum possible CPU ID in the
 running kernel.  It is what all kernel code uses for dynamically sized per-CPU
 arrays such as UMA.  It can be smaller than MAXCPU if the platform does not
 support hotplug CPUs (true for all of our current platforms).
 
 maxcpus was only added to export the value of MAXCPU so that user code that
 uses libkvm to read kernel memory directly can work with datastructures that
 use statically sized arrays (foo[MAXCPU]) rather than dynamically sized
 arrays.  Aside from that one specific use case, maxcpus should not be used.
 

ah, I see now.  sys/kern/subr_smp.c::mp_maxid is exported via the systcl
kern.smp.maxid

thank you for clarifying.

Sean

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-28 Thread Robert N. M. Watson

On 28 Sep 2010, at 19:40, Sean Bruno wrote:

 If you go fully dynamic you should use mp_maxid + 1 rather than maxcpus.
 
 I assume that mp_maxid is the new kern.smp.maxcpus?  Can you inject some
 history here so I can understand why one is better than the other?

So, unlike maxcpus, mp_maxid is in theory susceptible to races in a brave new 
world in which we support hotplug CPUs -- a brave new world we're not yet ready 
for, however. If you do use mp_maxid, be aware you need to add one to get the 
size of the array -- maxcpus is the number of CPUs that can be used, whereas 
mp_maxid is the highest CPU ID (counting from 0) that is used.

Robert___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


MAXCPU preparations

2010-09-27 Thread Sean Bruno
Does this look like an appropriate modification to libmemstat?

Sean


 //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
- /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
@@ -28,12 +28,13 @@
 
 #ifndef _MEMSTAT_H_
 #define_MEMSTAT_H_
+#include sys/param.h
 
 /*
  * Number of CPU slots in library-internal data structures.  This
should be
  * at least the value of MAXCPU from param.h.
  */
-#defineMEMSTAT_MAXCPU  64
+#defineMEMSTAT_MAXCPU  MAXCPU /* defined in
sys/${ARCH}/include/param.h */
 
 /*
  * Amount of caller data to maintain for each caller data slot.
Applications


___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Julian Elischer

 On 9/27/10 8:26 AM, Sean Bruno wrote:

Does this look like an appropriate modification to libmemstat?

Sean


 //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
- /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
@@ -28,12 +28,13 @@

  #ifndef _MEMSTAT_H_
  #define_MEMSTAT_H_
+#includesys/param.h

  /*
   * Number of CPU slots in library-internal data structures.  This
should be
   * at least the value of MAXCPU from param.h.
   */
-#defineMEMSTAT_MAXCPU  64
+#defineMEMSTAT_MAXCPU  MAXCPU /* defined in
sys/${ARCH}/include/param.h */




wouldn't it be better to do a sysctlbyname() and use the real value 
for the system?



  /*
   * Amount of caller data to maintain for each caller data slot.
Applications


___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org



___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Scott Long
There's no reason not to include sys/param.h.  I'm a little reluctant to have 
it depend on the static MAXCPU definition, though.  What happens when you 
mix-and match userland and kernel and they no longer agree on the definition of 
MAXCPU?  I suggest creating a sysctl that exports the kernel's definition of 
MAXCPU, and have libmemstat look for that first, and fall back to using the 
static MAXCPU definition if the sysctl fails/doesn't exit.

Scott



On Sep 27, 2010, at 9:26 AM, Sean Bruno wrote:

 Does this look like an appropriate modification to libmemstat?
 
 Sean
 
 
  //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
 - /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
 @@ -28,12 +28,13 @@
 
 #ifndef _MEMSTAT_H_
 #define_MEMSTAT_H_
 +#include sys/param.h
 
 /*
  * Number of CPU slots in library-internal data structures.  This
 should be
  * at least the value of MAXCPU from param.h.
  */
 -#defineMEMSTAT_MAXCPU  64
 +#defineMEMSTAT_MAXCPU  MAXCPU /* defined in
 sys/${ARCH}/include/param.h */
 
 /*
  * Amount of caller data to maintain for each caller data slot.
 Applications
 
 
 ___
 freebsd-current@freebsd.org mailing list
 http://lists.freebsd.org/mailman/listinfo/freebsd-current
 To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Attilio Rao
2010/9/27 Sean Bruno sean...@yahoo-inc.com:
 Does this look like an appropriate modification to libmemstat?

 Sean


  //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
 - /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
 @@ -28,12 +28,13 @@

  #ifndef _MEMSTAT_H_
  #define        _MEMSTAT_H_
 +#include sys/param.h

  /*
  * Number of CPU slots in library-internal data structures.  This
 should be
  * at least the value of MAXCPU from param.h.
  */
 -#define        MEMSTAT_MAXCPU  64
 +#define        MEMSTAT_MAXCPU  MAXCPU /* defined in
 sys/${ARCH}/include/param.h */

  /*
  * Amount of caller data to maintain for each caller data slot.
 Applications

I would not include sys/param and would axe out the comment.

Just make sure anything compiles with these modifies eventually.

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Sean Bruno

 
 I would not include sys/param and would axe out the comment.
 
 Just make sure anything compiles with these modifies eventually.
 
 Thanks,
 Attilio
 
 

Ah, yes.  The include is completely pointless.  The value can be
assigned without it.

Sean

=== //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
- /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
@@ -33,7 +33,7 @@
  * Number of CPU slots in library-internal data structures.  This
should be
  * at least the value of MAXCPU from param.h.
  */
-#defineMEMSTAT_MAXCPU  64
+#defineMEMSTAT_MAXCPU  MAXCPU 
 
 /*
  * Amount of caller data to maintain for each caller data slot.
Applications


___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Sean Bruno
On Mon, 2010-09-27 at 08:53 -0700, Julian Elischer wrote:
 On 9/27/10 8:26 AM, Sean Bruno wrote:
  Does this look like an appropriate modification to libmemstat?
 
  Sean
 
 
   //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
  - /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
  @@ -28,12 +28,13 @@
 
#ifndef _MEMSTAT_H_
#define_MEMSTAT_H_
  +#includesys/param.h
 
/*
 * Number of CPU slots in library-internal data structures.  This
  should be
 * at least the value of MAXCPU from param.h.
 */
  -#defineMEMSTAT_MAXCPU  64
  +#defineMEMSTAT_MAXCPU  MAXCPU /* defined in
  sys/${ARCH}/include/param.h */
 
 
 
 wouldn't it be better to do a sysctlbyname() and use the real value 
 for the system?
 

That was my initial thought (as prodded by scottl and peter).  

If it is made dynamic, could this be opening a race condition where the
call to sysctlbyname() returns a count of CPUS that is in turn changed
by the offlining of a CPU?  Or am I thinking to much about this?

Sean

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Attilio Rao
2010/9/27 Sean Bruno sean...@yahoo-inc.com:
 On Mon, 2010-09-27 at 08:53 -0700, Julian Elischer wrote:
 On 9/27/10 8:26 AM, Sean Bruno wrote:
  Does this look like an appropriate modification to libmemstat?
 
  Sean
 
 
   //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
  - /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
  @@ -28,12 +28,13 @@
 
    #ifndef _MEMSTAT_H_
    #define        _MEMSTAT_H_
  +#includesys/param.h
 
    /*
     * Number of CPU slots in library-internal data structures.  This
  should be
     * at least the value of MAXCPU from param.h.
     */
  -#define        MEMSTAT_MAXCPU  64
  +#define        MEMSTAT_MAXCPU  MAXCPU /* defined in
  sys/${ARCH}/include/param.h */
 


 wouldn't it be better to do a sysctlbyname() and use the real value
 for the system?


 That was my initial thought (as prodded by scottl and peter).

 If it is made dynamic, could this be opening a race condition where the
 call to sysctlbyname() returns a count of CPUS that is in turn changed
 by the offlining of a CPU?  Or am I thinking to much about this?

We still can't support CPU hotplugging so the easy answer is 'don't
bother about variadic CPUs number'.
I don't really know what libmemstat is willing to do with that macro
(and I don't have time to look at it now) maybe you could shade a
light about what's its usage? Does it really needs to know MAXCPUS or
just wants a large enough value to fill anything?

Thanks,
Attilio


-- 
Peace can only be achieved by understanding - A. Einstein
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread mdf
On Mon, Sep 27, 2010 at 9:21 AM, Sean Bruno sean...@yahoo-inc.com wrote:
 On Mon, 2010-09-27 at 08:53 -0700, Julian Elischer wrote:
 On 9/27/10 8:26 AM, Sean Bruno wrote:
  Does this look like an appropriate modification to libmemstat?
 
  Sean
 
 
   //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
  - /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
  @@ -28,12 +28,13 @@
 
    #ifndef _MEMSTAT_H_
    #define        _MEMSTAT_H_
  +#includesys/param.h
 
    /*
     * Number of CPU slots in library-internal data structures.  This
  should be
     * at least the value of MAXCPU from param.h.
     */
  -#define        MEMSTAT_MAXCPU  64
  +#define        MEMSTAT_MAXCPU  MAXCPU /* defined in
  sys/${ARCH}/include/param.h */
 


 wouldn't it be better to do a sysctlbyname() and use the real value
 for the system?


 That was my initial thought (as prodded by scottl and peter).

 If it is made dynamic, could this be opening a race condition where the
 call to sysctlbyname() returns a count of CPUS that is in turn changed
 by the offlining of a CPU?  Or am I thinking to much about this?

The maximum number of CPUs supported by a running instance will not
change.  Only, potentially, the current number of CPUs.  So a sysctl
to fetch the kernel's compiled-in MAXCPU is safe.

Thanks,
matthew
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Robert Watson


On Mon, 27 Sep 2010, Scott Long wrote:

There's no reason not to include sys/param.h.  I'm a little reluctant to 
have it depend on the static MAXCPU definition, though.  What happens when 
you mix-and match userland and kernel and they no longer agree on the 
definition of MAXCPU?  I suggest creating a sysctl that exports the kernel's 
definition of MAXCPU, and have libmemstat look for that first, and fall back 
to using the static MAXCPU definition if the sysctl fails/doesn't exit.


I suppose, in a very worst case scenario, we can read the source code for 
libmemstat and see what it does.


Robert



Scott



On Sep 27, 2010, at 9:26 AM, Sean Bruno wrote:


Does this look like an appropriate modification to libmemstat?

Sean


 //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
- /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
@@ -28,12 +28,13 @@

#ifndef _MEMSTAT_H_
#define_MEMSTAT_H_
+#include sys/param.h

/*
 * Number of CPU slots in library-internal data structures.  This
should be
 * at least the value of MAXCPU from param.h.
 */
-#defineMEMSTAT_MAXCPU  64
+#defineMEMSTAT_MAXCPU  MAXCPU /* defined in
sys/${ARCH}/include/param.h */

/*
 * Amount of caller data to maintain for each caller data slot.
Applications


___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Robert Watson


On Mon, 27 Sep 2010, Sean Bruno wrote:

wouldn't it be better to do a sysctlbyname() and use the real value for the 
system?


libmemstat contains some useful sample code showing how this might be done.


That was my initial thought (as prodded by scottl and peter).

If it is made dynamic, could this be opening a race condition where the call 
to sysctlbyname() returns a count of CPUS that is in turn changed by the 
offlining of a CPU?  Or am I thinking to much about this?


Yes, you are.  MAXCPU is a compile-time constant for kernel builds, so (at 
least a the world is today), that can't happen.


I think there's a reasonable argument that MEMSTAT_MAXCPU should be phased out 
and all internal structures in libmemstat should be dynamically sized. 
However, core counts aren't growing that fast, and it's quite a bit of work, 
and probably not worth it just yet.


I'm somewhat averse to using MAXCPU in libmemstat, however, because MAXCPU is 
actually not a constant in the general case: FreeBSD/i386, for example, 
regularly uses two different values: 1 for !SMP kernels, and 32 for SMP 
kernels.  That's why libmemstat encodes its own value, for better or worse.


A reasonable alternative would be to replace 32 with MAXCPU * 2, or if we're 
feeling particularly optimistic, MAXCPU * 4.  Or just another big number, like 
256.


Robert
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Sean Bruno
On Mon, 2010-09-27 at 12:41 -0500, Attilio Rao wrote:
 2010/9/27 Sean Bruno sean...@yahoo-inc.com:
  On Mon, 2010-09-27 at 08:53 -0700, Julian Elischer wrote:
  On 9/27/10 8:26 AM, Sean Bruno wrote:
   Does this look like an appropriate modification to libmemstat?
  
   Sean
  
  
    //depot/yahoo/ybsd_7/src/lib/libmemstat/memstat.h#4
   - /home/seanbru/ybsd_7/src/lib/libmemstat/memstat.h 
   @@ -28,12 +28,13 @@
  
 #ifndef _MEMSTAT_H_
 #define_MEMSTAT_H_
   +#includesys/param.h
  
 /*
  * Number of CPU slots in library-internal data structures.  This
   should be
  * at least the value of MAXCPU from param.h.
  */
   -#defineMEMSTAT_MAXCPU  64
   +#defineMEMSTAT_MAXCPU  MAXCPU /* defined in
   sys/${ARCH}/include/param.h */
  
 
 
  wouldn't it be better to do a sysctlbyname() and use the real value
  for the system?
 
 
  That was my initial thought (as prodded by scottl and peter).
 
  If it is made dynamic, could this be opening a race condition where the
  call to sysctlbyname() returns a count of CPUS that is in turn changed
  by the offlining of a CPU?  Or am I thinking to much about this?
 
 We still can't support CPU hotplugging so the easy answer is 'don't
 bother about variadic CPUs number'.
 I don't really know what libmemstat is willing to do with that macro
 (and I don't have time to look at it now) maybe you could shade a
 light about what's its usage? Does it really needs to know MAXCPUS or
 just wants a large enough value to fill anything?
 
 Thanks,
 Attilio
 
 

Give or take, MEMSTAT_MAXCPU is used to allocate an array of per cpu
stats (see lib/libmemstat/memstat_internal.h::struct memory_type).

If the number of probed CPUs is more that this value, the library
returns an error.



Sean




___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread John Baldwin
On Monday, September 27, 2010 5:21:31 pm Robert Watson wrote:
 
 On Mon, 27 Sep 2010, Sean Bruno wrote:
 
  wouldn't it be better to do a sysctlbyname() and use the real value for 
  the 
  system?
 
 libmemstat contains some useful sample code showing how this might be done.
 
  That was my initial thought (as prodded by scottl and peter).
 
  If it is made dynamic, could this be opening a race condition where the 
  call 
  to sysctlbyname() returns a count of CPUS that is in turn changed by the 
  offlining of a CPU?  Or am I thinking to much about this?
 
 Yes, you are.  MAXCPU is a compile-time constant for kernel builds, so (at 
 least a the world is today), that can't happen.
 
 I think there's a reasonable argument that MEMSTAT_MAXCPU should be phased 
 out 
 and all internal structures in libmemstat should be dynamically sized. 
 However, core counts aren't growing that fast, and it's quite a bit of work, 
 and probably not worth it just yet.
 
 I'm somewhat averse to using MAXCPU in libmemstat, however, because MAXCPU is 
 actually not a constant in the general case: FreeBSD/i386, for example, 
 regularly uses two different values: 1 for !SMP kernels, and 32 for SMP 
 kernels.  That's why libmemstat encodes its own value, for better or worse.
 
 A reasonable alternative would be to replace 32 with MAXCPU * 2, or if we're 
 feeling particularly optimistic, MAXCPU * 4.  Or just another big number, 
 like 
 256.

A prerequisite for this idea though is that MAXCPU needs to be fixed to
export the SMP value to userland rather than the UP value.  For example, this
code in amd64/include/param.h:

#if defined(SMP) || defined(KLD_MODULE)
#define MAXCPU  32
#else
#define MAXCPU  1
#endif

would need to change so that !_KERNEL is in the conditional.  I do think we
should avoid using MAXCPU in userland as much as possible and use the
existing sysctl for mp_maxid instead to dynamically allocate arrays.

Also, I think we should either fix MAXCPU to export the SMP value to
userland, or hide it from userland completely.  Exporting the UP value is
Just Wrong (tm).

-- 
John Baldwin
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Robert Watson


On Mon, 27 Sep 2010, John Baldwin wrote:

Also, I think we should either fix MAXCPU to export the SMP value to 
userland, or hide it from userland completely.  Exporting the UP value is 
Just Wrong (tm).


Well, it's useful in the sense that it tells you what the maximum number of 
CPUs a kernel can support is, which is helpful, especially if you're futzing 
with MAXCPU as a kernel option :-).


But, more generally, many things that use MAXCPU should probably use either 
mp_maxid or DPCPU.  Not everything, but most things.


Robert
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org


Re: MAXCPU preparations

2010-09-27 Thread Joshua Neal
I hit this bug at one point, and had to bump MEMSTAT_MAXCPU.  It's
already asking the kernel for the max number and throwing an error if
it doesn't agree:

   if (sysctlbyname(kern.smp.maxcpus, maxcpus, size, NULL, 0)  0) {

[...]

if (maxcpus  MEMSTAT_MAXCPU) {
list-mtl_error = MEMSTAT_ERROR_TOOMANYCPUS;
return (-1);
}

I was thinking a more future-proof fix would be to get rid of the
static allocations and allocate the library's internal structures
based on the value of kern.smp.maxcpus.

- Joshua

On Mon, Sep 27, 2010 at 2:42 PM, Robert Watson rwat...@freebsd.org wrote:

 On Mon, 27 Sep 2010, John Baldwin wrote:

 Also, I think we should either fix MAXCPU to export the SMP value to
 userland, or hide it from userland completely.  Exporting the UP value is
 Just Wrong (tm).

 Well, it's useful in the sense that it tells you what the maximum number of
 CPUs a kernel can support is, which is helpful, especially if you're futzing
 with MAXCPU as a kernel option :-).

 But, more generally, many things that use MAXCPU should probably use either
 mp_maxid or DPCPU.  Not everything, but most things.

 Robert
 ___
 freebsd-current@freebsd.org mailing list
 http://lists.freebsd.org/mailman/listinfo/freebsd-current
 To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org