Re: [PATCH v2] locking/rtmutex: Annotate API and implementation

[Sebastian Andrzej Siewior]

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