Re: [PATCH v3 02/12] block/bdev: Annotate the blk_holder_ops callback invocations

Marco Elver
2026-04-13T20:04:02Z

On Mon, 13 Apr 2026 at 18:06, Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 4/12/26 11:00 PM, Marco Elver wrote:
> > For now, that's intended - focus is on getting rid of false positives.
> > Doing what you suggest above would require abusing the attributes to
> > effectively become type qualifiers; that's generally questionable,
> > because that's not what attributes are meant to do (and reworking TSA
> > to provide real type-qualifiers is an intrusion into the type system
> > that's also a hard sell). So for now, I'll try to do the simpler
> > change, and later may look into adding -Wthread-safety specific
> > warnings that points out such mismatches.
>
> Is there agreement that the Linux kernel Clang build shouldn't break, no
> matter what version of Clang is used to build the kernel (ignoring Clang
> bugs)? If so, I think we either need a new macro to annotate function
> pointers or thread-safety checking has to be disabled for Clang versions
> that do not support annotating function pointers.
>
> If I annotate function pointers with __releases() then the following
> error message is reported with a Clang compiler built from the LLVM main
> branch:

Note the function pointer support hasn't been merged yet - waiting for
someone to review my LLVM PR...

> In file included from block/blk-core.c:16:
> In file included from ./include/linux/module.h:20:
> In file included from ./include/linux/elf.h:6:
> In file included from ./arch/x86/include/asm/elf.h:10:
> In file included from ./arch/x86/include/asm/ia32.h:7:
> In file included from ./include/linux/compat.h:17:
> ./include/linux/fs.h:2309:55: error: use of undeclared identifier 'sb'
> 2309 | void (*kill_sb) (struct super_block *sb)
> __releases(&sb->s_umount);
> |
>
> How to prevent such error messages? Is adding the capability to the
> Linux kernel to detect whether or not annotating function pointers is
> supported and introducing a new macro for annotating function pointers
> the only solution?
>
> Additionally, the __release() annotations introduced by this patch will
> have to be surrounded by an #ifdef/#endif pair or thread-safety checking
> in the Linux kernel will have to be disabled for Clang versions that do
> not support adding attributes to function pointers.

Either conditionally enabling differently named macros as you suggest,
or simply bumping the Clang compiler version. I think for Context
Analysis specifically we're better off bumping the required compiler
version, because otherwise we end up with a mess of macros we have to
undo later.

I already sent the patch to bump min. Clang to 23 for Context
Analysis: https://lore.kernel.org/all/acqELlVC6vEeEF-W@elver.google.com/
[1]

While for 1-2 new features we might be fine with more macros (as I
initially tried for the multi-arg guarded_by), I think that doesn't
scale if we need to support more versions and even more features. I
think that's also what Peter suggested, and last I checked he's just
waiting for the dev version of Clang 23 to be new enough to take the
above patch [1].

I hope the function pointer support will land in Clang 23 as well
(when it will be released in ~August). So once the function pointer
support lands, we should definitely bump min. Clang version.

While it's not that convenient that Clang 22 will become unsupported
for Context Analysis, I think in the longer term we're better off
(cleaner code, less quirks, easier to maintain). I think we suspected
we'll have to bump the required version sooner than later - but
hopefully Clang 23 is "the one." ;-)