Messages in this thread | | | From | "Laba, SlawomirX" <> | Subject | RE: [PATCH net v2] iavf: Detach device during reset task | Date | Tue, 30 Aug 2022 20:49:54 +0000 |
| |
> -----Original Message----- > From: Ivan Vecera <ivecera@redhat.com> > Sent: Tuesday, August 30, 2022 10:16 AM > Subject: [PATCH net v2] iavf: Detach device during reset task > > iavf_reset_task() takes crit_lock at the beginning and holds it during whole call. > The function subsequently calls > iavf_init_interrupt_scheme() that grabs RTNL. Problem occurs when userspace > initiates during the reset task any ndo callback that runs under RTNL like > iavf_open() because some of that functions tries to take crit_lock. This leads to > classic A-B B-A deadlock scenario. > > To resolve this situation the device should be detached in > iavf_reset_task() prior taking crit_lock to avoid subsequent ndos running under > RTNL and reattach the device at the end. > > Fixes: 62fe2a865e6d ("i40evf: add missing rtnl_lock() around > i40evf_set_interrupt_capability") > Cc: Jacob Keller <jacob.e.keller@intel.com> > Cc: Patryk Piotrowski <patryk.piotrowski@intel.com> > Cc: SlawomirX Laba <slawomirx.laba@intel.com> > Tested-by: Vitaly Grinberg <vgrinber@redhat.com> > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > > @@ -2884,7 +2889,7 @@ static void iavf_reset_task(struct work_struct *work) > if (adapter->state != __IAVF_REMOVE) > queue_work(iavf_wq, &adapter->reset_task); > > - return; > + goto reset_finish; > } > > while (!mutex_trylock(&adapter->client_lock))
Ivan, what do you think about this flow [1]? Shouldn't it also goto reset_finish label?
if (i == IAVF_RESET_WAIT_COMPLETE_COUNT) { dev_err(&adapter->pdev->dev, "Reset never finished (%x)\n", reg_val); iavf_disable_vf(adapter); mutex_unlock(&adapter->client_lock); mutex_unlock(&adapter->crit_lock); return; /* Do not attempt to reinit. It's dead, Jim. */ }
I am concerned that if the reset never finishes and iavf goes into disabled state, and then for example if driver reload operation is performed, bad things can happen.
[1] https://elixir.bootlin.com/linux/v6.0-rc3/source/drivers/net/ethernet/intel/iavf/iavf_main.c#L2939
| |