From: "Romanowski, Rafal" <rafal.romanowski@intel.com>
To: "Kitszel, Przemyslaw" <przemyslaw.kitszel@intel.com>,
"Keller, Jacob E" <jacob.e.keller@intel.com>,
Intel Wired LAN <intel-wired-lan@lists.osuosl.org>,
"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>
Cc: "Ertman, David M" <david.m.ertman@intel.com>
Subject: Re: [Intel-wired-lan] [PATCH iwl-net] ice: fix LAG and VF lock dependency in ice_reset_vf()
Date: Thu, 18 Apr 2024 12:24:13 +0000 [thread overview]
Message-ID: <SJ0PR11MB5865A7CDB2316F2941D839488F0E2@SJ0PR11MB5865.namprd11.prod.outlook.com> (raw)
In-Reply-To: <4fb6b49e-7301-48ae-94dc-bd4d3c4bea06@intel.com>
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Przemek Kitszel
> Sent: Thursday, April 11, 2024 2:29 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>; Intel Wired LAN <intel-wired-
> lan@lists.osuosl.org>; Nguyen, Anthony L <anthony.l.nguyen@intel.com>
> Cc: Ertman, David M <david.m.ertman@intel.com>
> 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 <jacob.e.keller@intel.com>
> > Reviewed-by: Dave Ertman <david.m.ertman@intel.com>
> > ---
> > 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 <przemyslaw.kitszel@intel.com>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
prev parent reply other threads:[~2024-04-18 12:24 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-08 23:03 [Intel-wired-lan] [PATCH iwl-net] ice: fix LAG and VF lock dependency in ice_reset_vf() Jacob Keller
2024-04-09 7:09 ` Mateusz Polchlopek
2024-04-11 12:28 ` Przemek Kitszel
2024-04-18 12:24 ` Romanowski, Rafal [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=SJ0PR11MB5865A7CDB2316F2941D839488F0E2@SJ0PR11MB5865.namprd11.prod.outlook.com \
--to=rafal.romanowski@intel.com \
--cc=anthony.l.nguyen@intel.com \
--cc=david.m.ertman@intel.com \
--cc=intel-wired-lan@lists.osuosl.org \
--cc=jacob.e.keller@intel.com \
--cc=przemyslaw.kitszel@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).