Linux filesystem development
 help / color / mirror / Atom feed
From: Timothy Day <timday@thelustrecollective.com>
To: Jan Kara <jack@suse.cz>
Cc: Timothy Day <timday@thelustrecollective.com>,
	Marco Elver <elver@google.com>,
	Nathan Chancellor <nathan@kernel.org>,
	linux-ext4@vger.kernel.org, Jan Kara <jack@suse.com>,
	Theodore Tso <tytso@mit.edu>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 0/8] Support Clang context analysis for ext2
Date: Fri, 11 Sep 2026 16:01:51 -0400	[thread overview]
Message-ID: <20260911200152.1588568-1-timday@thelustrecollective.com> (raw)
In-Reply-To: <46mgpyanbjl7eiln3nodqtpbubrwsc5blcjqd6r5pfokmssrce@k5qgaws7b2l7>

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

      reply	other threads:[~2026-09-11 20:02 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-11 16:03 [PATCH v2 0/8] Support Clang context analysis for ext2 Timothy Day
2026-08-11 16:03 ` [PATCH v2 1/8] ext2: mark s_next_generation as guarded by s_next_gen_lock Timothy Day
2026-08-11 16:03 ` [PATCH v2 2/8] ext2: annotate ext2_update_dynamic_rev() as requiring s_lock Timothy Day
2026-08-11 16:03 ` [PATCH v2 3/8] ext2: mark statfs overhead cache as guarded by s_lock Timothy Day
2026-08-11 16:03 ` [PATCH v2 4/8] ext2: mark s_mount_state " Timothy Day
2026-08-11 16:03 ` [PATCH v2 5/8] ext2: annotate ext2_init_block_alloc_info() as requiring truncate_mutex Timothy Day
2026-08-11 16:03 ` [PATCH v2 6/8] ext2: annotate block-mapping helpers " Timothy Day
2026-08-11 16:03 ` [PATCH v2 7/8] ext2: annotate s_rsv_window_root as requiring s_rsv_window_lock Timothy Day
2026-08-11 16:03 ` [PATCH v2 8/8] ext2: enable context analysis support for ext2 filesystem Timothy Day
2026-08-12 11:14 ` [PATCH v2 0/8] Support Clang context analysis for ext2 Marco Elver
2026-08-18  9:58 ` Jan Kara
2026-09-03  7:27   ` Nathan Chancellor
2026-09-03 10:34     ` Marco Elver
2026-09-03 11:06       ` Jan Kara
2026-09-03 12:06         ` Marco Elver
2026-09-03 12:43           ` Jan Kara
2026-09-11 20:01             ` Timothy Day [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260911200152.1588568-1-timday@thelustrecollective.com \
    --to=timday@thelustrecollective.com \
    --cc=elver@google.com \
    --cc=jack@suse.com \
    --cc=jack@suse.cz \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox