netdev - Re: [PATCH v2 3/7] ice: move devlink locking outside the port creation

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]

Date: Fri, 7 Jun 2024 07:10:13 +0200
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Jacob Keller <jacob.e.keller@...el.com>,
	netdev <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>,
	Sujai Buvaneswaran <sujai.buvaneswaran@...el.com>,
	Jiri Pirko <jiri@...nulli.us>
Subject: Re: [PATCH v2 3/7] ice: move devlink locking outside the port
 creation

On Thu, Jun 06, 2024 at 05:56:34PM -0700, Jakub Kicinski wrote:
> On Wed, 05 Jun 2024 13:40:43 -0700 Jacob Keller wrote:
> > From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> > 
> > In case of subfunction lock will be taken for whole port creation. Do
> > the same in VF case.
> 
> No interactions with other locks worth mentioning?
> 

You right, I could have mentioned also removing path. The patch is only
about devlink lock during port representor creation / removing.

> > diff --git a/drivers/net/ethernet/intel/ice/devlink/devlink.c b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> > index 704e9ad5144e..f774781ab514 100644
> > --- a/drivers/net/ethernet/intel/ice/devlink/devlink.c
> > +++ b/drivers/net/ethernet/intel/ice/devlink/devlink.c
> > @@ -794,10 +794,8 @@ int ice_devlink_rate_init_tx_topology(struct devlink *devlink, struct ice_vsi *v
> >  
> >  	tc_node = pi->root->children[0];
> >  	mutex_lock(&pi->sched_lock);
> > -	devl_lock(devlink);
> >  	for (i = 0; i < tc_node->num_children; i++)
> >  		ice_traverse_tx_tree(devlink, tc_node->children[i], tc_node, pf);
> > -	devl_unlock(devlink);
> >  	mutex_unlock(&pi->sched_lock);
> 
> Like this didn't use to cause a deadlock?
> 
> Seems ice_devlink_rate_node_del() takes this lock and it's already
> holding the devlink instance lock.

ice_devlink_rate_init_tx_topology() wasn't (till now) called with
devlink lock, because it is called from port representor creation flow,
not from the devlink.

Thanks,
Michal