| Message ID | 20180503035931.22439-2-pasha.tatashin@oracle.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Jeff Kirsher |
| Headers | show |
| Series | multi-threading device shutdown | expand |
Hi Alex, > I'm not a fan of dropping the mutex while we go through > ixgbe_close_suspend. I'm concerned it will result in us having a > number of races on shutdown. I would agree, but ixgbe_close_suspend() is already called without this mutex from ixgbe_close(). This path is executed also during machine shutdown but when kexec'ed. So, it is either an existing bug or there are no races. But, because ixgbe_close() is called from the userland, and a little earlier than ixgbe_shutdown() I think this means there are no races. > If anything, I think we would need to find a replacement for the mutex > that can operate at the per-netdev level if you are wanting to > parallelize this. Yes, if lock is necessary, it can be replaced in this place (and added to ixgbe_close()) with something scalable. Thank you, Pavel
> -----Original Message----- > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On > Behalf Of Pavel Tatashin > Sent: Wednesday, May 2, 2018 9:00 PM > To: pasha.tatashin@oracle.com; steven.sistare@oracle.com; > daniel.m.jordan@oracle.com; linux-kernel@vger.kernel.org; Kirsher, Jeffrey > T <jeffrey.t.kirsher@intel.com>; intel-wired-lan@lists.osuosl.org; > netdev@vger.kernel.org; gregkh@linuxfoundation.org > Subject: [Intel-wired-lan] [PATCH 1/2] ixgbe: release lock for the duration of > ixgbe_suspend_close() > > Currently, during device_shutdown() ixgbe holds rtnl_lock for the duration of > lengthy ixgbe_close_suspend(). On machines with multiple ixgbe cards this > lock prevents scaling if device_shutdown() function is multi-threaded. > > It is not necessary to hold this lock during ixgbe_close_suspend() as it is not > held when ixgbe_close() is called also during shutdown but for kexec case. > > Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index afadba99f7b8..e7875b58854b 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -6748,8 +6748,15 @@ static int __ixgbe_shutdown(struct pci_dev *pdev, bool *enable_wake) rtnl_lock(); netif_device_detach(netdev); - if (netif_running(netdev)) + if (netif_running(netdev)) { + /* Suspend takes a long time, device_shutdown may be + * parallelized this function, so drop lock for the + * duration of this call. + */ + rtnl_unlock(); ixgbe_close_suspend(adapter); + rtnl_lock(); + } ixgbe_clear_interrupt_scheme(adapter); rtnl_unlock();
Currently, during device_shutdown() ixgbe holds rtnl_lock for the duration of lengthy ixgbe_close_suspend(). On machines with multiple ixgbe cards this lock prevents scaling if device_shutdown() function is multi-threaded. It is not necessary to hold this lock during ixgbe_close_suspend() as it is not held when ixgbe_close() is called also during shutdown but for kexec case. Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)