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

[Marco Elver]

On Tue, 14 Jul 2026 at 23:14, Timothy Day
<timday@xxxxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue, 14 Jul 2026 00:56:56 +0200, Marco Elver wrote:
> > 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).
>
> The fix wasn't enough. The commit message on patch 1/9 omitted another
> warning. On vanilla Clang 23, I see:
>
> linux/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)))
>       |             ^
>
> and:
>
> linux/fs/ext2/xattr.c:860:2: error: releasing rw_semaphore \
>   'EXT2_I().xattr_sem' that was not held [-Werror,-Wthread-safety-analysis]
>   860 |         up_write(&EXT2_I(inode)->xattr_sem);
>       |         ^
>
> If I apply your patch, it resolves the second warning but not the first.
> I tried to generate a fix [1]. That branch has both your patch and my
> fix applied. Those two changes combined fix both warnings. My fix
> tries to add some special casing for tracking contexts acquired by
> trylocks. It works, but I'm not very confident on the correctness.

Nice, thanks. I'll have a look, but it appears right. Need to get my
PR merged first (need approval), then please feel free to send yours
with me as reviewer/approver.

> About next steps, it seems like everyone is happy with the rest of the
> series. But the first patch should be dropped in favor of a fix to Clang.
>
> Should I submit a v2 with 1/9 omitted?

I'd wait for the Clang fixes to land. They look relatively
straightforward, but just to be sure...

> When would we be safe to merge
> the series? Once the fix is merged to Clang? Is there some kind of
> periodic LLVM release we ought to wait for?

https://llvm.org has release schedule. While a release for 23 is not
yet cut, and we do require the unreleased Clang 23, we can respin the
series after ~1 week of the Clang fixes being merged. Only some CI
systems build Clang at head, but it's generally assumed that given
it's the pre-release, they will catch up with head with some days of
delay.

Thanks,
-- Marco