> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf Of > Przemek Kitszel > Sent: Thursday, April 11, 2024 2:29 PM > To: Keller, Jacob E <[email protected]>; Intel Wired LAN <intel-wired- > [email protected]>; Nguyen, Anthony L <[email protected]> > Cc: Ertman, David M <[email protected]> > Subject: Re: [Intel-wired-lan] [PATCH iwl-net] ice: fix LAG and VF lock > dependency in ice_reset_vf() > > On 4/9/24 01:03, Jacob Keller wrote: > > 9f74a3dfcf83 ("ice: Fix VF Reset paths when interface in a failed over > > aggregate"), the ice driver has acquired the LAG mutex in ice_reset_vf(). > > The commit placed this lock acquisition just prior to the acquisition > > of the VF configuration lock. > > > > If ice_reset_vf() acquires the configuration lock via the > > ICE_VF_RESET_LOCK flag, this could deadlock with ice_vc_cfg_qs_msg() > > because it always acquires the locks in the order of the VF > > configuration lock and then the LAG mutex. > > > > Lockdep reports this violation almost immediately on creating and then > > removing 2 VF: > > > > [...] > > > To avoid deadlock, we must acquire the LAG mutex only after acquiring > > the VF configuration lock. Fix the ice_reset_vf() to acquire the LAG > > mutex only after we either acquire or check that the VF configuration lock > > is > held. > > > > Fixes: 9f74a3dfcf83 ("ice: Fix VF Reset paths when interface in a > > failed over aggregate") > > Signed-off-by: Jacob Keller <[email protected]> > > Reviewed-by: Dave Ertman <[email protected]> > > --- > > drivers/net/ethernet/intel/ice/ice_vf_lib.c | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_vf_lib.c > > b/drivers/net/ethernet/intel/ice/ice_vf_lib.c > > index 21d26e19338a..d10a4be965b5 100644 > > --- a/drivers/net/ethernet/intel/ice/ice_vf_lib.c > > +++ b/drivers/net/ethernet/intel/ice/ice_vf_lib.c > > @@ -856,6 +856,11 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags) > > return 0; > > } > > > > + if (flags & ICE_VF_RESET_LOCK) > > + mutex_lock(&vf->cfg_lock); > > + else > > + lockdep_assert_held(&vf->cfg_lock); > > + > > lag = pf->lag; > > mutex_lock(&pf->lag_mutex); > > if (lag && lag->bonded && lag->primary) { @@ -867,11 +872,6 @@ int > > ice_reset_vf(struct ice_vf *vf, u32 flags) > > act_prt = ICE_LAG_INVALID_PORT; > > } > > > > - if (flags & ICE_VF_RESET_LOCK) > > - mutex_lock(&vf->cfg_lock); > > - else > > - lockdep_assert_held(&vf->cfg_lock); > > - > > if (ice_is_vf_disabled(vf)) { > > vsi = ice_get_vf_vsi(vf); > > if (!vsi) { > > @@ -956,14 +956,14 @@ int ice_reset_vf(struct ice_vf *vf, u32 flags) > > ice_mbx_clear_malvf(&vf->mbx_info); > > > > out_unlock: > > - if (flags & ICE_VF_RESET_LOCK) > > - mutex_unlock(&vf->cfg_lock); > > - > > if (lag && lag->bonded && lag->primary && > > act_prt != ICE_LAG_INVALID_PORT) > > ice_lag_move_vf_nodes_cfg(lag, pri_prt, act_prt); > > mutex_unlock(&pf->lag_mutex); > > > > + if (flags & ICE_VF_RESET_LOCK) > > + mutex_unlock(&vf->cfg_lock); > > + > > return err; > > } > > > > > > base-commit: 3ca3256cde596573d060eda8c477996435c6d63f > > Thanks, > Tested-by: Przemek Kitszel <[email protected]>
Tested-by: Rafal Romanowski <[email protected]>
