fs/ext2/Makefile | 2 ++ fs/ext2/balloc.c | 4 ++++ fs/ext2/ext2.h | 19 +++++++++++-------- fs/ext2/inode.c | 7 +++++++ fs/ext2/super.c | 35 +++++++++++++++++++---------------- 5 files changed, 43 insertions(+), 24 deletions(-)
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!
Changes from v1:
* A false positive has been fixed in Clang [3] and ported to Clang 23.
Hence, the first patch (silencing that false positive) has been dropped.
* Use guard() and scoped_guard() instead of context_unsafe(), when
possible, to express that certain fields are being protected by a newly
initialized lock during init.
Link to v1: https://lore.kernel.org/linux-fsdevel/20260712165610.366474-1-timday@thelustrecollective.com/
[1] https://lore.kernel.org/lkml/20251219154418.3592607-1-elver@google.com/
[2] https://lore.kernel.org/all/20260706141452.3008233-1-nilay@linux.ibm.com/
[3] https://github.com/llvm/llvm-project/pull/209796
Timothy Day (8):
ext2: mark s_next_generation as guarded by s_next_gen_lock
ext2: annotate ext2_update_dynamic_rev() as requiring s_lock
ext2: mark statfs overhead cache as guarded by s_lock
ext2: mark s_mount_state as guarded by s_lock
ext2: annotate ext2_init_block_alloc_info() as requiring
truncate_mutex
ext2: annotate block-mapping helpers as requiring truncate_mutex
ext2: annotate s_rsv_window_root as requiring s_rsv_window_lock
ext2: enable context analysis support for ext2 filesystem
fs/ext2/Makefile | 2 ++
fs/ext2/balloc.c | 4 ++++
fs/ext2/ext2.h | 19 +++++++++++--------
fs/ext2/inode.c | 7 +++++++
fs/ext2/super.c | 35 +++++++++++++++++++----------------
5 files changed, 43 insertions(+), 24 deletions(-)
--
2.43.0
Hello! 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 :). Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
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)))
| ^
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'
--
Cheers,
Nathan
On Thu, 3 Sept 2026 at 09:28, Nathan Chancellor <nathan@kernel.org> 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@google.com/
> 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
On Thu 03-09-26 12:34:34, Marco Elver wrote:
> On Thu, 3 Sept 2026 at 09:28, Nathan Chancellor <nathan@kernel.org> wrote:
> > 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.
Yes, at the beginning of ext4_fill_super() we do:
sbi = kzalloc_obj(*sbi);
...
sb->s_fs_info = sbi;
and EXT2_SB(sb) is just sb->s_fs_info. Are you saying that clang is not
able to infer that sb->s_fs_info and sbi are still pointing to the same
memory later in the function where we do scoped_guard()?
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
On Thu, 3 Sept 2026 at 13:06, Jan Kara <jack@suse.cz> wrote:
>
> On Thu 03-09-26 12:34:34, Marco Elver wrote:
> > On Thu, 3 Sept 2026 at 09:28, Nathan Chancellor <nathan@kernel.org> wrote:
> > > 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.
>
> Yes, at the beginning of ext4_fill_super() we do:
>
> sbi = kzalloc_obj(*sbi);
> ...
> sb->s_fs_info = sbi;
>
> and EXT2_SB(sb) is just sb->s_fs_info. Are you saying that clang is not
> able to infer that sb->s_fs_info and sbi are still pointing to the same
> memory later in the function where we do scoped_guard()?
Yes - alias tracking is only done through local aliases. sb is
non-local, along with additional member indirection; unfortunately,
the C language doesn't give us the guarantees that it wasn't modified
somewhere in between, say after a function call (this rule is applied
also for local aliases if clang sees that they "escape" their local
scope via non-const pointer to pointer).
On Thu 03-09-26 14:06:22, Marco Elver wrote:
> On Thu, 3 Sept 2026 at 13:06, Jan Kara <jack@suse.cz> wrote:
> >
> > On Thu 03-09-26 12:34:34, Marco Elver wrote:
> > > On Thu, 3 Sept 2026 at 09:28, Nathan Chancellor <nathan@kernel.org> wrote:
> > > > 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.
> >
> > Yes, at the beginning of ext4_fill_super() we do:
> >
> > sbi = kzalloc_obj(*sbi);
> > ...
> > sb->s_fs_info = sbi;
> >
> > and EXT2_SB(sb) is just sb->s_fs_info. Are you saying that clang is not
> > able to infer that sb->s_fs_info and sbi are still pointing to the same
> > memory later in the function where we do scoped_guard()?
>
> Yes - alias tracking is only done through local aliases. sb is
> non-local, along with additional member indirection; unfortunately,
> the C language doesn't give us the guarantees that it wasn't modified
> somewhere in between, say after a function call (this rule is applied
> also for local aliases if clang sees that they "escape" their local
> scope via non-const pointer to pointer).
Hrm, ok, understood (but still it's annoying ;)). I've pushed out the patch
with the suggested fixup.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
On Thu, 03 Sep 2026 14:43:03 +0200, Jan Kara wrote:
> On Thu 03-09-26 14:06:22, Marco Elver wrote:
> > On Thu, 3 Sept 2026 at 13:06, Jan Kara <jack@suse.cz> wrote:
> > >
> > > On Thu 03-09-26 12:34:34, Marco Elver wrote:
> > > > On Thu, 3 Sept 2026 at 09:28, Nathan Chancellor <nathan@kernel.org> wrote:
> > > > > 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.
> > >
> > > Yes, at the beginning of ext4_fill_super() we do:
> > >
> > > sbi = kzalloc_obj(*sbi);
> > > ...
> > > sb->s_fs_info = sbi;
> > >
> > > and EXT2_SB(sb) is just sb->s_fs_info. Are you saying that clang is not
> > > able to infer that sb->s_fs_info and sbi are still pointing to the same
> > > memory later in the function where we do scoped_guard()?
> >
> > Yes - alias tracking is only done through local aliases. sb is
> > non-local, along with additional member indirection; unfortunately,
> > the C language doesn't give us the guarantees that it wasn't modified
> > somewhere in between, say after a function call (this rule is applied
> > also for local aliases if clang sees that they "escape" their local
> > scope via non-const pointer to pointer).
>
> Hrm, ok, understood (but still it's annoying ;)). I've pushed out the patch
> with the suggested fixup.
Sorry for the late reply. I was out last month due to lung trouble
(spontaneous pneumothorax corrected via surgery). Thanks Jan for
pulling the series. I took a look at your branch [2] and everything
seems fine. I agree the scoped guards can be peculiar. If you
wanted to make some minor changes, I have no objection.
Thanks Marco for the compilation fix in the other thread [1]. It seems
like a reasonable fix. If there is anything else needed from me, let
me know.
Now that I'm back, I'm hoping to resume work on some more context
analysis patches. I have a few that were close to done that I
hope to share soon.
Tim Day
[1] https://lore.kernel.org/all/20260903101843.3462767-1-elver@google.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git/log/?h=for_next
On Tue, 11 Aug 2026 at 18:04, Timothy Day <timday@thelustrecollective.com> 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! > > Changes from v1: > > * A false positive has been fixed in Clang [3] and ported to Clang 23. > Hence, the first patch (silencing that false positive) has been dropped. > * Use guard() and scoped_guard() instead of context_unsafe(), when > possible, to express that certain fields are being protected by a newly > initialized lock during init. Acked-by: Marco Elver <elver@google.com> But ultimately up to maintainers. Also, thanks for helping improve the Clang side (FWIW, Clang 23 will release August 23)! > Link to v1: https://lore.kernel.org/linux-fsdevel/20260712165610.366474-1-timday@thelustrecollective.com/ > > [1] https://lore.kernel.org/lkml/20251219154418.3592607-1-elver@google.com/ > [2] https://lore.kernel.org/all/20260706141452.3008233-1-nilay@linux.ibm.com/ > [3] https://github.com/llvm/llvm-project/pull/209796 > > Timothy Day (8): > ext2: mark s_next_generation as guarded by s_next_gen_lock > ext2: annotate ext2_update_dynamic_rev() as requiring s_lock > ext2: mark statfs overhead cache as guarded by s_lock > ext2: mark s_mount_state as guarded by s_lock > ext2: annotate ext2_init_block_alloc_info() as requiring > truncate_mutex > ext2: annotate block-mapping helpers as requiring truncate_mutex > ext2: annotate s_rsv_window_root as requiring s_rsv_window_lock > ext2: enable context analysis support for ext2 filesystem > > fs/ext2/Makefile | 2 ++ > fs/ext2/balloc.c | 4 ++++ > fs/ext2/ext2.h | 19 +++++++++++-------- > fs/ext2/inode.c | 7 +++++++ > fs/ext2/super.c | 35 +++++++++++++++++++---------------- > 5 files changed, 43 insertions(+), 24 deletions(-) > > -- > 2.43.0 >
© 2016 - 2026 Red Hat, Inc.