Skip to contentSkip to conversation

ixgbe: Fix free irq process when removing device due to PCI Errors]

PATCH New Guilherme G. Piccoli 3 participants 3 messages Apr 18, 2018 – Apr 26, 2018 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 20180418175613.GA2121@korriban

Guilherme G. Piccoli

author
#1 lore raw

PATCH 1/1 ixgbe: Fix free irq process when removing device due to PCI Errors]

Thanks Mauro, nice fix! A minor suggestion below.
Other than that, feel free to add:

Reviewed-by Guilherme G. Piccoli <[email protected]>

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c int ixgbe_close(struct net_device *netdev) · line 6683

Quoted 13 lines

[...]

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +2 -1
index afadba9..d170de8 100644
@@ -6679,7 +6679,8 @@ int ixgbe_close(struct net_device *netdev)
 
  	ixgbe_ptp_stop(adapter);
 
-	if (netif_device_present(netdev))
+	if (netif_device_present(netdev) ||
+	    adapter->pdev->error_state == pci_channel_io_perm_failure)

How about using pci_channel_offline() here instead of manually checking the error_state flag? It's a minor though heheh

Cheers,

Guilherme

Quoted 4 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +0 -0
  		ixgbe_close_suspend(adapter);
 
  	ixgbe_fdir_filter_exit(adapter);
#2 lore raw

PATCH 1/1 ixgbe: Fix free irq process when removing device due to PCI Errors]

Hi Guilherme, thanks for your review, answer to your suggestion is inline.

Quoted 13 lines · Guilherme G. Piccoli

Thanks Mauro, nice fix! A minor suggestion below.
Other than that, feel free to add:

Reviewed-by Guilherme G. Piccoli <[email protected]>
Quoted 8 lines

[...]

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +0 -0
index afadba9..d170de8 100644
@@ -6679,7 +6679,8 @@ int ixgbe_close(struct net_device *netdev)
Quoted 8 lines

drivers/net/ethernet/intel/ixgbe/ixgbe_main.c int ixgbe_close(struct net_device *netdev) · line 6683

Quoted 5 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +2 -1
 	ixgbe_ptp_stop(adapter);
 
-	if (netif_device_present(netdev))
+	if (netif_device_present(netdev) ||
+	    adapter->pdev->error_state == pci_channel_io_perm_failure)

How about using pci_channel_offline() here instead of manually checking the error_state flag? It's a minor though heheh

So the thing here is that I want to proceed through out this path only when the device is in pci_channle_io_perm_failure. Looking at the function you suggested there is a third state that would return True here which is pci_channel_io_frozen, it's also a concern that this list of possible values can increase eventually as well, so I would rather keep this as is.

I've just realized that I need to fix this in ixgbevf as well, so I'll send a second version of this one soon.

Thanks,

Mauro

Quoted 11 lines

Cheers,

Guilherme

Quoted 4 lines
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +0 -0
 		ixgbe_close_suspend(adapter);
 
 	ixgbe_fdir_filter_exit(adapter);