diff mbox series

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

Message ID 20221030111759.95092-1-wuchi.zero@gmail.com
State New
Headers
Series locking/lockdep: Remove unused variable in __lockdep_count*() |

Commit Message

wuchi Oct. 30, 2022, 11:17 a.m. UTC 
  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(-)

Comments

Waiman Long Oct. 30, 2022, 3:20 p.m. UTC | #1 
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
wuchi Oct. 31, 2022, 5:55 a.m. UTC | #2 
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
diff mbox series

Patch

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;
 }