From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f49.google.com (mail-wm1-f49.google.com [209.85.128.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id BD30636D51B for ; Thu, 2 Apr 2026 22:21:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.49 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775168492; cv=none; b=b2NYxHvmeN4HaV6/JhCd0C57Exn1BeyCHw4pVUlC1VYK+l54t1mmBKYaynltvo6Rx4GpviPzq2BE+S/efVqZo94GgCSNT9OoRcD5kLE3Ps6S0TKm5sWOXrEgAr98nS6uZjVe9ROf0EnPRqTAD4V1xeskseyDj73DqwC2U0x3NYU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775168492; c=relaxed/simple; bh=JO36LpQ5gVtcWZ8brznoqUIkSZhSat8fICNZkux4Jcs=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=V1GZkYqd2dTJuUHVqsorMgicOeJxwGxBXu6140gO6CAteaXD/tx7vhNnvKVQ+VjFSQVfrvunh1nt5rOJvQV8Qd1rq+AsOLSkKc/Ir+u72Dlf8Fp7nFzA93WPaDAfzvehwry8BzZ2znh4loyQ/Fvfk5gGKfrI4MlAPofnP7387Qg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=DIdXPFjy; arc=none smtp.client-ip=209.85.128.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="DIdXPFjy" Received: by mail-wm1-f49.google.com with SMTP id 5b1f17b1804b1-48702d51cd0so17544715e9.2 for ; Thu, 02 Apr 2026 15:21:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1775168489; x=1775773289; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=dI/NcwHPy2CKiwxvNl8NgCnSDMnJLWJEe2Db1Alx/A0=; b=DIdXPFjyvyE29tkijnDmXva+c7sPAamv1XeBVojQCohIdwx/KbzLwMcxltCGsPqDbu 8XFQr1wZp5xmx7VsWMj9Klk9xHq6gdbUF4P/1GWceV7Hq+NV8IYPYNGoQg5Cw4goeU72 UCInCS7IFd8XvTUSFNBIFqmnRQ2tPC8FliuBHt5uqUtOR1RkMJ+fUhO1NhM1pLd4N6Vq LDxlQ+/0IzP7XRWsoEru2bpPo47TTyeFgEcUNke77+rA7BsL7Q7XfoDUxQ/6aMVxGmiF vhvfTF6G90MpTE78Rj8Gu0NphSRBPbJ9ZwiChfHW5cfxRPGbi3Epn66ImphiXZK/JN5H P0XA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775168489; x=1775773289; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=dI/NcwHPy2CKiwxvNl8NgCnSDMnJLWJEe2Db1Alx/A0=; b=JrmpObWB1gVLFvS2jD9+swpeIMjsf9qCBlSn7Iz+s5V+/bgoIxPuLLg+tGWZrK7BRM IbyeA8KrzSvd5qOpmlS9JeQ+Di1Rn9cDfgonjfO8yKj0RN4gTZTmdCC26/LFdWzLkuFl UtKp9lRtIspS/5/2laUKD9BqHzS+WanxsXMvDaYGwZZl4JlEEwFMwaqzUufG0Po/uWYq rSK8cxiUqEaYv4s9vtIO0EK+9doJM5MUPD3MUz1MsJJTmHakQSAOnRIYscFrLM0/oPWe UpLB1X6O8NQGObe6sICCM8KDGffkdvM+5uRAiC5O2QaOpNtgR1mJ+nCMVFbvcTvfkIw/ RTDA== X-Forwarded-Encrypted: i=1; AJvYcCUK81uRCIwlntv30sqGHqtrmU1YTlKP2sz4qgEbpRloNLZ864oosBuM5RE7GROGXWkSwsH+CWlxWETjfSA=@vger.kernel.org X-Gm-Message-State: AOJu0Yz8jrMMtP4AM1G0ygZ4syYjjLy92aeXyVrg+7yy4iPSsBDPYoNu 73IKQ7PwZfPX/GiDR+lD3uCmVBc+nfeG0sOmPPhKfRU6dyFnzpVtN1il X-Gm-Gg: ATEYQzyO7VPqA2difIsQJhYm5KZ16TmLeT6qTmbXLbjafW7kqbXi/rfTqgzJIpqhn80 RbJ/+qxfWxd1rvbhDMkkcm7rs4Sl6tmRyqeqvdejn0W7TPKYD3H/qXDN4jj/7aJf+8W/eXqIwT9 VdWbkygL/nDwpA0nRXaNg//bmLucUEKdErkIIH2Kmn60mT8+91d+/6FXEXOhMn9tCNzmmscJc8w ipo0/2rFBBMMRYPIjeOLHzHUoM7E2226fltr2+qjotZAON1+6EnWRcKzx2eWsZ/DXneFIuY66vJ oZbuRBmjNG3pnq4Z3ktM3DUqaBZCABJ6ZuIvF9EaF34sKgosuAhPMkcwPGhghP5acOFLTdzK/uF 6u8fJRUMLocKvpObE+kd4sft8epgfmI2yz3hFVqMukaGQV296WZrI1/e6EK2QKZwsRPRZNXRLtW g3v7nPADrvBHcqOyEz/BgRkDv7CBMZS3ewR55WbF0qbnQXxp9GPBMvzFgFPCtX X-Received: by 2002:a05:600c:a4a:b0:488:7f69:4abf with SMTP id 5b1f17b1804b1-488997426f9mr10489185e9.12.1775168488839; Thu, 02 Apr 2026 15:21:28 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4887e832585sm415970045e9.6.2026.04.02.15.21.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Apr 2026 15:21:28 -0700 (PDT) Date: Thu, 2 Apr 2026 23:21:26 +0100 From: David Laight To: Yury Norov Cc: Luiz Angelo Daros de Luca , Andrew Lunn , Vladimir Oltean , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Simon Horman , Linus Walleij , Alvin =?UTF-8?B?xaBpcHJhZ2E=?= , Yury Norov , Rasmus Villemoes , Russell King , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [net-next PATCH 04/10] bitfield.h: add FIELD_WIDTH() Message-ID: <20260402232126.2e74631e@pumpkin> In-Reply-To: References: <20260331-realtek_forward-v1-0-44fb63033b7e@gmail.com> <20260331-realtek_forward-v1-4-44fb63033b7e@gmail.com> <20260402102717.5eb48393@pumpkin> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Thu, 2 Apr 2026 09:52:49 -0400 Yury Norov wrote: > On Thu, Apr 02, 2026 at 10:27:17AM +0100, David Laight wrote: > > On Thu, 2 Apr 2026 01:00:20 -0300 > > Luiz Angelo Daros de Luca wrote: > > > > > > > +/** > > > > > + * FIELD_WIDTH() - return the width of a bitfield > > > > > + * @_mask: shifted mask defining the field's length and position > > > > > + * > > > > > + * Returns the number of contiguous bits covered by @_mask. > > > > > + * This corresponds to the bit width of FIELD_MAX(@_mask). > > > > > + */ > > > > > +#define FIELD_WIDTH(_mask) \ > > > > > > > > Please no underscored names unless necessary. > > > > > > I used _mask to maintain consistency with the existing public macros > > > in this file, such as FIELD_GET, FIELD_PREP, FIELD_MAX, and FIELD_FIT. > > > All of them use the underscore prefix for parameters. Should I diverge > > > from them? > > > > > > > > + ({ \ > > > > > + __BF_FIELD_CHECK(_mask, 0ULL, 0ULL, "FIELD_WIDTH: "); \ > > > > > + __bf_shf(~FIELD_MAX(_mask)); \ > > > > > + }) > > > > > > > > I believe, this should be: > > > > > > > > #define FIELD_WIDTH(mask) ({ \ > > > > __BF_FIELD_CHECK_MASK(mask, 0ULL, "FIELD_WIDTH: "); \ > > > > HWEIGHT(mask); \ > > > > }) > > > > > > HWEIGHT() is indeed much cleaner. However, to keep bitfield.h > > > self-contained and avoid adding more includes, I'll try > > > __builtin_popcountll() in a similar way __builtin_ffsll is already > > > used. > > > > > > I also noticed the suggestion to use __BF_FIELD_CHECK_MASK instead of > > > __BF_FIELD_CHECK. Both FIELD_MAX and FIELD_FIT currently use the full > > > __BF_FIELD_CHECK with 0ULL as a dummy register to ensure the mask fits > > > within the header's supported limits. If __BF_FIELD_CHECK_MASK is > > > preferred for FIELD_WIDTH, I can certainly use it, but should we also > > > update FIELD_MAX and FIELD_FIT for consistency? > > > > All of the calls with the 0ULL placeholder (especially for the register) > > should really be removed. > > Last time I looked there where some calls that only had placeholders. > > They just bloat the pre-processor output and slow down compilation. > > Have you any numbers? Can you send a patch? It was all in a patch I send a while back. Basically GENMASK() is a couple of hundred bytes and FIELD_PREP(GENMASK()) hits a few thousand due to the number of times mask (in particular) get expanded. The problem was bit-fields... > > > > I intend to send a v2 with the following implementation: > > > > > > #define __bf_shf(x) (__builtin_ffsll(x) - 1) > > > +#define __bf_hweight(x) __builtin_popcountll(x) > > > > > > #define __scalar_type_to_unsigned_cases(type) \ > > > unsigned type: (unsigned type)0, \ > > > @@ -111,6 +112,19 @@ > > > (typeof(_mask))((_mask) >> __bf_shf(_mask)); \ > > > }) > > > > > > +/** > > > + * FIELD_WIDTH() - return the width of a bitfield > > > + * @_mask: shifted mask defining the field's length and position > > > + * > > > + * Returns the number of contiguous bits covered by @_mask. > > > + * This corresponds to the bit width of FIELD_MAX(@_mask). > > > + */ > > > +#define FIELD_WIDTH(_mask) \ > > > + ({ \ > > > > You ought to have: > > auto _fw_mask = mask; > > here. While _mask has to be a constant, if it comes from GENMASK() > > it is very long. > > Yes, but what this _fw means? Here it could be just auto __mask = mask. > That is what the underscores are used. The problems arise when the expansions get nested. Using unique names is compex (and unreadable). Provided only different defines are nested it is enough to use different names in each one. > > > > + __BF_FIELD_CHECK_MASK(_mask, 0ULL, "FIELD_WIDTH: "); \ > > > + (typeof(_mask))__bf_hweight(_mask); \ > > > > Why the cast of the result? > > They are everywhere in that file, and many are pointless. > > But there is no point adding another one. > > > > I'm not even sure you need the extra define. > > Just use __builtin_popcountll(). > > For __BF_FIELD_CHECK_MASK() check, I guess. I meant why have __bf_hweight(). David