Messages in this thread |  | | Date | Wed, 6 May 2026 09:35:41 +0200 | | From | Sebastian Andrzej Siewior <> | | Subject | Re: [PATCH v2] locking/rtmutex: Annotate API and implementation |
| |
On 2026-05-05 22:05:51 [+0200], Bart Van Assche wrote: > On 5/5/26 6:12 PM, Sebastian Andrzej Siewior wrote: > > On 2026-05-05 04:26:44 [+0200], Bart Van Assche wrote: > > > +context_lock_struct(rt_mutex); > > > > What does this do? Shouldn't this define the struct? > > This enables context locking support for struct rt_mutex. I placed > context_lock_struct() on a line by itself because in my opinion that > results in a header file that is easier to read compared to > context_lock_struct(name) { ... }.
Hmm. This was the confusing part, because everything else such as rwlock, rw_semaphore, mutex, … use that way.
> > > --- a/kernel/locking/rtmutex_api.c > > > +++ b/kernel/locking/rtmutex_api.c > > > @@ -66,12 +67,14 @@ EXPORT_SYMBOL(rt_mutex_base_init); > > > * @subclass: the lockdep subclass > > > */ > > > void __sched rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int subclass) > > > + __no_context_analysis /* ignoring the return value below is fine in this case */ > > > { > > > __rt_mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, NULL, subclass); > > > } > > > EXPORT_SYMBOL_GPL(rt_mutex_lock_nested); > > > void __sched _rt_mutex_lock_nest_lock(struct rt_mutex *lock, struct lockdep_map *nest_lock) > > > + __no_context_analysis /* ignoring the return value below is fine in this case */ > > > { > > > __rt_mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, nest_lock, 0); > > > } > > > > *Why* is it okay? Because the void always acquire the lock and only the > > conditional locking (which can be interrupted by signal/ timeout) return > > an error if they failed to acquire the lock. > > Yes, that's correct.
I meant this as an improved comment ;)
> > Something like that would be nice for the comment. > > > > Not sure if "__no_context_analysis" is the right thing to do here. > > __acquires(lock) __no_context_analysis > > > > might be better if just __acquires leads to trouble. > > There is an alternative that does not require __no_context_analysis: > > void __sched rt_mutex_lock_nested(struct rt_mutex *lock, unsigned int > subclass) > { > int ret = __rt_mutex_lock_common(lock, TASK_UNINTERRUPTIBLE, NULL, > subclass); > > BUG_ON(ret); > } > EXPORT_SYMBOL_GPL(rt_mutex_lock_nested); > > Please let me know which style you prefer.
Hmm. This mostly reassembles __mutex_lock() from mutex.c which does the same thing. Couldn't we get away doing the same thing meaning a __cond_acquires() on those with a return value and a __acquire() in the void case? I think it would make sense to keep those two close in terms of annotations.
> Thanks, > > Bart.
Sebastian
|  |