Re: [PATCH] nvme-pci: Make nvme_pci_npages_prp() __always_inline

[Kees Cook <kees@kernel.org>]

On Wed, May 07, 2025 at 08:59:13AM +0200, Christoph Hellwig wrote:
> On Tue, May 06, 2025 at 10:55:31PM -0700, Kees Cook wrote:
> > On Wed, May 07, 2025 at 06:47:54AM +0200, Christoph Hellwig wrote:
> > > On Tue, May 06, 2025 at 08:35:40PM -0700, Kees Cook wrote:
> > > > The only reason nvme_pci_npages_prp() could be used as a compile-time
> > > > known result in BUILD_BUG_ON() is because the compiler was always choosing
> > > > to inline the function. Under special circumstances (sanitizer coverage
> > > > functions disabled for __init functions on ARCH=um), the compiler decided
> > > > to stop inlining it:
> > > 
> > > Can we place just fix um to still force inlining inline functions instead
> > > of needing these workarounds?
> > 
> > Oh, I don't have the history here. Is there something about UM and
> > forcing off inlining?
> 
> Maybe I'm misunderstandng your report, but what causes the failure
> to inline?

I don't know precisely, but whatever internal heuristics the compiler
uses to change a function from "static" to "static inline" got disrupted
by the build options, and manifested with this failure. It's fully
reproducible on all architectures if I mark the function as "noinline".
:)

So, the solution for the "accidentally depending on a function to be
inlined by the compiler" is to mark it as _required_ to be inlined,
which given its singular use in BUILD_BUG_ON(), looks like the correct
solution.

I took your comment about ARCH=um to mean there was some kind of
long-standing "UM regularly fails to inline stuff; can we fix UM
instead?" But regardless, I think this patch is still correct given
that the compiler could, at any time, decide to make this function not
inline, since it's not marked that way at all (but its usage depends on
it being inline).

-Kees

-- 
Kees Cook