Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 14 Mar 2008 11:40:04 +0000 (GMT)
From:      Robert Watson <rwatson@FreeBSD.org>
To:        "M. Warner Losh" <imp@bsdimp.com>
Cc:        freebsd-arch@FreeBSD.ORG
Subject:   Re: [PATCH] hwpmc(4) changes to use 'mp_maxid' instead of 'mp_ncpus'.
Message-ID:  <20080314112104.I60466@fledge.watson.org>
In-Reply-To: <20080314.003749.-432746071.imp@bsdimp.com>
References:  <200803131516.12284.jhb@freebsd.org> <84dead720803132232k15c3aad7pe59875f0c84e0c27@mail.gmail.com> <20080313200839.S1091@desktop> <20080314.003749.-432746071.imp@bsdimp.com>

next in thread | previous in thread | raw e-mail | index | archive | help
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.

Robert N M Watson
Computer Laboratory
University of Cambridge



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080314112104.I60466>