From owner-freebsd-arch@FreeBSD.ORG Thu Aug 14 17:12:15 2014 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id D27276B4; Thu, 14 Aug 2014 17:12:15 +0000 (UTC) Received: from mail-qa0-x232.google.com (mail-qa0-x232.google.com [IPv6:2607:f8b0:400d:c00::232]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 8347A22A3; Thu, 14 Aug 2014 17:12:15 +0000 (UTC) Received: by mail-qa0-f50.google.com with SMTP id s7so1175032qap.23 for ; Thu, 14 Aug 2014 10:12:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=6j+A+oW91Zgydj4KVISrAllRS1Rv6k8lUBS9fR0cSpc=; b=tRZmjaQB8fsyqEZ8gtX46b+z26UXRRIt+0fziULcr3YHUsneGDiyDkGWC22AACv25j G5nK3qwqejQSVQr76FYBB7w00AXwnISBL7lby5KNQwW1BxzI+3+e22fnWKtFdg8r1YNX BKUULgPKBdMetkQEDsZpQ5PkFEgvJGnHCjy3ATDxohzDLKZ694HgYH/5i5ARfGqD5e+A /vZ4mAYn1Pq0E00DWx/NMfl41Eb/YcCLeJhVQ9Qh5NbIjAFTgcSptSiVoEtOTpvO6nul B34svEHBNeCvOJsRdm3X/iGxaMjLt0uyX6xVtiv/qaX5l0YHSQnBEzZ1tmJ5+EZl38iU CjbA== MIME-Version: 1.0 X-Received: by 10.224.30.77 with SMTP id t13mr16616092qac.63.1408036333641; Thu, 14 Aug 2014 10:12:13 -0700 (PDT) Received: by 10.224.39.139 with HTTP; Thu, 14 Aug 2014 10:12:13 -0700 (PDT) In-Reply-To: <201408141143.01137.jhb@freebsd.org> References: <201408120939.56176.jhb@freebsd.org> <201408141143.01137.jhb@freebsd.org> Date: Thu, 14 Aug 2014 10:12:13 -0700 Message-ID: Subject: Re: RFC: cpuid_t From: Adrian Chadd To: John Baldwin Content-Type: text/plain; charset=UTF-8 Cc: "freebsd-arch@freebsd.org" X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Aug 2014 17:12:16 -0000 On 14 August 2014 08:43, John Baldwin wrote: > On Tuesday, August 12, 2014 6:17:27 pm Adrian Chadd wrote: >> On 12 August 2014 06:39, John Baldwin wrote: >> >> > I think Bruce's point is you can just fix the few places that use a u_char to >> > an int and call it a day. The biggest offender of those that I can think of >> > are td_oncpu/td_lastcpu and the corresponding fields in kinfo along with >> > expanding NOCPU to be -1 instead of 255. I think most other places already >> > use an int (pf is one place that doesn't, it borrows N bits from some other >> > field to store the CPU number, so it can't use a cpuid_t anyway), so the patch >> > to just use an int might actually be quite small. >> >> You could do that, but then we'd be left in a situation that it isn't >> 100% clear what type people should be using (except hoping they >> cut/paste code that uses an int and don't make dumb decisions like >> assuming they can pack it into other fields, like pf) and that they >> could treat the CPU ID as some kind of index number. Being able to >> grep for all the places where cpuid_t is will make it a lot easier in >> the future to uplift all of the cpu id manipulating code to treat CPU >> IDs differently (eg making things very sparse.) > > I'm not sure how a typedef makes it easier to handle sparse IDs over an int? > Plus, the IDs are already allowed to be sparse (hence CPU_ABSENT() and > CPU_FOREACH(), etc.). I doubt that a typedef would make it any easier to > deal with issues like pf either because the current code is not explicitly > using any type at all, it's just passed into magic macros. It doesn't. It's there to help you find places where people aren't doing the iteration correctly. You can do tricks like making cpuid_t temporarily a struct with the int embedded in it: it'll work fine where it's being copied around as an opaque type, but it'll compiler error out the minute some code tries passing it into a field that takes an int, or a char. >> So, I'm happy if the group decision is to just make it an int. It just >> has to be done before FreeBSD-11 ships. :) > > I think making it an int would be a good first step at the very least. It > would be good to see how many places don't already use it that way. > >> I don't have such an aversion to typedefs. It has always made it much >> easier to do type checking and changes to things in C that I'd >> normally do in C++ with classes. > > One downside is that for printf() (a non-trivial thing in C), you have to > either violate the abstraction (i.e. "know" it's an int) or use intmax_t casts > everywhere. That's why you have a cpuid_t_to_quad() macro and use that everywhere you need a numerical representation of the ID. That way it's up to the macro or inline to return it as the type you require and thus printf() and friends can have a well-known type in the string. (I dislike intmax_t casts. I'd rather there were explicit ways to convert those types to integer representations where appropriate rather than having the code do those casts just to print. Ugh.) -a