netdev - Re: [net-next V2 03/15] net/mlx5: Avoid false positive lockdep warning by adding lock_class

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a8c46806-3c2a-42cd-8adc-654c8902692d@intel.com>
Date: Thu, 12 Oct 2023 14:23:27 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Saeed Mahameed <saeed@...nel.org>, "David S. Miller"
	<davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
	<pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>
CC: Saeed Mahameed <saeedm@...dia.com>, <netdev@...r.kernel.org>, Tariq Toukan
	<tariqt@...dia.com>, Shay Drory <shayd@...dia.com>, Mark Bloch
	<mbloch@...dia.com>
Subject: Re: [net-next V2 03/15] net/mlx5: Avoid false positive lockdep
 warning by adding lock_class_key



On 10/12/2023 12:27 PM, Saeed Mahameed wrote:
> From: Shay Drory <shayd@...dia.com>
> 
> Downstream patch will add devcom component which will be locked in
> many places. This can lead to a false positive "possible circular
> locking dependency" warning by lockdep, on flows which lock more than
> one mlx5 devcom component, such as probing ETH aux device.
> Hence, add a lock_class_key per mlx5 device.
> 

Right, because the default init_rwsem creates a static key that would
then be re-used for each time. Makes sense.

I wondered if there was an init function that also handles registering
and initializing the key too, but doesn't appear to be something that
already exists, and plenty of examples doing it this way.

Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>

> Signed-off-by: Shay Drory <shayd@...dia.com>
> Reviewed-by: Mark Bloch <mbloch@...dia.com>
> Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c
> index 00e67910e3ee..89ac3209277e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c
> @@ -31,6 +31,7 @@ struct mlx5_devcom_comp {
>  	struct kref ref;
>  	bool ready;
>  	struct rw_semaphore sem;
> +	struct lock_class_key lock_key;
>  };
>  
>  struct mlx5_devcom_comp_dev {
> @@ -119,6 +120,8 @@ mlx5_devcom_comp_alloc(u64 id, u64 key, mlx5_devcom_event_handler_t handler)
>  	comp->key = key;
>  	comp->handler = handler;
>  	init_rwsem(&comp->sem);
> +	lockdep_register_key(&comp->lock_key);
> +	lockdep_set_class(&comp->sem, &comp->lock_key);
>  	kref_init(&comp->ref);
>  	INIT_LIST_HEAD(&comp->comp_dev_list_head);
>  
> @@ -133,6 +136,7 @@ mlx5_devcom_comp_release(struct kref *ref)
>  	mutex_lock(&comp_list_lock);
>  	list_del(&comp->comp_list);
>  	mutex_unlock(&comp_list_lock);
> +	lockdep_unregister_key(&comp->lock_key);
>  	kfree(comp);
>  }
>