All of lore.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: 18+ 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-28 19:51   ` [tip: locking/core] " tip-bot2 for Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 2/6] compiler-context-analysis: Introduce scoped init guards Marco Elver
2026-01-28 19:51   ` [tip: locking/core] " tip-bot2 for Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 3/6] kcov: Use scoped init guard Marco Elver
2026-01-28 19:51   ` [tip: locking/core] " tip-bot2 for Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 4/6] crypto: " Marco Elver
2026-01-28 19:51   ` [tip: locking/core] " tip-bot2 for Marco Elver
2026-01-19  9:05 ` [PATCH tip/locking/core 5/6] tomoyo: " Marco Elver
2026-01-28 19:51   ` [tip: locking/core] " tip-bot2 for 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-28 19:51   ` [tip: locking/core] " tip-bot2 for 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.