From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (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 735893ACF1B for ; Sun, 30 Aug 2026 11:43:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788090223; cv=none; b=auvVqcUoXqUjKGzTXrCLc2u7A4XD+zi0nu3jrDLOXzzb3Mq18XZEXgUJd5bxgHdPG7A8dW1bwZuGvxudbfe8elY/4CO0uMm+kt5/z4R5Y+ckb4wC45hG1ebI75cshhmH6wzEL32pVIn8bal247Vr39RJrp09qLfU9XUbDqWXKaE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788090223; c=relaxed/simple; bh=ZSBcoQd59itycfzYZkz0w0S+20hQkmQsFxDYsaMFR3s=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=JsaiJKdg52tVWksT+/a5bHUoSwR36WW3RrMcG2JT27fDqNzdt4qE7r5LRl4UH2LvHfw3ZXVqWgLwfaUfcJcWsRD/HUpFtFKup3GWxcclDK4UO9RrrO0FllDIwpo8/ke45TQhm/Z4XXmh8zi2uzIlN5YBb5emeio4Lf+Lv/5+55s= 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=edPIi5Q+; arc=none smtp.client-ip=209.85.128.41 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="edPIi5Q+" Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-49b8eeb3ff2so17600065e9.2 for ; Sun, 30 Aug 2026 04:43:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1788090219; x=1788695019; darn=vger.kernel.org; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=WlfkYyar9vumibw7mRgIAgoFjQzAhszDTK4deysoT4I=; b=edPIi5Q+OpL5K0wrWuEECGgjvO4wRDh/IjeR4XbmFuaGG73HoijSuPnQbZG38eTRw+ lcUdJ6vRtCgWLBdicYlvHczEQ8qOuDIjZnzkt58Pog9a4mVqXvNyciBl4VaCCReEYS5g ZWXEm9BNIJ3N6eTaM8HmNAErtpiabk/IqXmM12kMUfpMXPWlrVd5BLEsaclw7gB0lCKt 0srVDVG7f2/c4l1WCuXz5ssn78n8j5Fa5iYSyFtk0Mf6JEQmewRpQnOZv4DPaAPOtffY eLRm0lfPFHBBLpVvoEX77uWxH/Vv9PRUHS/IXEsFFLRsrNK3WgJmxFTxbvFNchakbBQC QR1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1788090219; x=1788695019; h=content-transfer-encoding:content-type: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 :content-type; bh=WlfkYyar9vumibw7mRgIAgoFjQzAhszDTK4deysoT4I=; b=TXovXSmgFxACCtfUYamB5isWqhMH09cXykclMCSi3wJAD+wx1rFsZAt7mhPI+q/8Dh 0v4IjTloOEdTWTILFemjetik/NtYApUR6T+cyMXC+9hhfD+xcbjkKDvhCCsvSW0pL0Hl BAIXdC9gaY/AKMLSwHUOhl/i2Lo5TaF4+08v7z+TFvWh6s6AjxeaIyoVJgjMQ+wjDkvB Gpvyi8rUMeDMdZrispN4kq9r46l8k635xUE/ZlFknj4Zsmd4QKtzOTPgmrsHcptgSbSx Th51Hsz1TTTD9K/T+JzdOWbBh532YhWXSp3aSJgjYrwYR/yB/udBU4aNg/UuXmsj3SAW 9jVQ== X-Forwarded-Encrypted: i=1; AHgh+Rpz1MsIK1gXTH9iZGy51yOsk2XIX45AXxdy7JhHjtUPhBIEMieFC4Bhwb1V2kzgg80GYvlX6RXOz2XIblo=@vger.kernel.org X-Gm-Message-State: AFuF++ktdX3TcBsZOIBQwpuU/qGsHy6tsRhQOrI7PdTWRKKzP9faelOl NAx9HudHtcR9gdPDKfi1lzlmlPmeYpy5CjHwe2j82qKNRkn2Z4dVBoCK X-Gm-Gg: AR+sD13c6xkmqGaaFHkr7+M9bxmu8labitDew3I68I+8SQJ98//1221642MlSl69TXf 4BIKxCQF1aLhiS1rIQPj6tQKhGubeYLxIyMiBhMmJOUQJ0JNlL6Xi76rhMs9fxjH/8xoYsIXZ+J VkWheWwngNfT6V3ds6qTHdG8MQMEAxBpljXNMw6MuugzT1819oHzA21/iWfM1b9qcKXE0xrqLoG Ob6kWEY+qTD7RR0k8WDekyL0ScOQ4DJTM1M6kSJ22J+3mR71AiAjPcXqyVUoj8jUKS3WTp8r9RT XE/bvMdYdoANhUiqXlgSwSQpjfVqXeyH1eeHFJ2sL4sa0qBH+Ep3hJ3cGustDkdTbPVcmhsnWVO 1dKFmHBjCOqrcSN+xuNLz14OCgYQeBdVyXT9B+xnPVuW/M2lYGdGKJyjv/cCW0q1N+8vPjVYP4R 9JZgcIi30+45D9OfVVDS2+9HJF2GX0rDJ102lxl6Bym8DIbaWTWqn2dyY3XaQm8ke9D6SnAHmzS pxxi1yFNKTLZVrDgyx4icXtaz6XWHETWjcD X-Received: by 2002:a05:600c:a016:b0:49c:d60c:74e0 with SMTP id 5b1f17b1804b1-49cd60c74eemr23912455e9.13.1788090219141; Sun, 30 Aug 2026 04:43:39 -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-49b94dc1076sm195266535e9.3.2026.08.30.04.43.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Aug 2026 04:43:38 -0700 (PDT) Date: Sun, 30 Aug 2026 12:43:34 +0100 From: David Laight To: David Gow Cc: Jim Cromie , "Maciej W . Rozycki" , Andrew Morton , Matthew Auld , Arun Pravin , Joel Fernandes , David Airlie , Simona Vetter , Chris Mason , David Sterba , dri-devel@lists.freedesktop.org, linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/3] linux/log2.h: Add 64-bit safe variants of power-of-two functions Message-ID: <20260830124334.105b84b1@pumpkin> In-Reply-To: <20260830103321.2042968-1-david@davidgow.net> References: <20260830103321.2042968-1-david@davidgow.net> 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 Sun, 30 Aug 2026 18:33:15 +0800 David Gow wrote: > The existing roundup_pow_of_two() and rounddown_pow_of_two() functions work > on values of type unsigned long, which is 32-bit on 32-bit systems. > Equally, is_power_of_2() operates on an unsigned long. > > There are several instances where 64-bit safe versions of these (which > operate on a 64-bit value regardless of sizeof(long)) are required. Most > particularly, some hardware (especially GPUs) have 64-bit address spaces, > and some formats (such as filesystems) use 64-bit offsets. Some of these > (such as i915 and btrfs) have already implemented their own 64-bit > is_power_of_2() helpers. > > Add a version of these which always operate on a 64-bit value. These have > the (unimaginative) names: > - is_power_of_2_u64() > - roundup_pow_of_two_u64(), and > - rounddown_pow_of_two_u64() > and otherwise work identically to their unsigned long counterparts. Why not just change the definitions (back?) to #defines. Then they can be size neutral and you don't have to guess the correct one. You may need to use __builtin_constant_p(x <= ~0u) to select between 32 and 64 bit versions. It is also worth checking what gcc/clang generate for the 64bit versions on 32bit when passed a 32bit variable. It might be that they optimise the code and avoid all the 64bit maths. David > > To avoid conflicts, the i915 implementation is also removed here. The btrfs > one (which has a different name) is replaced in a separate patch. > > Signed-off-by: David Gow > --- > > This patch adds u64 helpers, and the following two use them. And v2 also > has the i915 change to remove the conflicting implementation. > > So I'm not sure who best wants to take these. Ultimately it's an include/linux > change, but it touches i915, patch 2 touches GPU/DRM, and patch 3 btrfs. > Personally, I'm keen to get patch 2 in, as it fixes a real issue, so if taking > 1 and 2 via DRM makes more sense, that's fine by me. > > Changes since v1: > https://lore.kernel.org/all/20260821091918.1902032-1-david@ingeniumdigital.com/ > - Include is_power_of_2_u64() as well, and remove the i915 version > (Thanks, Matthew) > - Use _u64 as a suffix for the 64-bit versions, not just 64 > (This is a much nicer name, and matches what everone else was doing) > - Fix some comment typos. > - Add a third patch which removes a similar is_power_of_two_u64() helper > from btrfs. > > --- > drivers/gpu/drm/i915/i915_utils.h | 5 --- > include/linux/log2.h | 73 +++++++++++++++++++++++++++++++ > 2 files changed, 73 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_utils.h b/drivers/gpu/drm/i915/i915_utils.h > index ecc20e0528f4..1cec51984d8c 100644 > --- a/drivers/gpu/drm/i915/i915_utils.h > +++ b/drivers/gpu/drm/i915/i915_utils.h > @@ -75,11 +75,6 @@ struct drm_i915_private; > __idx; \ > }) > > -static inline bool is_power_of_2_u64(u64 n) > -{ > - return (n != 0 && ((n & (n - 1)) == 0)); > -} > - > void add_taint_for_CI(struct drm_i915_private *i915, unsigned int taint); > static inline void __add_taint_for_CI(unsigned int taint) > { > diff --git a/include/linux/log2.h b/include/linux/log2.h > index e17ceb32e0c9..fc44e59f5732 100644 > --- a/include/linux/log2.h > +++ b/include/linux/log2.h > @@ -47,6 +47,22 @@ bool is_power_of_2(unsigned long n) > return n - 1 < (n ^ (n - 1)); > } > > +/** > + * is_power_of_2_u64() - check if a 64-bit value is a power of two > + * @n: the value to check > + * > + * Determine whether some value is a power of two, where zero is > + * *not* considered a power of two. Unlike is_power_of_2, this version > + * always operates on 64-bit values, even on 32-bit architectures where > + * long is 32-bit. > + * Return: true if @n is a power of 2, otherwise false. > + */ > +static __always_inline __attribute_const__ > +bool is_power_of_2_u64(u64 n) > +{ > + return n - 1 < (n ^ (n - 1)); > +} > + > /** > * __roundup_pow_of_two() - round up to nearest power of two > * @n: value to round up > @@ -195,6 +211,63 @@ unsigned long __rounddown_pow_of_two(unsigned long n) > __rounddown_pow_of_two(n) \ > ) > > +/** > + * __rounddown_pow_of_two_64() - round a 64-bit value down to nearest power of two > + * @n: value to round down > + */ > +static inline __attribute_const__ > +u64 __rounddown_pow_of_two_u64(u64 n) > +{ > + return 1ULL << ilog2(n); > +} > + > +/** > + * rounddown_pow_of_two_u64 - round a 64-bit value down to nearest power of two > + * @n: parameter > + * > + * round the given value down to the nearest power of two > + * - this always operates on 64-bit values, even on 32-bit systems > + * - the result is undefined when n == 0 > + * - this can be used to initialise global variables from constant data > + */ > +#define rounddown_pow_of_two_u64(n) \ > +( \ > + __builtin_constant_p(n) ? ( \ > + ((n) == 1) ? 1ULL : \ > + (1ULL << ilog2((n))) \ > + ) : \ > + __rounddown_pow_of_two_u64(n) \ > +) > + > + > +/** > + * __roundup_pow_of_two_u64() - round a 64-bit value up to nearest power of two > + * @n: value to round up > + */ > +static inline __attribute_const__ > +u64 __roundup_pow_of_two_u64(u64 n) > +{ > + return 1ULL << (ilog2(n - 1) + 1); > +} > + > +/** > + * roundup_pow_of_two_u64 - round a 64-bit value up to nearest power of two > + * @n: parameter > + * > + * round the given value up to the nearest power of two > + * - this always operates on 64-bit values, even on 32-bit systems > + * - the result is undefined when n == 0 > + * - this can be used to initialise global variables from constant data > + */ > +#define roundup_pow_of_two_u64(n) \ > +( \ > + __builtin_constant_p(n) ? ( \ > + ((n) == 1) ? 1ULL : \ > + (1ULL << (ilog2((n) - 1) + 1)) \ > + ) : \ > + __roundup_pow_of_two_u64(n) \ > +) > + > static inline __attribute_const__ > int __order_base_2(unsigned long n) > {