[PATCH v1 0/9] Support Clang context analysis for ext2

[PATCH v1 0/9] Support Clang context analysis for ext2

[Timothy Day]

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-rc2 (since the
minimum Clang version was recently bumped to 23 in 7.2+ [3]).

I'd appreciate reviews and suggestions. I'm especially interested in
suggestions on how we can make this easier to enable for other
filesystems.

Thanks!

[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://lore.kernel.org/all/20260515124426.2227783-1-elver@google.com/

Timothy Day (9):
  ext2: fix ext2_xattr_delete_inode() context analysis warning
  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: enabled 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  | 18 +++++++++++++-----
 fs/ext2/xattr.c  |  4 +++-
 6 files changed, 40 insertions(+), 14 deletions(-)

-- 
2.39.5

[PATCH v1 1/9] ext2: fix ext2_xattr_delete_inode() context analysis warning

[Timothy Day]

Clang reports the following warning when context analysis is enabled:

fs/ext2/xattr.c:829:6: error: rw_semaphore 'EXT2_I().xattr_sem' is not \
  held on every path through here [-Werror,-Wthread-safety-analysis]
  829 |         if (WARN_ON_ONCE(!down_write_trylock(&EXT2_I(inode)->xattr_sem)))
      |             ^

When WARN_ON_ONCE() wraps down_write_trylock(), it adds an unlikely()
annotation which hides the conditional acquire annotation from Clang's
context analysis. This triggers a false positive. Pull the trylock out
of the macro to silence this warning.

Signed-off-by: Timothy Day <timday@thelustrecollective.com>
---
 fs/ext2/xattr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index e55d16abf422f..e5ed27f2ae733 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -826,8 +826,10 @@ ext2_xattr_delete_inode(struct inode *inode)
 	 * here to avoid false-positive warning from lockdep about reclaim
 	 * circular dependency.
 	 */
-	if (WARN_ON_ONCE(!down_write_trylock(&EXT2_I(inode)->xattr_sem)))
+	if (!down_write_trylock(&EXT2_I(inode)->xattr_sem)) {
+		WARN_ON_ONCE(1);
 		return;
+	}
 	if (!EXT2_I(inode)->i_file_acl)
 		goto cleanup;
 
-- 
2.39.5

[PATCH v1 2/9] ext2: mark s_next_generation as guarded by s_next_gen_lock

[Timothy Day]

s_next_generation is only ever modified while holding s_next_gen_lock
(in ext2_new_inode()), so annotate it with __guarded_by() for Clang's
context analysis.

The only other write is the initialisation in ext2_fill_super(), which
runs before the superblock is live. No concurrent access should be
possible. Since the s_next_gen_lock isn't being taken in this
case, wrap it in context_unsafe().

Signed-off-by: Timothy Day <timday@thelustrecollective.com>
---
 fs/ext2/ext2.h  | 2 +-
 fs/ext2/super.c | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 79f7b395258c2..e851d2a6be662 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -93,7 +93,7 @@ struct ext2_sb_info {
 	int s_inode_size;
 	int s_first_ino;
 	spinlock_t s_next_gen_lock;
-	u32 s_next_generation;
+	u32 s_next_generation __guarded_by(&s_next_gen_lock);
 	unsigned long s_dir_count;
 	u8 *s_debts;
 	struct percpu_counter s_freeblocks_counter;
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 3999f8f3b156e..8c7f3772917bb 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1125,9 +1125,11 @@ static int ext2_fill_super(struct super_block *sb, struct fs_context *fc)
 		goto failed_mount2;
 	}
 	sbi->s_gdb_count = db_count;
-	sbi->s_next_generation = get_random_u32();
 	spin_lock_init(&sbi->s_next_gen_lock);
 
+	/* concurrent access is not possible before the mount completes */
+	context_unsafe(sbi->s_next_generation = get_random_u32());
+
 	/* per filesystem reservation list head & lock */
 	spin_lock_init(&sbi->s_rsv_window_lock);
 	sbi->s_rsv_window_root = RB_ROOT;
-- 
2.39.5

[PATCH v1 3/9] ext2: annotate ext2_update_dynamic_rev() as requiring s_lock

[Timothy Day]

ext2_update_dynamic_rev() modifies several fields within
'struct ext2_super_block' and is already documented as requiring
s_lock. Express that with __must_hold() for Clang's context analysis.

Both callers ext2_xattr_update_super_block() and __ext2_write_inode()
already hold s_lock.

Signed-off-by: Timothy Day <timday@thelustrecollective.com>
---
 fs/ext2/ext2.h  | 3 ++-
 fs/ext2/super.c | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index e851d2a6be662..c8bf2e69675dc 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -760,7 +760,8 @@ extern __printf(3, 4)
 void ext2_error(struct super_block *, const char *, const char *, ...);
 extern __printf(3, 4)
 void ext2_msg(struct super_block *, const char *, const char *, ...);
-extern void ext2_update_dynamic_rev (struct super_block *sb);
+extern void ext2_update_dynamic_rev(struct super_block *sb)
+	__must_hold(&EXT2_SB(sb)->s_lock);
 extern void ext2_sync_super(struct super_block *sb, struct ext2_super_block *es,
 			    int wait);
 
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index 8c7f3772917bb..f742b9b6b017a 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -127,6 +127,7 @@ void ext2_msg(struct super_block *sb, const char *prefix,
  * This must be called with sbi->s_lock held.
  */
 void ext2_update_dynamic_rev(struct super_block *sb)
+	__must_hold(&EXT2_SB(sb)->s_lock)
 {
 	struct ext2_super_block *es = EXT2_SB(sb)->s_es;
 
-- 
2.39.5

[PATCH v1 4/9] ext2: mark statfs overhead cache as guarded by s_lock

[Timothy Day]

s_overhead_last and s_blocks_last cache the filesystem overhead
computation and are only ever accessed in ext2_statfs() under s_lock.
Annotate them with __guarded_by() for Clang's context analysis.

Signed-off-by: Timothy Day <timday@thelustrecollective.com>
---
 fs/ext2/ext2.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index c8bf2e69675dc..ecca898653eb8 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -77,8 +77,8 @@ struct ext2_sb_info {
 	unsigned long s_gdb_count;	/* Number of group descriptor blocks */
 	unsigned long s_desc_per_block;	/* Number of group descriptors per block */
 	unsigned long s_groups_count;	/* Number of groups in the fs */
-	unsigned long s_overhead_last;  /* Last calculated overhead */
-	unsigned long s_blocks_last;    /* Last seen block count */
+	unsigned long s_overhead_last __guarded_by(&s_lock);  /* Last calculated overhead */
+	unsigned long s_blocks_last __guarded_by(&s_lock);    /* Last seen block count */
 	struct buffer_head * s_sbh;	/* Buffer containing the super block */
 	struct ext2_super_block * s_es;	/* Pointer to the super block in the buffer */
 	struct buffer_head ** s_group_desc;
-- 
2.39.5

[PATCH v1 5/9] ext2: mark s_mount_state as guarded by s_lock

[Timothy Day]

s_mount_state tracks the filesystem's mount/error state and is protected
by s_lock everywhere it is accessed. Annotate it with __guarded_by() for
Clang's context analysis.

ext2_setup_super() reads s_mount_state and is called with s_lock held
from the remount path, so annotate it with __must_hold().

There are two accesses (the initial read in ext2_fill_super() and the
setup_super() call) in the mount flow. This is before the superblock
is live, so no concurrent access should be possible. Since the s_lock
isn't being taken in this case, wrap them in context_unsafe().

Signed-off-by: Timothy Day <timday@thelustrecollective.com>
---
 fs/ext2/ext2.h  | 2 +-
 fs/ext2/super.c | 7 +++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index ecca898653eb8..4c30d6f5c9726 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -86,7 +86,7 @@ struct ext2_sb_info {
 	unsigned long s_sb_block;
 	kuid_t s_resuid;
 	kgid_t s_resgid;
-	unsigned short s_mount_state;
+	unsigned short s_mount_state __guarded_by(&s_lock);
 	unsigned short s_pad;
 	int s_addr_per_block_bits;
 	int s_desc_per_block_bits;
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index f742b9b6b017a..a5a5285dd5784 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -633,6 +633,7 @@ static int ext2_parse_param(struct fs_context *fc, struct fs_parameter *param)
 static int ext2_setup_super (struct super_block * sb,
 			      struct ext2_super_block * es,
 			      int read_only)
+	__must_hold(&EXT2_SB(sb)->s_lock)
 {
 	int res = 0;
 	struct ext2_sb_info *sbi = EXT2_SB(sb);
@@ -1035,7 +1036,8 @@ static int ext2_fill_super(struct super_block *sb, struct fs_context *fc)
 	sbi->s_desc_per_block = sb->s_blocksize /
 					sizeof (struct ext2_group_desc);
 	sbi->s_sbh = bh;
-	sbi->s_mount_state = le16_to_cpu(es->s_state);
+	/* concurrent access is not possible before the mount completes */
+	context_unsafe(sbi->s_mount_state = le16_to_cpu(es->s_state));
 	sbi->s_addr_per_block_bits =
 		ilog2 (EXT2_ADDR_PER_BLOCK(sb));
 	sbi->s_desc_per_block_bits =
@@ -1203,7 +1205,8 @@ static int ext2_fill_super(struct super_block *sb, struct fs_context *fc)
 	if (EXT2_HAS_COMPAT_FEATURE(sb, EXT3_FEATURE_COMPAT_HAS_JOURNAL))
 		ext2_msg(sb, KERN_WARNING,
 			"warning: mounting ext3 filesystem as ext2");
-	if (ext2_setup_super (sb, es, sb_rdonly(sb)))
+	/* concurrent access is not possible before the mount completes */
+	if (context_unsafe(ext2_setup_super(sb, es, sb_rdonly(sb))))
 		sb->s_flags |= SB_RDONLY;
 	ext2_write_super(sb);
 	return 0;
-- 
2.39.5

[PATCH v1 6/9] ext2: annotate ext2_init_block_alloc_info() as requiring truncate_mutex

[Timothy Day]

ext2_init_block_alloc_info() sets up the inode's block reservation info
and is already documented as needing truncate_mutex protection. Express
that with __must_hold() for Clang's context analysis.

Both callers (ext2_get_blocks() and ext2_ioctl()) already hold
truncate_mutex.

Signed-off-by: Timothy Day <timday@thelustrecollective.com>
---
 fs/ext2/balloc.c | 1 +
 fs/ext2/ext2.h   | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
index adf0f31fbddd1..53c91cb38bdee 100644
--- a/fs/ext2/balloc.c
+++ b/fs/ext2/balloc.c
@@ -414,6 +414,7 @@ static inline int rsv_is_empty(struct ext2_reserve_window *rsv)
  * Needs truncate_mutex protection prior to calling this function.
  */
 void ext2_init_block_alloc_info(struct inode *inode)
+	__must_hold(&EXT2_I(inode)->truncate_mutex)
 {
 	struct ext2_inode_info *ei = EXT2_I(inode);
 	struct ext2_block_alloc_info *block_i;
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index 4c30d6f5c9726..b6ae29b2e6ec1 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -710,7 +710,8 @@ extern struct ext2_group_desc * ext2_get_group_desc(struct super_block * sb,
 						    struct buffer_head ** bh);
 extern void ext2_discard_reservation (struct inode *);
 extern int ext2_should_retry_alloc(struct super_block *sb, int *retries);
-extern void ext2_init_block_alloc_info(struct inode *);
+extern void ext2_init_block_alloc_info(struct inode *inode)
+	__must_hold(&EXT2_I(inode)->truncate_mutex);
 extern void ext2_rsv_window_add(struct super_block *sb, struct ext2_reserve_window_node *rsv);
 
 /* dir.c */
-- 
2.39.5

[PATCH v1 7/9] ext2: annotate block-mapping helpers as requiring truncate_mutex

[Timothy Day]

The indirect-block mapping helpers in inode.c all run under the
inode's truncate_mutex.

ext2_find_goal(), ext2_alloc_blocks(), ext2_alloc_branch() and
ext2_splice_branch() are reached only from ext2_get_blocks() while
it holds the mutex. ext2_find_shared(), ext2_free_data() and
ext2_free_branches() are called from __ext2_truncate_blocks() while
it holds the mutex.

Add __must_hold() annotations for the Clang's context analysis to reflect
this.

Signed-off-by: Timothy Day <timday@thelustrecollective.com>
---
 fs/ext2/inode.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 29808629cce56..f0be5236d04e0 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -329,6 +329,7 @@ static ext2_fsblk_t ext2_find_near(struct inode *inode, Indirect *ind)
 
 static inline ext2_fsblk_t ext2_find_goal(struct inode *inode, long block,
 					  Indirect *partial)
+	__must_hold(&EXT2_I(inode)->truncate_mutex)
 {
 	struct ext2_block_alloc_info *block_i;
 
@@ -399,6 +400,7 @@ ext2_blks_to_allocate(Indirect * branch, int k, unsigned long blks,
 static int ext2_alloc_blocks(struct inode *inode,
 			ext2_fsblk_t goal, int indirect_blks, int blks,
 			ext2_fsblk_t new_blocks[4], int *err)
+	__must_hold(&EXT2_I(inode)->truncate_mutex)
 {
 	int target, i;
 	unsigned long count = 0;
@@ -479,6 +481,7 @@ static int ext2_alloc_blocks(struct inode *inode,
 static int ext2_alloc_branch(struct inode *inode,
 			int indirect_blks, int *blks, ext2_fsblk_t goal,
 			int *offsets, Indirect *branch)
+	__must_hold(&EXT2_I(inode)->truncate_mutex)
 {
 	int blocksize = inode->i_sb->s_blocksize;
 	int i, n = 0;
@@ -560,6 +563,7 @@ static int ext2_alloc_branch(struct inode *inode,
  */
 static void ext2_splice_branch(struct inode *inode,
 			long block, Indirect *where, int num, int blks)
+	__must_hold(&EXT2_I(inode)->truncate_mutex)
 {
 	int i;
 	struct ext2_block_alloc_info *block_i;
@@ -1002,6 +1006,7 @@ static Indirect *ext2_find_shared(struct inode *inode,
 				int offsets[4],
 				Indirect chain[4],
 				__le32 *top)
+	__must_hold(&EXT2_I(inode)->truncate_mutex)
 {
 	Indirect *partial, *p;
 	int k, err;
@@ -1057,6 +1062,7 @@ static Indirect *ext2_find_shared(struct inode *inode,
  *	appropriately.
  */
 static inline void ext2_free_data(struct inode *inode, __le32 *p, __le32 *q)
+	__must_hold(&EXT2_I(inode)->truncate_mutex)
 {
 	ext2_fsblk_t block_to_free = 0, count = 0;
 	ext2_fsblk_t nr;
@@ -1097,6 +1103,7 @@ static inline void ext2_free_data(struct inode *inode, __le32 *p, __le32 *q)
  *	appropriately.
  */
 static void ext2_free_branches(struct inode *inode, __le32 *p, __le32 *q, int depth)
+	__must_hold(&EXT2_I(inode)->truncate_mutex)
 {
 	struct buffer_head * bh;
 	ext2_fsblk_t nr;
-- 
2.39.5

[PATCH v1 8/9] ext2: annotate s_rsv_window_root as requiring s_rsv_window_lock

[Timothy Day]

The per-filesystem reservation window rb-tree (s_rsv_window_root) is
protected by s_rsv_window_lock. Mark the s_rsv_window_root field
with __guarded_by() for Clang's context analysis.

The helpers (ext2_rsv_window_add, rsv_window_remove, and
find_next_reservable_window) that mutate or walk the tree are all
called with the s_rsv_window_lock held. Annotate these helpers
with __must_hold().

The accesses in ext2_fill_super() are before the superblock is live.
Since no concurrent access should be possible, s_rsv_window_lock
is not taken. Hence, wrap these accesses in context_unsafe().

Signed-off-by: Timothy Day <timday@thelustrecollective.com>
---
 fs/ext2/balloc.c | 3 +++
 fs/ext2/ext2.h   | 5 +++--
 fs/ext2/super.c  | 6 ++++--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/ext2/balloc.c b/fs/ext2/balloc.c
index 53c91cb38bdee..80acc1e193871 100644
--- a/fs/ext2/balloc.c
+++ b/fs/ext2/balloc.c
@@ -334,6 +334,7 @@ search_reserve_window(struct rb_root *root, ext2_fsblk_t goal)
  */
 void ext2_rsv_window_add(struct super_block *sb,
 		    struct ext2_reserve_window_node *rsv)
+	__must_hold(&EXT2_SB(sb)->s_rsv_window_lock)
 {
 	struct rb_root *root = &EXT2_SB(sb)->s_rsv_window_root;
 	struct rb_node *node = &rsv->rsv_node;
@@ -373,6 +374,7 @@ void ext2_rsv_window_add(struct super_block *sb,
  */
 static void rsv_window_remove(struct super_block *sb,
 			      struct ext2_reserve_window_node *rsv)
+	__must_hold(&EXT2_SB(sb)->s_rsv_window_lock)
 {
 	rsv->rsv_start = EXT2_RESERVE_WINDOW_NOT_ALLOCATED;
 	rsv->rsv_end = EXT2_RESERVE_WINDOW_NOT_ALLOCATED;
@@ -759,6 +761,7 @@ static int find_next_reservable_window(
 				struct super_block * sb,
 				ext2_fsblk_t start_block,
 				ext2_fsblk_t last_block)
+	__must_hold(&EXT2_SB(sb)->s_rsv_window_lock)
 {
 	struct rb_node *next;
 	struct ext2_reserve_window_node *rsv, *prev;
diff --git a/fs/ext2/ext2.h b/fs/ext2/ext2.h
index b6ae29b2e6ec1..404f4223a6bdc 100644
--- a/fs/ext2/ext2.h
+++ b/fs/ext2/ext2.h
@@ -102,7 +102,7 @@ struct ext2_sb_info {
 	struct blockgroup_lock *s_blockgroup_lock;
 	/* root of the per fs reservation window tree */
 	spinlock_t s_rsv_window_lock;
-	struct rb_root s_rsv_window_root;
+	struct rb_root s_rsv_window_root __guarded_by(&s_rsv_window_lock);
 	struct ext2_reserve_window_node s_rsv_window_head;
 	/*
 	 * s_lock protects against concurrent modifications of s_mount_state,
@@ -712,7 +712,8 @@ extern void ext2_discard_reservation (struct inode *);
 extern int ext2_should_retry_alloc(struct super_block *sb, int *retries);
 extern void ext2_init_block_alloc_info(struct inode *inode)
 	__must_hold(&EXT2_I(inode)->truncate_mutex);
-extern void ext2_rsv_window_add(struct super_block *sb, struct ext2_reserve_window_node *rsv);
+extern void ext2_rsv_window_add(struct super_block *sb, struct ext2_reserve_window_node *rsv)
+	__must_hold(&EXT2_SB(sb)->s_rsv_window_lock);
 
 /* dir.c */
 int ext2_add_link(struct dentry *, struct inode *);
diff --git a/fs/ext2/super.c b/fs/ext2/super.c
index a5a5285dd5784..693e59c1aba5a 100644
--- a/fs/ext2/super.c
+++ b/fs/ext2/super.c
@@ -1135,7 +1135,8 @@ static int ext2_fill_super(struct super_block *sb, struct fs_context *fc)
 
 	/* per filesystem reservation list head & lock */
 	spin_lock_init(&sbi->s_rsv_window_lock);
-	sbi->s_rsv_window_root = RB_ROOT;
+	/* concurrent access is not possible before the mount completes */
+	context_unsafe(sbi->s_rsv_window_root = RB_ROOT);
 	/*
 	 * Add a single, static dummy reservation to the start of the
 	 * reservation window list --- it gives us a placeholder for
@@ -1146,7 +1147,8 @@ static int ext2_fill_super(struct super_block *sb, struct fs_context *fc)
 	sbi->s_rsv_window_head.rsv_end = EXT2_RESERVE_WINDOW_NOT_ALLOCATED;
 	sbi->s_rsv_window_head.rsv_alloc_hit = 0;
 	sbi->s_rsv_window_head.rsv_goal_size = 0;
-	ext2_rsv_window_add(sb, &sbi->s_rsv_window_head);
+	/* concurrent access is not possible before the mount completes */
+	context_unsafe(ext2_rsv_window_add(sb, &sbi->s_rsv_window_head));
 
 	err = percpu_counter_init(&sbi->s_freeblocks_counter,
 				ext2_count_free_blocks(sb), GFP_KERNEL);
-- 
2.39.5

[PATCH v1 9/9] ext2: enabled context analysis support for ext2 filesystem

[Timothy Day]

Update ext2 Makefile to support context analysis [1].

[1] https://docs.kernel.org/dev-tools/context-analysis.html

Signed-off-by: Timothy Day <timday@thelustrecollective.com>
---
 fs/ext2/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ext2/Makefile b/fs/ext2/Makefile
index 8860948ef9ca4..33db2e9dc9082 100644
--- a/fs/ext2/Makefile
+++ b/fs/ext2/Makefile
@@ -3,6 +3,8 @@
 # Makefile for the linux ext2-filesystem routines.
 #
 
+CONTEXT_ANALYSIS := y
+
 obj-$(CONFIG_EXT2_FS) += ext2.o
 
 ext2-y := balloc.o dir.o file.o ialloc.o inode.o \
-- 
2.39.5

Re: [PATCH v1 1/9] ext2: fix ext2_xattr_delete_inode() context analysis warning

[Theodore Tso]

On Sun, Jul 12, 2026 at 12:56:02PM -0500, Timothy Day wrote:
> Clang reports the following warning when context analysis is enabled:
> 
> fs/ext2/xattr.c:829:6: error: rw_semaphore 'EXT2_I().xattr_sem' is not \
>   held on every path through here [-Werror,-Wthread-safety-analysis]
>   829 |         if (WARN_ON_ONCE(!down_write_trylock(&EXT2_I(inode)->xattr_sem)))
>       |             ^
> 
> When WARN_ON_ONCE() wraps down_write_trylock(), it adds an unlikely()
> annotation which hides the conditional acquire annotation from Clang's
> context analysis. This triggers a false positive. Pull the trylock out
> of the macro to silence this warning.

Does this mean that using Clang's context analysis means that we can't
use unlikely()?  That seems.... unfortunate.  Is there any other way
we can fix the false positive?  Even disabling unlikely() if and only
if Clang context analysis is enabled might be a better choice, since
we don't necessarily need to build with context analysis enabled when
building a production kernel.

				- Ted

Re: [PATCH v1 1/9] ext2: fix ext2_xattr_delete_inode() context analysis warning

[Timothy Day]

On Mon, 13 Jul 2026 10:32:56 -0400, Theodore Tso wrote:
> On Sun, Jul 12, 2026 at 12:56:02PM -0500, Timothy Day wrote:
> > Clang reports the following warning when context analysis is enabled:
> >
> > fs/ext2/xattr.c:829:6: error: rw_semaphore 'EXT2_I().xattr_sem' is not \
> >   held on every path through here [-Werror,-Wthread-safety-analysis]
> >   829 |         if (WARN_ON_ONCE(!down_write_trylock(&EXT2_I(inode)->xattr_sem)))
> >       |             ^
> >
> > When WARN_ON_ONCE() wraps down_write_trylock(), it adds an unlikely()
> > annotation which hides the conditional acquire annotation from Clang's
> > context analysis. This triggers a false positive. Pull the trylock out
> > of the macro to silence this warning.
>
> Does this mean that using Clang's context analysis means that we can't
> use unlikely()?  That seems.... unfortunate.  Is there any other way
> we can fix the false positive?

We can't wrap a function that acquires a context inside unlikely(). We
can use unlikely everywhere else. AFAICS, I think the trylock functions
are only problem right now. Grepping for ".*WARN.*trylock.*" and
".*unlikely.*trylock.*" shows a few other examples of code like this.
Not a huge amount, but not zero. I looked at a few other
series [1][2][3][4], but I haven't seen anyone do a similar conversion.

[1] (block) https://lore.kernel.org/all/91cb8c790fc8b26b8aa742569fbf8c2c1d099dac.1780682325.git.bvanassche@acm.org/
[2] (drdb) https://lore.kernel.org/all/77d0fd75811d7f604fa80b5c93172b5653b52880.1781042470.git.bvanassche@acm.org/
[3] (raid) https://lore.kernel.org/all/20260708090614.1431014-2-hch@lst.de/
[4] (nvme) https://lore.kernel.org/all/20260713115444.465704-19-nilay@linux.ibm.com/

> Even disabling unlikely() if and only
> if Clang context analysis is enabled might be a better choice, since
> we don't necessarily need to build with context analysis enabled when
> building a production kernel.

I don't think we'd want to disable unlikely() globally when context
analysis is enabled. Only for cases where it'd conflict with the
context analysis annotation. Long term, I think we'd want context
analysis enabled by default for Clang builds.

I think we have a couple options:

a) Current fix - it's okay, but a bit ugly
b) Teach LLVM/Clang to handle this better - no idea how practical this is
c) Rework the code to not take the lock at all. The comment "We could
   as well just not acquire xattr_sem at all" seems to imply this is fine.
   Although I haven't tested it very much:

diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c
index e55d16abf422f..d2aebf4b2ced9 100644
--- a/fs/ext2/xattr.c
+++ b/fs/ext2/xattr.c
@@ -815,6 +815,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
  */
 void
 ext2_xattr_delete_inode(struct inode *inode)
+       __must_not_hold(&EXT2_I(inode)->xattr_sem)
 {
        struct buffer_head *bh = NULL;
        struct ext2_sb_info *sbi = EXT2_SB(inode->i_sb);
@@ -826,8 +827,7 @@ ext2_xattr_delete_inode(struct inode *inode)
         * here to avoid false-positive warning from lockdep about reclaim
         * circular dependency.
         */
-       if (WARN_ON_ONCE(!down_write_trylock(&EXT2_I(inode)->xattr_sem)))
-               return;
+       lockdep_assert_not_held(&EXT2_I(inode)->xattr_sem);
        if (!EXT2_I(inode)->i_file_acl)
                goto cleanup;
 
@@ -857,7 +857,6 @@ ext2_xattr_delete_inode(struct inode *inode)
 
 cleanup:
        brelse(bh);
-       up_write(&EXT2_I(inode)->xattr_sem);
 }

d) Use some macro magic in unlikely() to skip the annotation if
   wrapping a context-acquiring function.

I think a) or c) is most practical. Open to other ideas.

- Tim Day

Re: [PATCH v1 1/9] ext2: fix ext2_xattr_delete_inode() context analysis warning

[Marco Elver]

On Mon, 13 Jul 2026 at 18:44, Timothy Day
<timday@thelustrecollective.com> wrote:
>
> On Mon, 13 Jul 2026 10:32:56 -0400, Theodore Tso wrote:
> > On Sun, Jul 12, 2026 at 12:56:02PM -0500, Timothy Day wrote:
> > > Clang reports the following warning when context analysis is enabled:
> > >
> > > fs/ext2/xattr.c:829:6: error: rw_semaphore 'EXT2_I().xattr_sem' is not \
> > >   held on every path through here [-Werror,-Wthread-safety-analysis]
> > >   829 |         if (WARN_ON_ONCE(!down_write_trylock(&EXT2_I(inode)->xattr_sem)))
> > >       |             ^
> > >
> > > When WARN_ON_ONCE() wraps down_write_trylock(), it adds an unlikely()
> > > annotation which hides the conditional acquire annotation from Clang's
> > > context analysis. This triggers a false positive. Pull the trylock out
> > > of the macro to silence this warning.
> >
> > Does this mean that using Clang's context analysis means that we can't
> > use unlikely()?  That seems.... unfortunate.  Is there any other way
> > we can fix the false positive?
>
> We can't wrap a function that acquires a context inside unlikely(). We
> can use unlikely everywhere else. AFAICS, I think the trylock functions
> are only problem right now. Grepping for ".*WARN.*trylock.*" and
> ".*unlikely.*trylock.*" shows a few other examples of code like this.
> Not a huge amount, but not zero. I looked at a few other
> series [1][2][3][4], but I haven't seen anyone do a similar conversion.
>
> [1] (block) https://lore.kernel.org/all/91cb8c790fc8b26b8aa742569fbf8c2c1d099dac.1780682325.git.bvanassche@acm.org/
> [2] (drdb) https://lore.kernel.org/all/77d0fd75811d7f604fa80b5c93172b5653b52880.1781042470.git.bvanassche@acm.org/
> [3] (raid) https://lore.kernel.org/all/20260708090614.1431014-2-hch@lst.de/
> [4] (nvme) https://lore.kernel.org/all/20260713115444.465704-19-nilay@linux.ibm.com/
>
> > Even disabling unlikely() if and only
> > if Clang context analysis is enabled might be a better choice, since
> > we don't necessarily need to build with context analysis enabled when
> > building a production kernel.
>
> I don't think we'd want to disable unlikely() globally when context
> analysis is enabled. Only for cases where it'd conflict with the
> context analysis annotation. Long term, I think we'd want context
> analysis enabled by default for Clang builds.
>
> I think we have a couple options:
>
> a) Current fix - it's okay, but a bit ugly
> b) Teach LLVM/Clang to handle this better - no idea how practical this is

I'm not quite sure what's going on here yet, but
__builtin_expect-wrapped trylock function calls are supported by
Context Analysis (Clang's Thread Safety Analysis). Some of the
additional layers that WARN_ON_ONCE adds is likely throwing it off.
I'll take a look, but might be a week or so. We are already bumping
min Clang version for Context Analysis to Clang 23, so I'm hoping this
can be fixed in Clang 23 (which is not yet released).

Re: [PATCH v1 1/9] ext2: fix ext2_xattr_delete_inode() context analysis warning

[Marco Elver]

On Tue, 14 Jul 2026 at 00:27, Marco Elver <elver@google.com> wrote:
>
> On Mon, 13 Jul 2026 at 18:44, Timothy Day
> <timday@thelustrecollective.com> wrote:
> >
> > On Mon, 13 Jul 2026 10:32:56 -0400, Theodore Tso wrote:
> > > On Sun, Jul 12, 2026 at 12:56:02PM -0500, Timothy Day wrote:
> > > > Clang reports the following warning when context analysis is enabled:
> > > >
> > > > fs/ext2/xattr.c:829:6: error: rw_semaphore 'EXT2_I().xattr_sem' is not \
> > > >   held on every path through here [-Werror,-Wthread-safety-analysis]
> > > >   829 |         if (WARN_ON_ONCE(!down_write_trylock(&EXT2_I(inode)->xattr_sem)))
> > > >       |             ^
> > > >
> > > > When WARN_ON_ONCE() wraps down_write_trylock(), it adds an unlikely()
> > > > annotation which hides the conditional acquire annotation from Clang's
> > > > context analysis. This triggers a false positive. Pull the trylock out
> > > > of the macro to silence this warning.
> > >
> > > Does this mean that using Clang's context analysis means that we can't
> > > use unlikely()?  That seems.... unfortunate.  Is there any other way
> > > we can fix the false positive?
> >
> > We can't wrap a function that acquires a context inside unlikely(). We
> > can use unlikely everywhere else. AFAICS, I think the trylock functions
> > are only problem right now. Grepping for ".*WARN.*trylock.*" and
> > ".*unlikely.*trylock.*" shows a few other examples of code like this.
> > Not a huge amount, but not zero. I looked at a few other
> > series [1][2][3][4], but I haven't seen anyone do a similar conversion.
> >
> > [1] (block) https://lore.kernel.org/all/91cb8c790fc8b26b8aa742569fbf8c2c1d099dac.1780682325.git.bvanassche@acm.org/
> > [2] (drdb) https://lore.kernel.org/all/77d0fd75811d7f604fa80b5c93172b5653b52880.1781042470.git.bvanassche@acm.org/
> > [3] (raid) https://lore.kernel.org/all/20260708090614.1431014-2-hch@lst.de/
> > [4] (nvme) https://lore.kernel.org/all/20260713115444.465704-19-nilay@linux.ibm.com/
> >
> > > Even disabling unlikely() if and only
> > > if Clang context analysis is enabled might be a better choice, since
> > > we don't necessarily need to build with context analysis enabled when
> > > building a production kernel.
> >
> > I don't think we'd want to disable unlikely() globally when context
> > analysis is enabled. Only for cases where it'd conflict with the
> > context analysis annotation. Long term, I think we'd want context
> > analysis enabled by default for Clang builds.
> >
> > I think we have a couple options:
> >
> > a) Current fix - it's okay, but a bit ugly
> > b) Teach LLVM/Clang to handle this better - no idea how practical this is
>
> I'm not quite sure what's going on here yet, but
> __builtin_expect-wrapped trylock function calls are supported by
> Context Analysis (Clang's Thread Safety Analysis). Some of the
> additional layers that WARN_ON_ONCE adds is likely throwing it off.
> I'll take a look, but might be a week or so. We are already bumping
> min Clang version for Context Analysis to Clang 23, so I'm hoping this
> can be fixed in Clang 23 (which is not yet released).

Relatively certain statement expressions were once again the problem -
this should fix it: https://github.com/llvm/llvm-project/pull/209330

I haven't had time to test the Clang fix on  fs/ext2/xattr.c with
context analysis enabled  (please do so if you already build Clang
yourself, otherwise I might get to it in ~week).

Re: [PATCH v1 1/9] ext2: fix ext2_xattr_delete_inode() context analysis warning

[Theodore Tso]

On Mon, Jul 13, 2026 at 12:44:17PM -0500, Timothy Day wrote:
> I don't think we'd want to disable unlikely() globally when context
> analysis is enabled. Only for cases where it'd conflict with the
> context analysis annotation. Long term, I think we'd want context
> analysis enabled by default for Clang builds.

I see that Marco has a potential change to make LLVM/Clang to handle
this better, which is great, but I'm not sure why disabling unlikely()
globally when context analysis is enabled would be that terrible.
Context analysis wouldn't be changed with or without unlikely(), and
if we use context analysis as a debugging build pass, we wouldn't care
about performance, right?  What am I missing?

      		   	   	     - Ted

Re: [PATCH v1 1/9] ext2: fix ext2_xattr_delete_inode() context analysis warning

[Timothy Day]

On Mon, 13 Jul 2026 22:26:02 -0400, Theodore Tso wrote:
> On Mon, Jul 13, 2026 at 12:44:17PM -0500, Timothy Day wrote:
> > I don't think we'd want to disable unlikely() globally when context
> > analysis is enabled. Only for cases where it'd conflict with the
> > context analysis annotation. Long term, I think we'd want context
> > analysis enabled by default for Clang builds.
>
> I see that Marco has a potential change to make LLVM/Clang to handle
> this better, which is great, but I'm not sure why disabling unlikely()
> globally when context analysis is enabled would be that terrible.
> Context analysis wouldn't be changed with or without unlikely(), and
> if we use context analysis as a debugging build pass, we wouldn't care
> about performance, right?  What am I missing?

It's mostly convenience, I think. If the context analysis could be
made to work without any runtime penalty, then you don't need a
separate debug build. If you're doing a bunch of kernel builds
anyway, that's not very compelling. But if it were automatically
enabled whenever Clang is being used, developers might catch these
issues faster. They might not think to enable these warnings
otherwise. And if distros ever used Clang for kernel builds more
widely, they could enable this feature by default. Then
out-of-tree modules could benefit from the feature without having
to make a custom kernel.

Tim Day

Re: [PATCH v1 1/9] ext2: fix ext2_xattr_delete_inode() context analysis warning

[Theodore Tso]

On Tue, Jul 14, 2026 at 12:17:24AM -0500, Timothy Day wrote:
> It's mostly convenience, I think. If the context analysis could be
> made to work without any runtime penalty, then you don't need a
> separate debug build.

That's a fair.  I don't mind doing two back-to-back compiles passes,
one with and one without context analysis enabled, but that's because
I have a fairly high end development machine.  I can see tht people
might avoid using context analysis if it slowed down their build times
too much, or if you had to explicitly enable it.  As a maintainer who
is used to periodically running lockdep and KASAN builds against
dozens of hours of VM time worth of regression testing, sharded across
many different VM's, an extra compile-only build is in the noise.  But
I'm probably an outlier.  :-)

					- Ted

Re: [PATCH v1 0/9] Support Clang context analysis for ext2

[Jan Kara]

On Sun 12-07-26 12:56:01, Timothy Day wrote:
> 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-rc2 (since the
> minimum Clang version was recently bumped to 23 in 7.2+ [3]).
> 
> I'd appreciate reviews and suggestions. I'm especially interested in
> suggestions on how we can make this easier to enable for other
> filesystems.
> 
> Thanks!

Overall this looks sane to me and I kind of like the documentation value of
__guarded_by and __must_hold annotations. The context_unsafe annotations
are a bit annoying but I can live with that for now. Just one question: do
I understand right that the compiler verifies whether the lock is held when
entering the function marked with __must_hold? So if we get the annotation
wrong, the compiler will tell us?

								Honza

> 
> [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://lore.kernel.org/all/20260515124426.2227783-1-elver@google.com/
> 
> Timothy Day (9):
>   ext2: fix ext2_xattr_delete_inode() context analysis warning
>   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: enabled 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  | 18 +++++++++++++-----
>  fs/ext2/xattr.c  |  4 +++-
>  6 files changed, 40 insertions(+), 14 deletions(-)
> 
> -- 
> 2.39.5
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR