[PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards

Marco Elver posted 6 patches 2 weeks, 5 days ago
Documentation/dev-tools/context-analysis.rst | 30 ++++++++++++++++++--
crypto/crypto_engine.c                       |  2 +-
crypto/drbg.c                                |  2 +-
include/linux/cleanup.h                      |  8 +++---
include/linux/compiler-context-analysis.h    |  9 ++----
include/linux/local_lock.h                   |  8 ++++++
include/linux/local_lock_internal.h          |  4 +--
include/linux/mutex.h                        |  4 ++-
include/linux/rwlock.h                       |  3 +-
include/linux/rwlock_rt.h                    |  1 -
include/linux/rwsem.h                        |  6 ++--
include/linux/seqlock.h                      |  6 +++-
include/linux/spinlock.h                     | 17 ++++++++---
include/linux/spinlock_rt.h                  |  1 -
include/linux/ww_mutex.h                     |  1 -
kernel/kcov.c                                |  2 +-
lib/test_context-analysis.c                  | 22 ++++++--------
security/tomoyo/common.c                     |  2 +-
18 files changed, 80 insertions(+), 48 deletions(-)
[PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards
Posted by Marco Elver 2 weeks, 5 days ago
Current context analysis treats lock_init() as implicitly "holding" the
lock to allow initializing guarded members. This causes false-positive
"double lock" reports if the lock is acquired immediately after
initialization in the same scope; for example:

	mutex_init(&d->mtx);
	/* ... counter is guarded by mtx ... */
	d->counter = 0;  /* ok, but mtx is now "held" */
	...
	mutex_lock(&d->mtx); /* warning: acquiring mutex already held */

This series proposes a solution to this by introducing scoped init
guards which Peter suggested, using the guard(type_init)(&lock) or
scoped_guard(type_init, ..) interface. This explicitly marks init scope
where we can initialize guarded members. With that we can revert the
"implicitly hold" after init annotations, which allows use after
initialization scope as follows:

	scoped_guard(mutex_init, &d->mtx) {
		d->counter = 0;
	}
	...
	mutex_lock(&d->mtx); /* ok */

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):

	mutex_init(&d->mtx);
	context_unsafe(d->counter = 0);  /* ok */
	...
	mutex_lock(&d->mtx);

This series is an alternative to the approach in [1]:

   * Scoped init guards (this series): Sound interface, requires use of
     guard(type_init)(&lock) or scoped_guard(type_init, ..) for guarded
     member initialization.

   * Reentrant init [1]: Less intrusive, type_init() just works, and
     also allows guarded member initialization with later lock use in
     the same function. But unsound, and e.g. misses double-lock bugs
     immediately after init, trading false positives for false negatives.

[1] https://lore.kernel.org/all/20260115005231.1211866-1-elver@google.com/

Marco Elver (6):
  cleanup: Make __DEFINE_LOCK_GUARD handle commas in initializers
  compiler-context-analysis: Introduce scoped init guards
  kcov: Use scoped init guard
  crypto: Use scoped init guard
  tomoyo: Use scoped init guard
  compiler-context-analysis: Remove __assume_ctx_lock from initializers

 Documentation/dev-tools/context-analysis.rst | 30 ++++++++++++++++++--
 crypto/crypto_engine.c                       |  2 +-
 crypto/drbg.c                                |  2 +-
 include/linux/cleanup.h                      |  8 +++---
 include/linux/compiler-context-analysis.h    |  9 ++----
 include/linux/local_lock.h                   |  8 ++++++
 include/linux/local_lock_internal.h          |  4 +--
 include/linux/mutex.h                        |  4 ++-
 include/linux/rwlock.h                       |  3 +-
 include/linux/rwlock_rt.h                    |  1 -
 include/linux/rwsem.h                        |  6 ++--
 include/linux/seqlock.h                      |  6 +++-
 include/linux/spinlock.h                     | 17 ++++++++---
 include/linux/spinlock_rt.h                  |  1 -
 include/linux/ww_mutex.h                     |  1 -
 kernel/kcov.c                                |  2 +-
 lib/test_context-analysis.c                  | 22 ++++++--------
 security/tomoyo/common.c                     |  2 +-
 18 files changed, 80 insertions(+), 48 deletions(-)

-- 
2.52.0.457.g6b5491de43-goog
Re: [PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards
Posted by Christoph Hellwig 2 weeks, 4 days ago
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.
Re: [PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards
Posted by Peter Zijlstra 2 weeks, 4 days ago
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.
Re: [PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards
Posted by Christoph Hellwig 2 weeks, 2 days ago
On Tue, Jan 20, 2026 at 11:52:11AM +0100, Peter Zijlstra wrote:
> > 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;
> 	...
> }

And this is just as bad as the original version, except it is now
even more obfuscated.

> 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);
> 	...

That's better.  What would be even better for everyone would be:

	mutex_prepare(&obj->mutex); /* acquire, but with a nice name */
	obj->data = FOO;
	mutex_init_prepared(&obj->mutex); /* release, barrier, actual init */

	mutex_lock(&obj->mutex); /* IFF needed only */
Re: [PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards
Posted by Peter Zijlstra 2 weeks, 1 day ago
On Thu, Jan 22, 2026 at 07:30:42AM +0100, Christoph Hellwig wrote:

> That's better.  What would be even better for everyone would be:
> 
> 	mutex_prepare(&obj->mutex); /* acquire, but with a nice name */
> 	obj->data = FOO;
> 	mutex_init_prepared(&obj->mutex); /* release, barrier, actual init */
> 
> 	mutex_lock(&obj->mutex); /* IFF needed only */
> 

This is cannot work. There is no such thing is a release-barrier.
Furthermore, store-release, load-acquire needs an address dependency to
work.

When publishing an object, which is what we're talking about, we have
two common patterns:

 1) a locked data-structure

 2) RCU


The way 1) works is:

	Publish				Use

	lock(&structure_lock);
	insert(&structure, obj);
	unlock(&structure_lock);

					lock(&structure_lock)
					obj = find(&structure, key);
					...
					unlock(&structure_lock);

And here the Publish-unlock is a release which pairs with the Use-lock's
acquire and guarantees that Use sees both 'structure' in a coherent
state and obj as it was at the time of insertion. IOW we have
release-acquire through the &structure_lock pointer.

The way 2) works is:

	Publish				Use

	lock(&structure_lock);
	insert(&structure, obj)
	   rcu_assign_pointer(ptr, obj);
	unlock(&structure_lock);
	  	
					rcu_read_lock();
					obj = find_rcu(&structure, key);
					...
					rcu_read_unlock();


And here rcu_assign_pointer() is a store-release that pairs with an
rcu_dereference() inside find_rcu() on the same pointer.

There is no alternative way to order things, there must be a
release-acquire through a common address.

In both cases it is imperative the obj is fully (or full enough)
initialized before publication, because the consumer is only guaranteed
to see the state of the object it was in at publish time.
Re: [PATCH tip/locking/core 0/6] compiler-context-analysis: Scoped init guards
Posted by Bart Van Assche 2 weeks, 4 days ago
On 1/19/26 1:05 AM, Marco Elver wrote:
> This series proposes a solution to this by introducing scoped init
> guards which Peter suggested, using the guard(type_init)(&lock) or
> scoped_guard(type_init, ..) interface.
Although I haven't had the time yet to do an in-depth review, from a
quick look all patches in this series look good to me.

Thanks,

Bart.