Re: [PATCH v2 01/34] compiler_types: Move lock checking attributes to compiler-capability-analysis.h

Dan Carpenter
2025-03-05T09:27:10Z

On Wed, Mar 05, 2025 at 10:13:44AM +0100, Marco Elver wrote:
> On Wed, Mar 05, 2025 at 11:36AM +0300, Dan Carpenter wrote:
> > On Tue, Mar 04, 2025 at 10:21:00AM +0100, Marco Elver wrote:
> > > +#ifndef _LINUX_COMPILER_CAPABILITY_ANALYSIS_H
> > > +#define _LINUX_COMPILER_CAPABILITY_ANALYSIS_H
> > > +
> > > +#ifdef __CHECKER__
> > > +
> > > +/* Sparse context/lock checking support. */
> > > +# define __must_hold(x) __attribute__((context(x,1,1)))
> > > +# define __acquires(x) __attribute__((context(x,0,1)))
> > > +# define __cond_acquires(x) __attribute__((context(x,0,-1)))
> > > +# define __releases(x) __attribute__((context(x,1,0)))
> > > +# define __acquire(x) __context__(x,1)
> > > +# define __release(x) __context__(x,-1)
> > > +# define __cond_lock(x, c) ((c) ? ({ __acquire(x); 1; }) : 0)
> > > +
> >
> > The other thing you might want to annotate is ww_mutex_destroy().
>
> We can add an annotation to check the lock is not held:
>

Sorry, my email was bad.

I haven't actually tried your patch at all. I have locking check in
Smatch so I'm just basing this on the things that I did...
https://github.com/error27/smatch/blob/master/smatch_locking.c
This isn't a mandatory thing. Whatever happens we're going to end up
doing dozens of patches all over the kernel later.

I thought you could destroy a mutex regardless or whether it was held
or not. I was getting false positives which said that we should drop
the lock on error but actually the mutex is destroyed on that path so it
doesn't matter.

regards,
dan carpenter