[PATCH] hwpmc(4) changes to use 'mp_maxid' instead of 'mp_ncpus'.

[Sam Leffler]

Robert Watson wrote:
> On Fri, 14 Mar 2008, M. Warner Losh wrote:
>
>> I'd like to echo these sentiments.  We've generally been willing to 
>> accept code from vendors that makes their lives easier, even when 
>> that code doesn't directly benefit the project.  We do this on the 
>> theory that if we make their life easy, they will contribute to the 
>> project. Juniper has certainly given a large chunk of code to the 
>> project (a fairly complete MIPS port that has been integrated with 
>> the so-called "mips2" port and will be headed into the tree soonish), 
>> which is certainly a lot more code than has been given from vendors 
>> whom we've made much bigger accommodations to.
>>
>> In this case a vendor came forward with a patch that introduces no 
>> real additional burdon to the volunteers who are maintaining the 
>> code.  It seems like a no brainer to me to commit it.  There's 
>> certainly no compelling technical argument against it.
>
> I think (hope?) everyone here would generally agree on the point 
> regarding vendors.  However, I think there is a technical point being 
> made as well, and we're at risk of losing track of it.
>
> Koshy has pointed out that changing just the kernel parts is 
> *insufficient* to remove the assumption of non-sparse CPU identifiers, 
> because the kernel parts are not all there is to hwpmc.  The 
> KASSERT()s document not just the assumptions of the kernel code, which 
> are updated by the proposed patch, but also relate to the guarantees 
> made by the user APIs for hwpmc libraries, tools, and documentation.  
> They are directly affected by the proposed change because they both 
> expose and rely on the non-sparse CPU identifier assumption, and also 
> need to be updated to reflect the changed assumption.
>
> FWIW, we should reemphasize here that sparse CPU identifiers, although 
> not all that well-supported by our kernel, do exist and function today 
> on all the SMP architectures that we support.  The hyperthreading 
> disable frob introduced a few years ago leads to sparse identifiers 
> for live CPUs on i386 and amd64, and triggered problems in several 
> pieces of code (now believed to mostly be resolved?).  We do need a 
> better general infrastructure for handling CPU information, and the 
> cpuset(2) API starts to address this.  I understand that a man page 
> for this will materialize soon :-).
>
> Still missing, and something to discuss in detail at the devsummit 
> since it will require non-trivial architectural changes, is how to 
> handle live CPU reconfiguration, which is increasingly relevant due to 
> hypervisor-driven virtualization.  It became rapidly clear when the 
> HTT frob was a run-time changeable sysctl (no longer true, I hope) 
> that changing the set of "absent" CPUs at run time caused our kernel 
> to behave in relatively catastrophic ways, and should be avoided, and 
> that's just a hint in the direction of the changes we'll need to make 
> to fully support hotplug.  Universal support for sparse CPU 
> identifiers throughout the system is just one prerequisite for getting 
> to hotplug.
>

hwpmc is a useful tool and needs to be improved.  It appears there are 
multiple groups/people interested in doing that and we need to leverage 
that, not discourage it (given the rate of progress on the existing 
implementation I can only guess it's too much work for one individual).  
Getting the kernel changes in will allow other work to go on in parallel 
and doesn't appear to impact any existing usage.

Please commit these changes and let's move on.

    Sam