From: Shahed Shaikh <shahed.shaikh@qlogic.com>
Date: Tue, 7 May 2013 15:55:03 -0400
> @@ -435,10 +435,7 @@ static void qlcnic_83xx_idc_attach_driver(struct qlcnic_adapter *adapter)> }> done:> netif_device_attach(netdev);> - if (netif_running(netdev)) {> - netif_carrier_on(netdev);> - netif_wake_queue(netdev);> - }> + adapter->netdev->trans_start = jiffies;> }> > static int qlcnic_83xx_idc_enter_failed_state(struct qlcnic_adapter *adapter,
This is not right.
Multiqueue aware drivers should never access netdev->trans_start
directly, and I see several such writes in this driver.
Anything you write here will be totally ignored by the rest of the
kernel, because this value is overwritten by every call to dev_trans_start()
which is the only valid method by which to determine this value.
dev_trans_start() walks all of the transmit queues, recording the most
recent txq->trans_start timestamp in netdev->trans_start.
So if you write netdev->trans_start in your driver, it does nothing,
because even the dev_watchdog() time is going to inspect the per-queue
txq->trans_start values and ignore the netdev->trans_start value for
devices whose driver suppports multiqueue as qlcnic does.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

> -----Original Message-----> From: David Miller [mailto:davem@davemloft.net]> Sent: Thursday, May 09, 2013 12:37 AM> To: Shahed Shaikh> Cc: netdev; Dept-NX Linux NIC Driver; Sony Chacko> Subject: Re: [PATCH net 5/8] qlcnic: Fix reset recovery after transmit timeout> > From: Shahed Shaikh <shahed.shaikh@qlogic.com>> Date: Tue, 7 May 2013 15:55:03 -0400> > > @@ -435,10 +435,7 @@ static void qlcnic_83xx_idc_attach_driver(struct> qlcnic_adapter *adapter)> > }> > done:> > netif_device_attach(netdev);> > - if (netif_running(netdev)) {> > - netif_carrier_on(netdev);> > - netif_wake_queue(netdev);> > - }> > + adapter->netdev->trans_start = jiffies;> > }> >> > static int qlcnic_83xx_idc_enter_failed_state(struct qlcnic_adapter> > *adapter,> > This is not right.> > Multiqueue aware drivers should never access netdev->trans_start directly,> and I see several such writes in this driver.> > Anything you write here will be totally ignored by the rest of the kernel,> because this value is overwritten by every call to dev_trans_start() which is> the only valid method by which to determine this value.> > dev_trans_start() walks all of the transmit queues, recording the most recent> txq->trans_start timestamp in netdev->trans_start.> > So if you write netdev->trans_start in your driver, it does nothing, because> even the dev_watchdog() time is going to inspect the per-queue> txq->trans_start values and ignore the netdev->trans_start value for> devices whose driver suppports multiqueue as qlcnic does.
Hi David,
We will resubmit the current patch after removing access to netdev->trans_start.
We will fix the same issue in the rest of the driver code in a follow-up patch.
Thanks,
Shahed
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html