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]

Message-ID: <49f3020a-1b2a-44c5-8ea5-938aa2195144@intel.com>
Date: Fri, 7 Jun 2024 14:20:01 -0700
From: Jacob Keller <jacob.e.keller@...el.com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>, Jakub Kicinski
	<kuba@...nel.org>
CC: 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 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?