| Message ID | 20230217004201.2895321-1-anthony.l.nguyen@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [net,v2,1/1] ice: avoid bonding causing auxiliary plug/unplug under RTNL lock | expand |
On 2/16/2023 4:42 PM, Tony Nguyen wrote: > From: Dave Ertman <david.m.ertman@intel.com> > > RDMA is not supported in ice on a PF that has been added to a bonded > interface. To enforce this, when an interface enters a bond, we unplug > the auxiliary device that supports RDMA functionality. This unplug > currently happens in the context of handling the netdev bonding event. > This event is sent to the ice driver under RTNL context. This is causing > a deadlock where the RDMA driver is waiting for the RTNL lock to complete > the removal. > > Defer the unplugging/re-plugging of the auxiliary device to the service > task so that it is not performed under the RTNL lock context. > > Cc: stable@vger.kernel.org # 6.1.x > Reported-by: Jaroslav Pulchart <jaroslav.pulchart@gooddata.com> > Link: https://lore.kernel.org/netdev/CAK8fFZ6A_Gphw_3-QMGKEFQk=sfCw1Qmq0TVZK3rtAi7vb621A@mail.gmail.com/ > Fixes: 5cb1ebdbc434 ("ice: Fix race condition during interface enslave") > Fixes: 4eace75e0853 ("RDMA/irdma: Report the correct link speed") > Signed-off-by: Dave Ertman <david.m.ertman@intel.com> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> > --- > v2: > (Removed from original pull request) > - Reversed order of bit processing in ice_service_task for PLUG/UNPLUG Please ignore/drop this. Just saw that this change doesn't solve the issue. https://lore.kernel.org/intel-wired-lan/ygay1oxikvo.fsf@localhost/T/#m83d7162b2830f248a51d121965443687d7f54654 Thanks, Tony > v1: https://lore.kernel.org/netdev/20230131213703.1347761-2-anthony.l.nguyen@intel.com/ > > drivers/net/ethernet/intel/ice/ice.h | 14 +++++--------- > drivers/net/ethernet/intel/ice/ice_main.c | 19 ++++++++----------- > 2 files changed, 13 insertions(+), 20 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h > index 713069f809ec..3cad5e6b2ad1 100644 > --- a/drivers/net/ethernet/intel/ice/ice.h > +++ b/drivers/net/ethernet/intel/ice/ice.h > @@ -506,6 +506,7 @@ enum ice_pf_flags { > ICE_FLAG_VF_VLAN_PRUNING, > ICE_FLAG_LINK_LENIENT_MODE_ENA, > ICE_FLAG_PLUG_AUX_DEV, > + ICE_FLAG_UNPLUG_AUX_DEV, > ICE_FLAG_MTU_CHANGED, > ICE_FLAG_GNSS, /* GNSS successfully initialized */ > ICE_PF_FLAGS_NBITS /* must be last */ > @@ -950,16 +951,11 @@ static inline void ice_set_rdma_cap(struct ice_pf *pf) > */ > static inline void ice_clear_rdma_cap(struct ice_pf *pf) > { > - /* We can directly unplug aux device here only if the flag bit > - * ICE_FLAG_PLUG_AUX_DEV is not set because ice_unplug_aux_dev() > - * could race with ice_plug_aux_dev() called from > - * ice_service_task(). In this case we only clear that bit now and > - * aux device will be unplugged later once ice_plug_aux_device() > - * called from ice_service_task() finishes (see ice_service_task()). > + /* defer unplug to service task to avoid RTNL lock and > + * clear PLUG bit so that pending plugs don't interfere > */ > - if (!test_and_clear_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) > - ice_unplug_aux_dev(pf); > - > + clear_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags); > + set_bit(ICE_FLAG_UNPLUG_AUX_DEV, pf->flags); > clear_bit(ICE_FLAG_RDMA_ENA, pf->flags); > } > #endif /* _ICE_H_ */ > diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c > index 8ec24f6cf6be..10d1c5b10d2a 100644 > --- a/drivers/net/ethernet/intel/ice/ice_main.c > +++ b/drivers/net/ethernet/intel/ice/ice_main.c > @@ -2316,18 +2316,15 @@ static void ice_service_task(struct work_struct *work) > } > } > > - if (test_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) { > - /* Plug aux device per request */ > - ice_plug_aux_dev(pf); > + /* unplug aux dev per request, if an unplug request came in > + * while processing a plug request, this will handle it > + */ > + if (test_and_clear_bit(ICE_FLAG_UNPLUG_AUX_DEV, pf->flags)) > + ice_unplug_aux_dev(pf); > > - /* Mark plugging as done but check whether unplug was > - * requested during ice_plug_aux_dev() call > - * (e.g. from ice_clear_rdma_cap()) and if so then > - * plug aux device. > - */ > - if (!test_and_clear_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) > - ice_unplug_aux_dev(pf); > - } > + /* Plug aux device per request */ > + if (test_and_clear_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) > + ice_plug_aux_dev(pf); > > if (test_and_clear_bit(ICE_FLAG_MTU_CHANGED, pf->flags)) { > struct iidc_event *event;
diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h index 713069f809ec..3cad5e6b2ad1 100644 --- a/drivers/net/ethernet/intel/ice/ice.h +++ b/drivers/net/ethernet/intel/ice/ice.h @@ -506,6 +506,7 @@ enum ice_pf_flags { ICE_FLAG_VF_VLAN_PRUNING, ICE_FLAG_LINK_LENIENT_MODE_ENA, ICE_FLAG_PLUG_AUX_DEV, + ICE_FLAG_UNPLUG_AUX_DEV, ICE_FLAG_MTU_CHANGED, ICE_FLAG_GNSS, /* GNSS successfully initialized */ ICE_PF_FLAGS_NBITS /* must be last */ @@ -950,16 +951,11 @@ static inline void ice_set_rdma_cap(struct ice_pf *pf) */ static inline void ice_clear_rdma_cap(struct ice_pf *pf) { - /* We can directly unplug aux device here only if the flag bit - * ICE_FLAG_PLUG_AUX_DEV is not set because ice_unplug_aux_dev() - * could race with ice_plug_aux_dev() called from - * ice_service_task(). In this case we only clear that bit now and - * aux device will be unplugged later once ice_plug_aux_device() - * called from ice_service_task() finishes (see ice_service_task()). + /* defer unplug to service task to avoid RTNL lock and + * clear PLUG bit so that pending plugs don't interfere */ - if (!test_and_clear_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) - ice_unplug_aux_dev(pf); - + clear_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags); + set_bit(ICE_FLAG_UNPLUG_AUX_DEV, pf->flags); clear_bit(ICE_FLAG_RDMA_ENA, pf->flags); } #endif /* _ICE_H_ */ diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c index 8ec24f6cf6be..10d1c5b10d2a 100644 --- a/drivers/net/ethernet/intel/ice/ice_main.c +++ b/drivers/net/ethernet/intel/ice/ice_main.c @@ -2316,18 +2316,15 @@ static void ice_service_task(struct work_struct *work) } } - if (test_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) { - /* Plug aux device per request */ - ice_plug_aux_dev(pf); + /* unplug aux dev per request, if an unplug request came in + * while processing a plug request, this will handle it + */ + if (test_and_clear_bit(ICE_FLAG_UNPLUG_AUX_DEV, pf->flags)) + ice_unplug_aux_dev(pf); - /* Mark plugging as done but check whether unplug was - * requested during ice_plug_aux_dev() call - * (e.g. from ice_clear_rdma_cap()) and if so then - * plug aux device. - */ - if (!test_and_clear_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) - ice_unplug_aux_dev(pf); - } + /* Plug aux device per request */ + if (test_and_clear_bit(ICE_FLAG_PLUG_AUX_DEV, pf->flags)) + ice_plug_aux_dev(pf); if (test_and_clear_bit(ICE_FLAG_MTU_CHANGED, pf->flags)) { struct iidc_event *event;