lkml.org 
[lkml]   [2021]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [RFC PATCH v1 2/3] locking/lockdep: Unify the return values of check_wait_context()
Date
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"?
> 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. For clarity,
maybe you should state the meaning of the return value in the comment
above the function.

Cheers,
Longman

check_wait_context

\
 
 \ /
  Last update: 2021-07-11 17:20    [W:0.113 / U:0.748 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site