[RFC] e1000e: Use rtnl_lock to prevent race conditions between net and pci/pm

From: Alexander Duyck <alexander.h.duyck@linux.intel.com>

This patch is meant to address possible race conditions that can exist
between network configuration and power management. A similar issue was
fixed for igb in commit 9474933caf21 ("igb: close/suspend race in
netif_device_detach").

In addition it consolidates the code so that the PCI error handling code
will essentially perform the power management freeze on the device prior to
attempting a reset, and will thaw the device afterwards if that is what it
is planning to do. Otherwise when we call close on the interface it should
see it is detached and not attempt to call the logic to down the interface
and free the IRQs again.

>From what I can tell the check that was adding the check for __E1000_DOWN
in e1000e_close was added when runtime power management was added. However
it should not be relevant for us as we perform a call to
pm_runtime_get_sync before we call e1000_down/free_irq so it should always
be back up before we call into this anyway.

Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
---

I'm putting this out as an RFC for now. I haven't had a chance to do much
testing yet, but I have verified no build issues, and the driver appears
to load, link, and pass traffic without problems.

This should address issues seen with either double freeing or never freeing
IRQs that have been seen on this and similar drivers in the past.

I'll submit this formally after testing it over the weekend assuming there
are no issues.

 drivers/net/ethernet/intel/e1000e/netdev.c |   33 ++++++++++++++--------------
 1 file changed, 17 insertions(+), 16 deletions(-)

On Fri, 2019-10-04 at 16:36 -0700, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
> This patch is meant to address possible race conditions that can exist
> between network configuration and power management. A similar issue was
> fixed for igb in commit 9474933caf21 ("igb: close/suspend race in
> netif_device_detach").
> 
> In addition it consolidates the code so that the PCI error handling code
> will essentially perform the power management freeze on the device prior to
> attempting a reset, and will thaw the device afterwards if that is what it
> is planning to do. Otherwise when we call close on the interface it should
> see it is detached and not attempt to call the logic to down the interface
> and free the IRQs again.
> 
> >From what I can tell the check that was adding the check for __E1000_DOWN
> in e1000e_close was added when runtime power management was added. However
> it should not be relevant for us as we perform a call to
> pm_runtime_get_sync before we call e1000_down/free_irq so it should always
> be back up before we call into this anyway.
> 
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
> 
> I'm putting this out as an RFC for now. I haven't had a chance to do much
> testing yet, but I have verified no build issues, and the driver appears
> to load, link, and pass traffic without problems.
> 
> This should address issues seen with either double freeing or never freeing
> IRQs that have been seen on this and similar drivers in the past.
> 
> I'll submit this formally after testing it over the weekend assuming there
> are no issues.
> 
>  drivers/net/ethernet/intel/e1000e/netdev.c |   33 ++++++++++++++--------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index d7d56e42a6aa..182a2c8f12d8 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -4715,12 +4715,12 @@ int e1000e_close(struct net_device *netdev)
> 
>  	pm_runtime_get_sync(&pdev->dev);
> 
> -	if (!test_bit(__E1000_DOWN, &adapter->state)) {
> +	if (netif_device_present(netdev)) {
>  		e1000e_down(adapter, true);
>  		e1000_free_irq(adapter);
> 
>  		/* Link status message must follow this format */
> -		pr_info("%s NIC Link is Down\n", adapter->netdev->name);
> +		pr_info("%s NIC Link is Down\n", netdev->name);
>  	}
> 
>  	napi_disable(&adapter->napi);
> @@ -6299,6 +6299,7 @@ static int e1000e_pm_freeze(struct device *dev)
>  	struct net_device *netdev = dev_get_drvdata(dev);
>  	struct e1000_adapter *adapter = netdev_priv(netdev);
> 
> +	rtnl_lock();
>  	netif_device_detach(netdev);
> 
>  	if (netif_running(netdev)) {
> @@ -6313,6 +6314,8 @@ static int e1000e_pm_freeze(struct device *dev)
>  		e1000e_down(adapter, false);
>  		e1000_free_irq(adapter);
>  	}
> +	rtnl_unlock();
> +
>  	e1000e_reset_interrupt_capability(adapter);
> 
>  	/* Allow time for pending master requests to run */
> @@ -6626,27 +6629,30 @@ static int __e1000_resume(struct pci_dev *pdev)
>  	return 0;
>  }
> 
> -#ifdef CONFIG_PM_SLEEP
>  static int e1000e_pm_thaw(struct device *dev)
>  {
>  	struct net_device *netdev = dev_get_drvdata(dev);
>  	struct e1000_adapter *adapter = netdev_priv(netdev);
> +	int rc = 0;
> 
>  	e1000e_set_interrupt_capability(adapter);
> -	if (netif_running(netdev)) {
> -		u32 err = e1000_request_irq(adapter);
> 
> -		if (err)
> -			return err;
> +	rtnl_lock();
> +	if (netif_running(netdev)) {
> +		rc = e1000_request_irq(adapter);
> +		if (rc)
> +			goto err_irq;
> 
>  		e1000e_up(adapter);
>  	}
> 
>  	netif_device_attach(netdev);
> -
> -	return 0;
> +	rtnl_unlock();
> +err_irq:
> +	return rc;
>  }
> 
> +#ifdef CONFIG_PM_SLEEP
>  static int e1000e_pm_suspend(struct device *dev)
>  {
>  	struct pci_dev *pdev = to_pci_dev(dev);
> @@ -6821,13 +6827,11 @@ static pci_ers_result_t e1000_io_error_detected(struct pci_dev *pdev,
>  	struct net_device *netdev = pci_get_drvdata(pdev);
>  	struct e1000_adapter *adapter = netdev_priv(netdev);
> 
> -	netif_device_detach(netdev);
> +	e1000e_pm_freeze(&pdev->dev);
> 
>  	if (state == pci_channel_io_perm_failure)
>  		return PCI_ERS_RESULT_DISCONNECT;
> 
> -	if (netif_running(netdev))
> -		e1000e_down(adapter, true);
>  	pci_disable_device(pdev);
> 
>  	/* Request a slot slot reset. */
> @@ -6893,10 +6897,7 @@ static void e1000_io_resume(struct pci_dev *pdev)
> 
>  	e1000_init_manageability_pt(adapter);
> 
> -	if (netif_running(netdev))
> -		e1000e_up(adapter);
> -
> -	netif_device_attach(netdev);
> +	e1000e_pm_thaw(&pdev->dev);
> 
>  	/* If the controller has AMT, do not set DRV_LOAD until the interface
>  	 * is up.  For all other cases, let the f/w know that the h/w is now
In e1000e_pm_thaw(), these 2 lines need to switch order to avoid
deadlock.
from:
+       rtnl_unlock();
+err_irq:

to:
+err_irq:
+       rtnl_unlock();

I will find hardware to test this patch next week. Will update the test
result later.

Thanks! - David

On Fri, Oct 4, 2019 at 7:18 PM David Z. Dai <zdai@linux.vnet.ibm.com> wrote:
>
> On Fri, 2019-10-04 at 16:36 -0700, Alexander Duyck wrote:
> > From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> >
> > This patch is meant to address possible race conditions that can exist
> > between network configuration and power management. A similar issue was
> > fixed for igb in commit 9474933caf21 ("igb: close/suspend race in
> > netif_device_detach").
> >
> > In addition it consolidates the code so that the PCI error handling code
> > will essentially perform the power management freeze on the device prior to
> > attempting a reset, and will thaw the device afterwards if that is what it
> > is planning to do. Otherwise when we call close on the interface it should
> > see it is detached and not attempt to call the logic to down the interface
> > and free the IRQs again.
> >
> > >From what I can tell the check that was adding the check for __E1000_DOWN
> > in e1000e_close was added when runtime power management was added. However
> > it should not be relevant for us as we perform a call to
> > pm_runtime_get_sync before we call e1000_down/free_irq so it should always
> > be back up before we call into this anyway.
> >
> > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> > ---
> >
> > I'm putting this out as an RFC for now. I haven't had a chance to do much
> > testing yet, but I have verified no build issues, and the driver appears
> > to load, link, and pass traffic without problems.
> >
> > This should address issues seen with either double freeing or never freeing
> > IRQs that have been seen on this and similar drivers in the past.
> >
> > I'll submit this formally after testing it over the weekend assuming there
> > are no issues.
> >
> >  drivers/net/ethernet/intel/e1000e/netdev.c |   33 ++++++++++++++--------------
> >  1 file changed, 17 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> > index d7d56e42a6aa..182a2c8f12d8 100644
> > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c

<snip>

> >
> > -#ifdef CONFIG_PM_SLEEP
> >  static int e1000e_pm_thaw(struct device *dev)
> >  {
> >       struct net_device *netdev = dev_get_drvdata(dev);
> >       struct e1000_adapter *adapter = netdev_priv(netdev);
> > +     int rc = 0;
> >
> >       e1000e_set_interrupt_capability(adapter);
> > -     if (netif_running(netdev)) {
> > -             u32 err = e1000_request_irq(adapter);
> >
> > -             if (err)
> > -                     return err;
> > +     rtnl_lock();
> > +     if (netif_running(netdev)) {
> > +             rc = e1000_request_irq(adapter);
> > +             if (rc)
> > +                     goto err_irq;
> >
> >               e1000e_up(adapter);
> >       }
> >
> >       netif_device_attach(netdev);
> > -
> > -     return 0;
> > +     rtnl_unlock();
> > +err_irq:
> > +     return rc;
> >  }
> >
> In e1000e_pm_thaw(), these 2 lines need to switch order to avoid
> deadlock.
> from:
> +       rtnl_unlock();
> +err_irq:
>
> to:
> +err_irq:
> +       rtnl_unlock();
>
> I will find hardware to test this patch next week. Will update the test
> result later.
>
> Thanks! - David

Thanks for spotting that. I will update my copy of the patch for when
I submit the final revision.

I'll probably wait to submit it for acceptance until you have had a
chance to verify that it resolves the issue you were seeing.

Thanks.

- Alex

On Sat, 2019-10-05 at 10:22 -0700, Alexander Duyck wrote:
> On Fri, Oct 4, 2019 at 7:18 PM David Z. Dai <zdai@linux.vnet.ibm.com> wrote:
> >
> > On Fri, 2019-10-04 at 16:36 -0700, Alexander Duyck wrote:
> > > From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> > >
> > > This patch is meant to address possible race conditions that can exist
> > > between network configuration and power management. A similar issue was
> > > fixed for igb in commit 9474933caf21 ("igb: close/suspend race in
> > > netif_device_detach").
> > >
> > > In addition it consolidates the code so that the PCI error handling code
> > > will essentially perform the power management freeze on the device prior to
> > > attempting a reset, and will thaw the device afterwards if that is what it
> > > is planning to do. Otherwise when we call close on the interface it should
> > > see it is detached and not attempt to call the logic to down the interface
> > > and free the IRQs again.
> > >
> > > >From what I can tell the check that was adding the check for __E1000_DOWN
> > > in e1000e_close was added when runtime power management was added. However
> > > it should not be relevant for us as we perform a call to
> > > pm_runtime_get_sync before we call e1000_down/free_irq so it should always
> > > be back up before we call into this anyway.
> > >
> > > Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> > > ---
> > >
> > > I'm putting this out as an RFC for now. I haven't had a chance to do much
> > > testing yet, but I have verified no build issues, and the driver appears
> > > to load, link, and pass traffic without problems.
> > >
> > > This should address issues seen with either double freeing or never freeing
> > > IRQs that have been seen on this and similar drivers in the past.
> > >
> > > I'll submit this formally after testing it over the weekend assuming there
> > > are no issues.
> > >
> > >  drivers/net/ethernet/intel/e1000e/netdev.c |   33 ++++++++++++++--------------
> > >  1 file changed, 17 insertions(+), 16 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> > > index d7d56e42a6aa..182a2c8f12d8 100644
> > > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> 
> <snip>
> 
> > >
> > > -#ifdef CONFIG_PM_SLEEP
> > >  static int e1000e_pm_thaw(struct device *dev)
> > >  {
> > >       struct net_device *netdev = dev_get_drvdata(dev);
> > >       struct e1000_adapter *adapter = netdev_priv(netdev);
> > > +     int rc = 0;
> > >
> > >       e1000e_set_interrupt_capability(adapter);
> > > -     if (netif_running(netdev)) {
> > > -             u32 err = e1000_request_irq(adapter);
> > >
> > > -             if (err)
> > > -                     return err;
> > > +     rtnl_lock();
> > > +     if (netif_running(netdev)) {
> > > +             rc = e1000_request_irq(adapter);
> > > +             if (rc)
> > > +                     goto err_irq;
> > >
> > >               e1000e_up(adapter);
> > >       }
> > >
> > >       netif_device_attach(netdev);
> > > -
> > > -     return 0;
> > > +     rtnl_unlock();
> > > +err_irq:
> > > +     return rc;
> > >  }
> > >
> > In e1000e_pm_thaw(), these 2 lines need to switch order to avoid
> > deadlock.
> > from:
> > +       rtnl_unlock();
> > +err_irq:
> >
> > to:
> > +err_irq:
> > +       rtnl_unlock();
> >
> > I will find hardware to test this patch next week. Will update the test
> > result later.
> >
> > Thanks! - David
> 
> Thanks for spotting that. I will update my copy of the patch for when
> I submit the final revision.
> 
> I'll probably wait to submit it for acceptance until you have had a
> chance to verify that it resolves the issue you were seeing.
> 
> Thanks.
> 
> - Alex
We have tested on one of the test box.
With this patch, it doesn't crash kernel anymore, which is good!

However we see this warning message from the log file for irq number 0:
[10206.317270] Trying to free already-free IRQ 0

With this stack:
[10206.317344] NIP [c00000000018cbf8] __free_irq+0x308/0x370
[10206.317346] LR [c00000000018cbf4] __free_irq+0x304/0x370
[10206.317347] Call Trace:
[10206.317348] [c00000008b92b970] [c00000000018cbf4] __free_irq
+0x304/0x370 (unreliable)
[10206.317351] [c00000008b92ba00] [c00000000018cd84] free_irq+0x84/0xf0
[10206.317358] [c00000008b92ba30] [d000000007449e60] e1000_free_irq
+0x98/0xc0 [e1000e]
[10206.317365] [c00000008b92ba60] [d000000007458a70] e1000e_pm_freeze
+0xb8/0x100 [e1000e]
[10206.317372] [c00000008b92baa0] [d000000007458b6c]
e1000_io_error_detected+0x34/0x70 [e1000e]
[10206.317375] [c00000008b92bad0] [c000000000040358] eeh_report_failure
+0xc8/0x190
[10206.317377] [c00000008b92bb20] [c00000000003eb2c] eeh_pe_dev_traverse
+0x9c/0x170
[10206.317379] [c00000008b92bbb0] [c000000000040d84]
eeh_handle_normal_event+0xe4/0x580
[10206.317382] [c00000008b92bc60] [c000000000041330] eeh_handle_event
+0x30/0x340
[10206.317384] [c00000008b92bd10] [c000000000041780] eeh_event_handler
+0x140/0x200
[10206.317386] [c00000008b92bdc0] [c0000000001397c8] kthread+0x1a8/0x1b0
[10206.317389] [c00000008b92be30] [c00000000000b560]
ret_from_kernel_thread+0x5c/0x7c

Thanks! - David

On Mon, Oct 7, 2019 at 8:51 AM David Z. Dai <zdai@linux.vnet.ibm.com> wrote:
>

<snip>

> We have tested on one of the test box.
> With this patch, it doesn't crash kernel anymore, which is good!
>
> However we see this warning message from the log file for irq number 0:
> [10206.317270] Trying to free already-free IRQ 0
>
> With this stack:
> [10206.317344] NIP [c00000000018cbf8] __free_irq+0x308/0x370
> [10206.317346] LR [c00000000018cbf4] __free_irq+0x304/0x370
> [10206.317347] Call Trace:
> [10206.317348] [c00000008b92b970] [c00000000018cbf4] __free_irq
> +0x304/0x370 (unreliable)
> [10206.317351] [c00000008b92ba00] [c00000000018cd84] free_irq+0x84/0xf0
> [10206.317358] [c00000008b92ba30] [d000000007449e60] e1000_free_irq
> +0x98/0xc0 [e1000e]
> [10206.317365] [c00000008b92ba60] [d000000007458a70] e1000e_pm_freeze
> +0xb8/0x100 [e1000e]
> [10206.317372] [c00000008b92baa0] [d000000007458b6c]
> e1000_io_error_detected+0x34/0x70 [e1000e]
> [10206.317375] [c00000008b92bad0] [c000000000040358] eeh_report_failure
> +0xc8/0x190
> [10206.317377] [c00000008b92bb20] [c00000000003eb2c] eeh_pe_dev_traverse
> +0x9c/0x170
> [10206.317379] [c00000008b92bbb0] [c000000000040d84]
> eeh_handle_normal_event+0xe4/0x580
> [10206.317382] [c00000008b92bc60] [c000000000041330] eeh_handle_event
> +0x30/0x340
> [10206.317384] [c00000008b92bd10] [c000000000041780] eeh_event_handler
> +0x140/0x200
> [10206.317386] [c00000008b92bdc0] [c0000000001397c8] kthread+0x1a8/0x1b0
> [10206.317389] [c00000008b92be30] [c00000000000b560]
> ret_from_kernel_thread+0x5c/0x7c
>
> Thanks! - David

Hmm. I wonder if it is possibly calling the report
e1000_io_error_detected multiple times. If so then the secondary calls
to e1000_pm_freeze would cause issues.

I will add a check so that we only down the interface and free the
IRQs if the interface is in the present and running state.

I'll submit an update patch shortly.

Thanks.

- Alex

On Mon, 2019-10-07 at 10:02 -0700, Alexander Duyck wrote:
> On Mon, Oct 7, 2019 at 8:51 AM David Z. Dai <zdai@linux.vnet.ibm.com> wrote:
> >
> 
> <snip>
> 
> > We have tested on one of the test box.
> > With this patch, it doesn't crash kernel anymore, which is good!
> >
> > However we see this warning message from the log file for irq number 0:
> > [10206.317270] Trying to free already-free IRQ 0
> >
> > With this stack:
> > [10206.317344] NIP [c00000000018cbf8] __free_irq+0x308/0x370
> > [10206.317346] LR [c00000000018cbf4] __free_irq+0x304/0x370
> > [10206.317347] Call Trace:
> > [10206.317348] [c00000008b92b970] [c00000000018cbf4] __free_irq
> > +0x304/0x370 (unreliable)
> > [10206.317351] [c00000008b92ba00] [c00000000018cd84] free_irq+0x84/0xf0
> > [10206.317358] [c00000008b92ba30] [d000000007449e60] e1000_free_irq
> > +0x98/0xc0 [e1000e]
> > [10206.317365] [c00000008b92ba60] [d000000007458a70] e1000e_pm_freeze
> > +0xb8/0x100 [e1000e]
> > [10206.317372] [c00000008b92baa0] [d000000007458b6c]
> > e1000_io_error_detected+0x34/0x70 [e1000e]
> > [10206.317375] [c00000008b92bad0] [c000000000040358] eeh_report_failure
> > +0xc8/0x190
> > [10206.317377] [c00000008b92bb20] [c00000000003eb2c] eeh_pe_dev_traverse
> > +0x9c/0x170
> > [10206.317379] [c00000008b92bbb0] [c000000000040d84]
> > eeh_handle_normal_event+0xe4/0x580
> > [10206.317382] [c00000008b92bc60] [c000000000041330] eeh_handle_event
> > +0x30/0x340
> > [10206.317384] [c00000008b92bd10] [c000000000041780] eeh_event_handler
> > +0x140/0x200
> > [10206.317386] [c00000008b92bdc0] [c0000000001397c8] kthread+0x1a8/0x1b0
> > [10206.317389] [c00000008b92be30] [c00000000000b560]
> > ret_from_kernel_thread+0x5c/0x7c
> >
> > Thanks! - David
> 
> Hmm. I wonder if it is possibly calling the report
> e1000_io_error_detected multiple times. If so then the secondary calls
> to e1000_pm_freeze would cause issues.
> 
> I will add a check so that we only down the interface and free the
> IRQs if the interface is in the present and running state.
> 
> I'll submit an update patch shortly.
> 
> Thanks.
> 
> - Alex
It only complains about IRQ number 0 in the log.
Could you please let me know the actual place where you will add the
check?
I can retest it again.

Thanks! - David

On Mon, Oct 7, 2019 at 10:12 AM David Z. Dai <zdai@linux.vnet.ibm.com> wrote:
>
> On Mon, 2019-10-07 at 10:02 -0700, Alexander Duyck wrote:
> > On Mon, Oct 7, 2019 at 8:51 AM David Z. Dai <zdai@linux.vnet.ibm.com> wrote:
> > >
> >
> > <snip>
> >
> > > We have tested on one of the test box.
> > > With this patch, it doesn't crash kernel anymore, which is good!
> > >
> > > However we see this warning message from the log file for irq number 0:
> > > [10206.317270] Trying to free already-free IRQ 0
> > >
> > > With this stack:
> > > [10206.317344] NIP [c00000000018cbf8] __free_irq+0x308/0x370
> > > [10206.317346] LR [c00000000018cbf4] __free_irq+0x304/0x370
> > > [10206.317347] Call Trace:
> > > [10206.317348] [c00000008b92b970] [c00000000018cbf4] __free_irq
> > > +0x304/0x370 (unreliable)
> > > [10206.317351] [c00000008b92ba00] [c00000000018cd84] free_irq+0x84/0xf0
> > > [10206.317358] [c00000008b92ba30] [d000000007449e60] e1000_free_irq
> > > +0x98/0xc0 [e1000e]
> > > [10206.317365] [c00000008b92ba60] [d000000007458a70] e1000e_pm_freeze
> > > +0xb8/0x100 [e1000e]
> > > [10206.317372] [c00000008b92baa0] [d000000007458b6c]
> > > e1000_io_error_detected+0x34/0x70 [e1000e]
> > > [10206.317375] [c00000008b92bad0] [c000000000040358] eeh_report_failure
> > > +0xc8/0x190
> > > [10206.317377] [c00000008b92bb20] [c00000000003eb2c] eeh_pe_dev_traverse
> > > +0x9c/0x170
> > > [10206.317379] [c00000008b92bbb0] [c000000000040d84]
> > > eeh_handle_normal_event+0xe4/0x580
> > > [10206.317382] [c00000008b92bc60] [c000000000041330] eeh_handle_event
> > > +0x30/0x340
> > > [10206.317384] [c00000008b92bd10] [c000000000041780] eeh_event_handler
> > > +0x140/0x200
> > > [10206.317386] [c00000008b92bdc0] [c0000000001397c8] kthread+0x1a8/0x1b0
> > > [10206.317389] [c00000008b92be30] [c00000000000b560]
> > > ret_from_kernel_thread+0x5c/0x7c
> > >
> > > Thanks! - David
> >
> > Hmm. I wonder if it is possibly calling the report
> > e1000_io_error_detected multiple times. If so then the secondary calls
> > to e1000_pm_freeze would cause issues.
> >
> > I will add a check so that we only down the interface and free the
> > IRQs if the interface is in the present and running state.
> >
> > I'll submit an update patch shortly.
> >
> > Thanks.
> >
> > - Alex
> It only complains about IRQ number 0 in the log.

I suspect that is because the IRQ is already freed. So there are no
other interrupts enabled and it thinks it is running in legacy IRQ
mode.

> Could you please let me know the actual place where you will add the
> check?
> I can retest it again.
>
> Thanks! - David

So I will need to add another variable called "present" to
e1000_pm_freeze, I will assign netif_device_present() to it after
taking the rtnl_lock and before I detach the interface, and will test
for it and netif_running() before calling the logic that calls
e1000_down and e1000_free_irq.

- Alex