[PATCH] locking/lockdep: Remove unused variable in __lockdep_count*()

[PATCH] locking/lockdep: Remove unused variable in __lockdep_count*()

[wuchi]

The target_entry variable will never be referenced because the
function noop_count() always returns false in __bfs().
So just remove that.

Signed-off-by: wuchi <wuchi.zero@gmail.com>
---
 kernel/locking/lockdep.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index e3375bc40dad..04d2ce5d0215 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2075,9 +2075,8 @@ static bool noop_count(struct lock_list *entry, void *data)
 static unsigned long __lockdep_count_forward_deps(struct lock_list *this)
 {
 	unsigned long  count = 0;
-	struct lock_list *target_entry;
 
-	__bfs_forwards(this, (void *)&count, noop_count, NULL, &target_entry);
+	__bfs_forwards(this, (void *)&count, noop_count, NULL, NULL);
 
 	return count;
 }
@@ -2100,9 +2099,8 @@ unsigned long lockdep_count_forward_deps(struct lock_class *class)
 static unsigned long __lockdep_count_backward_deps(struct lock_list *this)
 {
 	unsigned long  count = 0;
-	struct lock_list *target_entry;
 
-	__bfs_backwards(this, (void *)&count, noop_count, NULL, &target_entry);
+	__bfs_backwards(this, (void *)&count, noop_count, NULL, NULL);
 
 	return count;
 }
-- 
2.20.1

Re: [PATCH] locking/lockdep: Remove unused variable in __lockdep_count*()

[Waiman Long]

On 10/30/22 07:17, wuchi wrote:
> The target_entry variable will never be referenced because the
> function noop_count() always returns false in __bfs().
> So just remove that.
>
> Signed-off-by: wuchi <wuchi.zero@gmail.com>
> ---
>   kernel/locking/lockdep.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
> index e3375bc40dad..04d2ce5d0215 100644
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -2075,9 +2075,8 @@ static bool noop_count(struct lock_list *entry, void *data)
>   static unsigned long __lockdep_count_forward_deps(struct lock_list *this)
>   {
>   	unsigned long  count = 0;
> -	struct lock_list *target_entry;
>   
> -	__bfs_forwards(this, (void *)&count, noop_count, NULL, &target_entry);
> +	__bfs_forwards(this, (void *)&count, noop_count, NULL, NULL);
>   
>   	return count;
>   }
> @@ -2100,9 +2099,8 @@ unsigned long lockdep_count_forward_deps(struct lock_class *class)
>   static unsigned long __lockdep_count_backward_deps(struct lock_list *this)
>   {
>   	unsigned long  count = 0;
> -	struct lock_list *target_entry;
>   
> -	__bfs_backwards(this, (void *)&count, noop_count, NULL, &target_entry);
> +	__bfs_backwards(this, (void *)&count, noop_count, NULL, NULL);
>   
>   	return count;
>   }

There is no target_entry NULL check in __bfs(), so target_entry is 
always expected to point to a valid memory location. You will need to 
add the NULL check if you want to remove it from these two functions.

BTW, have you actually exercised the code to make sure that there was no 
unexpected side effect from this change?

Cheers,
Longman

Re: [PATCH] locking/lockdep: Remove unused variable in __lockdep_count*()

[chi wu]

Waiman Long <longman@redhat.com> 于2022年10月30日周日 23:21写道：
>
> There is no target_entry NULL check in __bfs(), so target_entry is
> always expected to point to a valid memory location. You will need to
> add the NULL check if you want to remove it from these two functions.
>
Yes,  add code in __bfs() as follow?

if (match(lock, data)) {
    if (target_entry)
        *target_entry = lock;
     return BFS_RMATCH;
}

> BTW, have you actually exercised the code to make sure that there was no
> unexpected side effect from this change?
Yes, the code "check_irq_usage -> __bfs_backwards" is an example and the
primary callers lockdep_stats_show and l_show in lockdep_proc.c work well.

But I'm not sure I missed anything.
>
> Cheers,
> Longman
>
Thank you for reply