Re: MAXCPU preparations
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
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
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
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
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
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
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
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
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
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
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
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
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
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/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
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
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/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
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
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
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
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
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
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
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