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] [day] [month] [year] [list]

Date: Mon, 10 Jun 2024 09:20:18 +0200
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: Jakub Kicinski <kuba@...nel.org>, 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 Fri, Jun 07, 2024 at 02:20:01PM -0700, Jacob Keller wrote:
> 
> 
> On 6/6/2024 10:10 PM, Michal Swiatkowski wrote:
> > 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
> 
> I take it you will make a respin of the 4 subfunction patches in this
> series then?

Ok, I will send.