public inbox for linux-security-module@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Marco Elver <elver@google.com>, Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Will Deacon <will@kernel.org>, Boqun Feng <boqun.feng@gmail.com>,
	Waiman Long <longman@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Bart Van Assche <bvanassche@acm.org>,
	kasan-dev@googlegroups.com, llvm@lists.linux.dev,
	linux-crypto@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards
Date: Tue, 20 Jan 2026 11:52:11 +0100	[thread overview]
Message-ID: <20260120105211.GW830755@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20260120072401.GA5905@lst.de>

On Tue, Jan 20, 2026 at 08:24:01AM +0100, Christoph Hellwig wrote:
> On Mon, Jan 19, 2026 at 10:05:50AM +0100, Marco Elver wrote:
> > Note: Scoped guarded initialization remains optional, and normal
> > initialization can still be used if no guarded members are being
> > initialized. Another alternative is to just disable context analysis to
> > initialize guarded members with `context_unsafe(var = init)` or adding
> > the `__context_unsafe(init)` function attribute (the latter not being
> > recommended for non-trivial functions due to lack of any checking):
> 
> I still think this is doing the wrong for the regular non-scoped
> cased, and I think I finally understand what is so wrong about it.
> 
> The fact that mutex_init (let's use mutexes for the example, applied
> to other primitives as well) should not automatically imply guarding
> the members for the rest of the function.  Because as soon as the
> structure that contains the lock is published that is not actually
> true, and we did have quite a lot of bugs because of that in the
> past.
> 
> So I think the first step is to avoid implying the safety of guarded
> member access by initialing the lock.  We then need to think how to
> express they are save, which would probably require explicit annotation
> unless we can come up with a scheme that makes these accesses fine
> before the mutex_init in a magic way.

But that is exactly what these patches do!

Note that the current state of things (tip/locking/core,next) is that
mutex_init() is 'special'. And I agree with you that that is quite
horrible.

Now, these patches, specifically patch 6, removes this implied
horribleness.

The alternative is an explicit annotation -- as you suggest.


So given something like:

struct my_obj {
	struct mutex	mutex;
	int		data __guarded_by(&mutex);
	...
};


tip/locking/core,next:

init_my_obj(struct my_obj *obj)
{
	mutex_init(&obj->mutex); // implies obj->mutex is taken until end of function
	obj->data = FOO;	 // OK, because &obj->mutex 'held'
	...
}

And per these patches that will no longer be true. So if you apply just
patch 6, which removes this implied behaviour, you get a compile fail.
Not good!

So patches 1-5 introduces alternatives.

So your preferred solution:

hch_my_obj(struct my_obj *obj)
{
	mutex_init(&obj->mutex);
	mutex_lock(&obj->mutex); // actually acquires lock
	obj->data = FOO;
	...
}

is perfectly fine and will work. But not everybody wants this. For the
people that only need to init the data fields and don't care about the
lock state we get:

init_my_obj(struct my_obj *obj)
{
	guard(mutex_init)(&obj->mutex); // initializes mutex and considers lock
					// held until end of function
	obj->data = FOO;
	...
}

For the people that want to first init the object but then actually lock
it, we get:

use_my_obj(struct my_obj *obj)
{
	scoped_guard (mutex_init, &obj->mutex) { // init mutex and 'hold' for scope
		obj->data = FOO;
		...
	}

	mutex_lock(&obj->lock);
	...
}

And for the people that *reaaaaaly* hate guards, it is possible to write
something like:

ugly_my_obj(struct my_obj *obj)
{
	mutex_init(&obj->mutex);
	__acquire_ctx_lock(&obj->mutex);
	obj->data = FOO;
	...
	__release_ctx_lock(&obj->mutex);

	mutex_lock(&obj->lock);
	...
}

And, then there is the option that C++ has:

init_my_obj(struct my_obj *obj)
	__no_context_analysis // STFU!
{
	mutex_init(&obj->mutex);
	obj->data = FOO;	 // WARN; but ignored
	...
}

All I can make from your email is that you must be in favour of these
patches.

  reply	other threads:[~2026-01-20 10:52 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-19  9:05 [PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 1/6] cleanup: Make __DEFINE_LOCK_GUARD handle commas in initializers Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 2/6] compiler-context-analysis: Introduce scoped init guards Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 3/6] kcov: Use scoped init guard Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 4/6] crypto: " Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 5/6] tomoyo: " Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 6/6] compiler-context-analysis: Remove __assume_ctx_lock from initializers Marco Elver
2026-01-20  7:24 ` [PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards Christoph Hellwig
2026-01-20 10:52   ` Peter Zijlstra [this message]
2026-01-22  6:30     ` Christoph Hellwig
2026-01-23  8:44       ` Peter Zijlstra
2026-01-20 18:24 ` Bart Van Assche

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=20260120105211.GW830755@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=boqun.feng@gmail.com \
    --cc=bvanassche@acm.org \
    --cc=elver@google.com \
    --cc=hch@lst.de \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=longman@redhat.com \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    /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