On Mon, May 27, 2019 at 05:14:38PM +0200, Peter Zijlstra wrote:
> On Fri, May 24, 2019 at 11:15:09PM +0300, Imre Deak wrote:
> > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> > index 967352d32af1..9e2a4ab6c731 100644
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -3637,6 +3637,11 @@ print_lock_nested_lock_not_held(struct task_struct
> > *curr,
> >
> > static int __lock_is_held(const struct lockdep_map *lock, int read);
> >
> > +static int hlock_reference(int reference)
> > +{
> > + return reference ? : 1;
> > +}
> > +
> > /*
> > * This gets called for every mutex_lock*()/spin_lock*() operation.
> > * We maintain the dependency maps and validate the locking attempt:
> > @@ -3702,17 +3707,15 @@ static int __lock_acquire(struct lockdep_map *lock,
> > unsigned int subclass,
> > if (depth) {
> > hlock = curr->held_locks + depth - 1;
> > if (hlock->class_idx == class_idx && nest_lock) {
> > - if (hlock->references) {
> > - /*
> > - * Check: unsigned int references overflow.
> > - */
> > - if (DEBUG_LOCKS_WARN_ON(hlock->references ==
> > UINT_MAX))
>
> What tree is this against?
I just used our
git://anongit.freedesktop.org/drm-tip
and the most recent upstream commit in that is:
$ git merge-base drm-tip origin/master
6b0538da5a6ca2129b93cea5afc997226875c402
which has the commit
commit a188339ca5a396acc588e5851ed7e19f66b0ebd9
Author: Linus Torvalds <torva...@linux-foundation.org>
Date: Sun May 19 15:47:09 2019 -0700
Linux 5.2-rc1
> Afaict this is still 12 bits ?!
In the above tree I see
unsigned int references;
in held_lock which is 32 bits.
>
> > - return 0;
> > + /*
> > + * Check: unsigned int references overflow.
> > + */
> > + if
> > (DEBUG_LOCKS_WARN_ON(hlock_reference(hlock->references) >
> > + UINT_MAX -
> > hlock_reference(references)))
>
> Idem. Also very weird overflow check..
We could have instead (replacing the addition itself too below):
if (DEBUG_LOCKS_WARN_ON(
check_add_overflow(hlock_reference(hlock->references),
hlock_reference(references),
&hlock_references)))
return 0;
by having hlock_reference() take and return unsigned int too.
>
> > + return 0;
> >
> > - hlock->references++;
> > - } else {
> > - hlock->references = 2;
> > - }
> > + hlock->references = hlock_reference(hlock->references) +
> > + hlock_reference(references);
> >
> > return 2;
> > }
> > --
> > 2.17.1
> >
