[Intel-wired-lan] ixgbevf: fix atomicity off IXGBEVF_FLAG_QUEUE_RESET_REQUESTED flag

Alexander Duyck alexander.duyck at gmail.com
Wed Dec 2 21:32:29 UTC 2015


On 12/02/2015 12:25 PM, John Greene wrote:
> When doing a 'reboot' of a VM, the "ip link set ethX down" of an interface
> using the ixgbevf.ko driver may hang indefinitely blocking the complete
> boot down of the OS.    The "ip" command is executed from an rc6.d script.
> The driver in performing the ioctl() was stuck in ixgbevf_down() looping
> at this line forever:
>
> while (adapter->flags & IXGBE_FLAG_IN_WATCHDOG_TASK)
>                  msleep(1);
>
> While this issue has been fixed already by other upstream patches, it's noted
> that IXGBEVF_FLAG_QUEUE_RESET_REQUESTED flag should likewise be changed.
>
> Signed-off-by: Scott Otto <otts62 at yahoo.com>
> Signed-off-by: John Greene <jogreene at redhat.com>
> ---
>   drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> index 592ff23..826b04f 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
> @@ -1983,7 +1983,7 @@ static int ixgbevf_configure_dcb(struct ixgbevf_adapter *adapter)
>   		hw->mbx.timeout = 0;
>   
>   		/* wait for watchdog to come around and bail us out */
> -		adapter->flags |= IXGBEVF_FLAG_QUEUE_RESET_REQUESTED;
> +		set_bit(IXGBEVF_FLAG_QUEUE_RESET_REQUESTED, &adapter->flags);
>   	}
>   
>   	return 0;
> @@ -3230,10 +3230,10 @@ static void ixgbevf_queue_reset_subtask(struct ixgbevf_adapter *adapter)
>   {
>   	struct net_device *dev = adapter->netdev;
>   
> -	if (!(adapter->flags & IXGBEVF_FLAG_QUEUE_RESET_REQUESTED))
> +	if (!(test_bit(IXGBEVF_FLAG_QUEUE_RESET_REQUESTED, &adapter->flags)))
>   		return;
>   
> -	adapter->flags &= ~IXGBEVF_FLAG_QUEUE_RESET_REQUESTED;
> +	clear_bit(IXGBEVF_FLAG_QUEUE_RESET_REQUESTED, &adapter->flags);
>   
>   	/* if interface is down do nothing */
>   	if (test_bit(__IXGBEVF_DOWN, &adapter->state) ||

Actually looking through this I realized this should also be throwing 
type errors since state is a u32 and last I knew the test/set/clear_bit 
calls expect to be pointing at unsigned longs.

My advice is if you want to go forward with this patch you might look at 
just removing adapter->flags entirely and look at moving RESET_REQUESTED 
and QUEUE_RESET_REQUESTED over into adapter->state as a couple more bits.

- Alex


More information about the Intel-wired-lan mailing list