public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Nathan Chancellor <nathan@kernel.org>
To: James Clark <james.clark@linaro.org>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Nick Desaulniers <nick.desaulniers+lkml@gmail.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Mark Brown <broonie@kernel.org>, Vinod Koul <vkoul@kernel.org>,
	Arnd Bergmann <arnd@arndb.de>, Guodong Xu <guodong@riscstar.com>,
	iommu@lists.linux.dev, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev
Subject: Re: [PATCH] dma-mapping: Allow use of DMA_BIT_MASK(64) in global scope
Date: Thu, 30 Oct 2025 11:47:28 -0700	[thread overview]
Message-ID: <20251030184728.GA694763@ax162> (raw)
In-Reply-To: <20251030-james-fix-dma_bit_mask-v1-1-ad1ce7cfab6e@linaro.org>

On Thu, Oct 30, 2025 at 02:05:27PM +0000, James Clark wrote:
> Clang doesn't like that (1ULL<<(64)) overflows when initializing a
> global scope variable, even if that part of the ternary isn't used when
> n = 64. The same initialization can be done without warnings in function
> scopes, and GCC doesn't mind either way.
> 
> The build failure that highlighted this was already fixed in a different
> way [1], which also has detailed links to the Clang issues. However it's
> not going to be long before the same thing happens again, so it's better
> to fix the root cause.

Yeah, it already has since the instance you bring up below:

https://git.kernel.org/gregkh/usb/c/93741bd104ce07a790519ecee3a331ee7cf61ae3

> Fix it by using GENMASK_ULL() which does exactly the same thing, is much
> more readable anyway, and doesn't have a shift that overflows.
> 
> [1]: https://lore.kernel.org/all/20250918-mmp-pdma-simplify-dma-addressing-v1-1-5c2be2b85696@riscstar.com/
> 
> Signed-off-by: James Clark <james.clark@linaro.org>

Thanks, this should hopefully be fixed in clang relatively soon [1] but
you are right that there is nothing to stop this from occuring again,
especially since older clang versions will still be impacted by this
behavior. My basic testing shows that this should be equivalent so:

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

[1]: https://github.com/llvm/llvm-project/pull/163885

> ---
>  include/linux/dma-mapping.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 8248ff9363ee..2ceda49c609f 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -90,7 +90,7 @@
>   */
>  #define DMA_MAPPING_ERROR		(~(dma_addr_t)0)
>  
> -#define DMA_BIT_MASK(n)	(((n) == 64) ? ~0ULL : ((1ULL<<(n))-1))
> +#define DMA_BIT_MASK(n)	GENMASK_ULL(n - 1, 0)
>  
>  struct dma_iova_state {
>  	dma_addr_t addr;
> 
> ---
> base-commit: e53642b87a4f4b03a8d7e5f8507fc3cd0c595ea6
> change-id: 20251030-james-fix-dma_bit_mask-624dbeb89afa
> 
> Best regards,
> -- 
> James Clark <james.clark@linaro.org>
> 

  reply	other threads:[~2025-10-30 18:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20251030140656eucas1p1db40ecf9e84bcabee73b9cfb0b777c4a@eucas1p1.samsung.com>
2025-10-30 14:05 ` [PATCH] dma-mapping: Allow use of DMA_BIT_MASK(64) in global scope James Clark
2025-10-30 18:47   ` Nathan Chancellor [this message]
2025-11-05 12:45   ` Marek Szyprowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251030184728.GA694763@ax162 \
    --to=nathan@kernel.org \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --cc=guodong@riscstar.com \
    --cc=iommu@lists.linux.dev \
    --cc=james.clark@linaro.org \
    --cc=justinstitt@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=m.szyprowski@samsung.com \
    --cc=morbo@google.com \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=robin.murphy@arm.com \
    --cc=sfr@canb.auug.org.au \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox