Re: [PATCH] sparse fix: add many lock annotations

[Alexey Dobriyan]

On Thu, Nov 23, 2006 at 12:18:42AM -0800, Ira Snyder wrote:
> On Wed, 22 Nov 2006 21:33:07 +0300
> Alexey Dobriyan <adobriyan@xxxxxxxxx> wrote:
> > On Wed, Nov 22, 2006 at 12:11:46AM -0800, Ira Snyder wrote:
> > > This patch adds many lock annotations to the kernel source to quiet
> > > warnings from sparse. In almost every case, it quiets the warning caused
> > > by locks that are intentionally grabbed in one function and released in
> > > another.
> > >
> > > In the other cases, __acquire() and __release() are used to make sparse
> > > believe that a lock was grabbed (even though it was not), in order to
> > > make all exit points have equal lock counts. These follow the style in
> > > kernel/sched.c.
> >
> > > --- a/arch/i386/kernel/smp.c
> > > +++ b/arch/i386/kernel/smp.c
> > > @@ -507,11 +507,13 @@ struct call_data_struct {
> > >  };
> > >
> > >  void lock_ipi_call_lock(void)
> > > +__acquires(call_lock)
> > >  {
> > >  	spin_lock_irq(&call_lock);
> > >  }
> > >
> > >  void unlock_ipi_call_lock(void)
> > > +__releases(call_lock)
> > >  {
> > >  	spin_unlock_irq(&call_lock);
> > >  }
> >
> > Wrong place. Prototypes should be marked instead. How else would you
> > know about:
> >
> > 	lock_ipi_call_lock();
> > 	if (foo)
> > 		return -E;
> > 	lock_ipi_call_lock();
> >
> > on another compilation unit?
> >
>
> I've re-thought about this since my last email, and I see what you are
> saying now. Functions which are static shouldn't need the __releases()
> or __acquires() anywhere except the definition, since they cannot be
> used outside of the file in which they reside. Non-static functions do
> need to be marked in the prototypes (so all external uses see the
> marking, and therefore get the benefit of sparse's checking) as well as
> in the definition (to quiet sparse itself).

Yes, that's it.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/