From owner-freebsd-arch@FreeBSD.ORG Fri Mar 14 11:40:05 2008 Return-Path: Delivered-To: freebsd-arch@FreeBSD.ORG Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3DBF91065672 for ; Fri, 14 Mar 2008 11:40:05 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (cyrus.watson.org [209.31.154.42]) by mx1.freebsd.org (Postfix) with ESMTP id 285458FC20 for ; Fri, 14 Mar 2008 11:40:04 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [209.31.154.41]) by cyrus.watson.org (Postfix) with ESMTP id 1945D46B8F; Fri, 14 Mar 2008 07:40:04 -0400 (EDT) Date: Fri, 14 Mar 2008 11:40:04 +0000 (GMT) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: "M. Warner Losh" In-Reply-To: <20080314.003749.-432746071.imp@bsdimp.com> Message-ID: <20080314112104.I60466@fledge.watson.org> References: <200803131516.12284.jhb@freebsd.org> <84dead720803132232k15c3aad7pe59875f0c84e0c27@mail.gmail.com> <20080313200839.S1091@desktop> <20080314.003749.-432746071.imp@bsdimp.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-arch@FreeBSD.ORG Subject: Re: [PATCH] hwpmc(4) changes to use 'mp_maxid' instead of 'mp_ncpus'. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Mar 2008 11:40:05 -0000 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