Re: [PATCH] hwpmc(4) changes to use 'mp_maxid' instead of - FreeBSD

This is a discussion on Re: [PATCH] hwpmc(4) changes to use 'mp_maxid' instead of - FreeBSD ; 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 ...

Re: [PATCH] hwpmc(4) changes to use 'mp_maxid' instead of

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.