Messages in this thread | | | From | Xiongwei Song <> | Date | Mon, 12 Jul 2021 15:48:42 +0800 | Subject | Re: [RFC PATCH v1 2/3] locking/lockdep: Unify the return values of check_wait_context() |
| |
On Sun, Jul 11, 2021 at 11:19 PM Waiman Long <llong@redhat.com> wrote: > > On 7/11/21 10:14 AM, Xiongwei Song wrote: > > From: Xiongwei Song <sxwjean@gmail.com> > > > > Unity the return values of check_wait_context() as check_prev_add(), > "Unify"? Sorry. Will improve the description.
> > check_irq_usage(), etc. 1 means no bug, 0 means there is a bug. > > > > The return values of print_lock_invalid_wait_context() are unnecessary, > > remove them. > > > > Signed-off-by: Xiongwei Song <sxwjean@gmail.com> > > --- > > kernel/locking/lockdep.c | 20 ++++++++++---------- > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c > > index bf1c00c881e4..8b50da42f2c6 100644 > > --- a/kernel/locking/lockdep.c > > +++ b/kernel/locking/lockdep.c > > @@ -4635,16 +4635,16 @@ static inline short task_wait_context(struct task_struct *curr) > > return LD_WAIT_MAX; > > } > > > > -static int > > +static void > > print_lock_invalid_wait_context(struct task_struct *curr, > > struct held_lock *hlock) > > { > > short curr_inner; > > > > if (!debug_locks_off()) > > - return 0; > > + return; > > if (debug_locks_silent) > > - return 0; > > + return; > > > > pr_warn("\n"); > > pr_warn("=============================\n"); > > @@ -4664,8 +4664,6 @@ print_lock_invalid_wait_context(struct task_struct *curr, > > > > pr_warn("stack backtrace:\n"); > > dump_stack(); > > - > > - return 0; > > } > > > > /* > > @@ -4691,7 +4689,7 @@ static int check_wait_context(struct task_struct *curr, struct held_lock *next) > > int depth; > > > > if (!next_inner || next->trylock) > > - return 0; > > + return 1; > > > > if (!next_outer) > > next_outer = next_inner; > > @@ -4723,10 +4721,12 @@ static int check_wait_context(struct task_struct *curr, struct held_lock *next) > > } > > } > > > > - if (next_outer > curr_inner) > > - return print_lock_invalid_wait_context(curr, next); > > + if (next_outer > curr_inner) { > > + print_lock_invalid_wait_context(curr, next); > > + return 0; > > + } > > > > - return 0; > > + return 1; > > } > > > > #else /* CONFIG_PROVE_LOCKING */ > > @@ -4962,7 +4962,7 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass, > > #endif > > hlock->pin_count = pin_count; > > > > - if (check_wait_context(curr, hlock)) > > + if (!check_wait_context(curr, hlock)) > > return 0; > > > > /* Initialize the lock usage bit */ > > There is also another check_wait_context() in the "#else > CONFIG_PROVE_LOCKING" path that needs to be kept in sync. Oops, my fault.
For clarity, > maybe you should state the meaning of the return value in the comment > above the function. Good point. Thanks.
Regards, Xiongwei > > Cheers, > Longman > > check_wait_context >
| |