Re: [PATCH v2 0/8] Support Clang context analysis for ext2

[Marco Elver]

On Thu, 3 Sept 2026 at 09:28, Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
> On Tue, Aug 18, 2026 at 11:58:20AM +0200, Jan Kara wrote:
> > On Tue 11-08-26 12:03:28, Timothy Day wrote:
> > > This description is mostly copied from v1:
> > >
> > > This series adds annotations for Clang's context analysis to ext2.
> > > Clang context analysis was recently added in a series by Marco
> > > Elver [1]. This allows the compiler to validate different
> > > locking patterns at compile time.
> > >
> > > This series enables context analysis, fixes pre-existing warnings,
> > > and adds new annotations. It is inspired by similar series in the
> > > block layer (NVMe host driver, for example [2]).
> > >
> > > I'm starting with ext2 since it's smaller and simpler compared to
> > > ext4/btrfs/etc. After ext2, I'd be interested in converting the
> > > other filesystems and infrastructure code in fs/. I think the ultimate
> > > goal would be to enable this by default across all of fs/.
> > >
> > > The series was built and tested with Clang 23 with
> > > CONFIG_WARN_CONTEXT_ANALYSIS enabled. I based on 7.2-rc7.
> >
> > Thanks for the patches! They look good to me. Once the merge window is over
> > I'll queue them to my tree. The only thing I'm not fully sure is how much I
> > like the spinlock_init scoped guards - they looked quite confusing to me at
> > the first sight (as much as I understand the convenience, conceptually how
> > can initialization of a global lock be scoped?). I'll sleep over it, maybe
> > I'll change them to just spinlock_init() + scoped_guard for the lock itself
> > or maybe I'll get used to them. Anyway, no action on your side needed :).
>
> This series is now in -next, where I see the following warnings (or errors with
> CONFIG_WERROR=y / W=e) with various configurations, such as ARCH=arm
> allmodconfig, when building with LLVM 23.1.0
>
>   fs/ext2/xattr.c:825:6: error: rw_semaphore 'EXT2_I().xattr_sem' is not held on every path through here [-Werror,-Wthread-safety-analysis]
>     825 |         if (WARN_ON_ONCE(!down_write_trylock(&EXT2_I(inode)->xattr_sem)))
>         |             ^
>   include/asm-generic/bug.h:180:2: note: expanded from macro 'WARN_ON_ONCE'
>     180 |         DO_ONCE_LITE_IF(condition, WARN_ON, 1)
>         |         ^
>   include/linux/once_lite.h:30:7: note: expanded from macro 'DO_ONCE_LITE_IF'
>      30 |                 if (__ONCE_LITE_IF(__ret_do_once))                      \
>         |                     ^
>   include/linux/once_lite.h:23:3: note: expanded from macro '__ONCE_LITE_IF'
>      23 |                 unlikely(__ret_once);                                   \
>         |                 ^
>   include/linux/compiler.h:77:22: note: expanded from macro 'unlikely'
>      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
>         |                         ^
>   fs/ext2/xattr.c:825:20: note: rw_semaphore acquired here
>     825 |         if (WARN_ON_ONCE(!down_write_trylock(&EXT2_I(inode)->xattr_sem)))
>         |                           ^

This one can be fixed with (should also be a bit more efficient):
https://lore.kernel.org/all/20260903101843.3462767-1-elver@xxxxxxxxxx/

>   fs/ext2/super.c:1149:3: error: calling function 'ext2_rsv_window_add' requires holding spinlock 'EXT2_SB(sb).s_rsv_window_lock' exclusively [-Werror,-Wthread-safety-precise]
>    1149 |                 ext2_rsv_window_add(sb, &sbi->s_rsv_window_head);
>         |                 ^
>   fs/ext2/super.c:1149:3: note: found near match '_res->s_rsv_window_lock'

sbi was just allocated, and EXT2_SB(sb) and sbi are pointing to the
same object (unless I misread the code), but the compiler can't tell
since aliases can't be tracked through non-local objects. So that
scoped_guard could just become:

  scoped_guard(spinlock, &EXT2_SB(sb)->s_rsv_window_lock) {
  ...

But I'll leave that to Jan and Tim.

Thanks,
-- Marco