Messages in this thread | | | From | Jules Irenge <> | Date | Sat, 1 Feb 2020 00:27:02 +0000 (GMT) | Subject | Re: [PATCH 3/3] mutex: Add missing annotations |
| |
Thanks for the feedback, I did not have a thorough analyse.
I just realised it. Yes, some warnings generated after are pointless others look genuine. Next time I will have a thorough analyse and test before sending.
I apologise for this.
I really appreciate your feedback.
Kind regards, Jules
On Mon, 27 Jan 2020, Peter Zijlstra wrote:
> On Fri, Jan 24, 2020 at 08:12:20PM +0000, Jules Irenge wrote: > > Sparse reports false warnings and hide real warnings > > where mutex_lock() and mutex_unlock() are used within the kernel > > An example is within the kernel cgroup files > > where the below warnings are found > > |warning: context imbalance in cgroup_lock_and_drain_offline() > > | - wrong count at exit > > |warning: context imbalance in cgroup_procs_write_finish() > > |- wrong count at exit > > |warning: context imbalance in cgroup_procs_write_start() > > |- wrong count at exit. > > > > To fix these, > > an __acquires(lock) is added to mutex_lock() declaration > > a __releases(lock) to mutex_unlock() declaration > > > > Signed-off-by: Jules Irenge <jbi.octave@gmail.com> > > --- > > include/linux/mutex.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/include/linux/mutex.h b/include/linux/mutex.h > > index aca8f36dfac9..a8ab4029913e 100644 > > --- a/include/linux/mutex.h > > +++ b/include/linux/mutex.h > > @@ -162,7 +162,7 @@ do { \ > > } while (0) > > > > #else > > -extern void mutex_lock(struct mutex *lock); > > +extern void mutex_lock(struct mutex *lock) __acquires(lock); > > extern int __must_check mutex_lock_interruptible(struct mutex *lock); > > extern int __must_check mutex_lock_killable(struct mutex *lock); > > extern void mutex_lock_io(struct mutex *lock); > > @@ -181,7 +181,7 @@ extern void mutex_lock_io(struct mutex *lock); > > * Returns 1 if the mutex has been acquired successfully, and 0 on contention. > > */ > > extern int mutex_trylock(struct mutex *lock); > > -extern void mutex_unlock(struct mutex *lock); > > +extern void mutex_unlock(struct mutex *lock) __releases(lock); > > > > extern int atomic_dec_and_mutex_lock(atomic_t *cnt, struct mutex *lock); > > *groan*, I despise these sparse things. > > The proposed patch only annotates a tiny part of the mutex interface, > and will thereby generate a flood of new (pointless) warnings. Worse, > annotating them all properly will require that __cond_lock() trainwreck. > >
| |