After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
MII_BMSR may reports 10Mbps, renders the network rather slow.
The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
Make watchdog use delayed work"), which esssentially introduces some
delay before running the watchdog task.
But there's still a chance that the hotplugging event and the queued
watchdog task gets run at the same time, then the original issue can be
observed once again.
So let's use mod_delayed_work() to add a deterministic 1 second delay
before running watchdog task, after an interrupt.
Signed-off-by: Kai-Heng Feng <[email protected]>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e4baa13b3cda..c83bf5349d53 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1780,8 +1780,8 @@ static irqreturn_t e1000_intr_msi(int __always_unused irq, void *data)
}
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
- queue_delayed_work(adapter->e1000_workqueue,
- &adapter->watchdog_task, 1);
+ mod_delayed_work(adapter->e1000_workqueue,
+ &adapter->watchdog_task, HZ);
}
/* Reset on uncorrectable ECC error */
@@ -1861,8 +1861,8 @@ static irqreturn_t e1000_intr(int __always_unused irq, void *data)
}
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
- queue_delayed_work(adapter->e1000_workqueue,
- &adapter->watchdog_task, 1);
+ mod_delayed_work(adapter->e1000_workqueue,
+ &adapter->watchdog_task, HZ);
}
/* Reset on uncorrectable ECC error */
@@ -1907,8 +1907,8 @@ static irqreturn_t e1000_msix_other(int __always_unused irq, void *data)
hw->mac.get_link_status = true;
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
- queue_delayed_work(adapter->e1000_workqueue,
- &adapter->watchdog_task, 1);
+ mod_delayed_work(adapter->e1000_workqueue,
+ &adapter->watchdog_task, HZ);
}
if (!test_bit(__E1000_DOWN, &adapter->state))
--
2.17.1
Dear Kai-Heng,
Thank you for the patch.
On 7/15/19 10:43 AM, Kai-Heng Feng wrote:
> After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
> MII_BMSR may reports 10Mbps, renders the network rather slow.
s/may reports/may report/
s/renders/rendering/
> The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
> Make watchdog use delayed work"), which esssentially introduces some
essentially
> delay before running the watchdog task.
>
> But there's still a chance that the hotplugging event and the queued
> watchdog task gets run at the same time, then the original issue can be
> observed once again.
>
> So let's use mod_delayed_work() to add a deterministic 1 second delay
> before running watchdog task, after an interrupt.
I am not clear about the effects for the user. Could you elaborate
please? Does the link now come up up to one second later?
> Signed-off-by: Kai-Heng Feng <[email protected]>
Any bug URL?
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Kind regards,
Paul
at 4:52 PM, Paul Menzel <[email protected]> wrote:
> Dear Kai-Heng,
>
>
> Thank you for the patch.
>
> On 7/15/19 10:43 AM, Kai-Heng Feng wrote:
>> After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
>> MII_BMSR may reports 10Mbps, renders the network rather slow.
>
> s/may reports/may report/
> s/renders/rendering/
Apparently English isn’t my mother tongue ;)
>
>> The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
>> Make watchdog use delayed work"), which esssentially introduces some
>
> essentially
Ok.
>
>> delay before running the watchdog task.
>>
>> But there's still a chance that the hotplugging event and the queued
>> watchdog task gets run at the same time, then the original issue can be
>> observed once again.
>>
>> So let's use mod_delayed_work() to add a deterministic 1 second delay
>> before running watchdog task, after an interrupt.
>
> I am not clear about the effects for the user. Could you elaborate
> please? Does the link now come up up to one second later?
Yes, the link will be up on a fixed one second later.
The delay varies between 0 to 2 seconds without this patch.
>
>> Signed-off-by: Kai-Heng Feng <[email protected]>
>
> Any bug URL?
If maintainers think it’s necessary then I’ll file one.
Kai-Heng
>
>> ---
>> drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>
> Kind regards,
>
> Paul
Dear Kai Heng,
(with or without hyphen?)
On 7/15/19 11:00 AM, Kai Heng Feng wrote:
> at 4:52 PM, Paul Menzel <[email protected]> wrote:
>> On 7/15/19 10:43 AM, Kai-Heng Feng wrote:
>>> After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
>>> MII_BMSR may reports 10Mbps, renders the network rather slow.
>>
>> s/may reports/may report/
>> s/renders/rendering/
>
> Apparently English isn’t my mother tongue ;)
No problem. Mine neither.
>>> The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
>>> Make watchdog use delayed work"), which esssentially introduces some
>>
>> essentially
>
> Ok.
>
>>> delay before running the watchdog task.
>>>
>>> But there's still a chance that the hotplugging event and the queued
>>> watchdog task gets run at the same time, then the original issue can be
>>> observed once again.
>>>
>>> So let's use mod_delayed_work() to add a deterministic 1 second delay
>>> before running watchdog task, after an interrupt.
>>
>> I am not clear about the effects for the user. Could you elaborate
>> please? Does the link now come up up to one second later?
>
> Yes, the link will be up on a fixed one second later.
>
> The delay varies between 0 to 2 seconds without this patch.
Is there no other fix? Regarding booting a system fast (less than six
seconds), a fixed one second delay is quite a regression on systems where
it worked before.
>>> Signed-off-by: Kai-Heng Feng <[email protected]>
>>
>> Any bug URL?
>
> If maintainers think it’s necessary then I’ll file one.
Not necessary, if there is none. I thought you had one in Launchpad or so.
Kind regards,
Paul
at 5:06 PM, Paul Menzel <[email protected]> wrote:
> Dear Kai Heng,
>
>
> (with or without hyphen?)
>
> On 7/15/19 11:00 AM, Kai Heng Feng wrote:
>> at 4:52 PM, Paul Menzel <[email protected]> wrote:
>
>>> On 7/15/19 10:43 AM, Kai-Heng Feng wrote:
>>>> After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
>>>> MII_BMSR may reports 10Mbps, renders the network rather slow.
>>>
>>> s/may reports/may report/
>>> s/renders/rendering/
>>
>> Apparently English isn’t my mother tongue ;)
>
> No problem. Mine neither.
>
>>>> The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
>>>> Make watchdog use delayed work"), which esssentially introduces some
>>>
>>> essentially
>>
>> Ok.
>>
>>>> delay before running the watchdog task.
>>>>
>>>> But there's still a chance that the hotplugging event and the queued
>>>> watchdog task gets run at the same time, then the original issue can be
>>>> observed once again.
>>>>
>>>> So let's use mod_delayed_work() to add a deterministic 1 second delay
>>>> before running watchdog task, after an interrupt.
>>>
>>> I am not clear about the effects for the user. Could you elaborate
>>> please? Does the link now come up up to one second later?
>>
>> Yes, the link will be up on a fixed one second later.
>>
>> The delay varies between 0 to 2 seconds without this patch.
>
> Is there no other fix? Regarding booting a system fast (less than six
> seconds), a fixed one second delay is quite a regression on systems where
> it worked before.
This only affects when ethernet cable is hot plugged.
Kai-Heng
>
>>>> Signed-off-by: Kai-Heng Feng <[email protected]>
>>>
>>> Any bug URL?
>>
>> If maintainers think it’s necessary then I’ll file one.
>
> Not necessary, if there is none. I thought you had one in Launchpad or so.
>
>
> Kind regards,
>
> Paul
After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
MII_BMSR may report 10Mbps, renders the network rather slow.
The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
Make watchdog use delayed work"), which essentially introduces some
delay before running the watchdog task.
But there's still a chance that the hotplugging event and the queued
watchdog task gets run at the same time, then the original issue can be
observed once again.
So let's use mod_delayed_work() to add a deterministic 1 second delay
before running watchdog task, after an interrupt.
Signed-off-by: Kai-Heng Feng <[email protected]>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e4baa13b3cda..c83bf5349d53 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -1780,8 +1780,8 @@ static irqreturn_t e1000_intr_msi(int __always_unused irq, void *data)
}
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
- queue_delayed_work(adapter->e1000_workqueue,
- &adapter->watchdog_task, 1);
+ mod_delayed_work(adapter->e1000_workqueue,
+ &adapter->watchdog_task, HZ);
}
/* Reset on uncorrectable ECC error */
@@ -1861,8 +1861,8 @@ static irqreturn_t e1000_intr(int __always_unused irq, void *data)
}
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
- queue_delayed_work(adapter->e1000_workqueue,
- &adapter->watchdog_task, 1);
+ mod_delayed_work(adapter->e1000_workqueue,
+ &adapter->watchdog_task, HZ);
}
/* Reset on uncorrectable ECC error */
@@ -1907,8 +1907,8 @@ static irqreturn_t e1000_msix_other(int __always_unused irq, void *data)
hw->mac.get_link_status = true;
/* guard against interrupt when we're going down */
if (!test_bit(__E1000_DOWN, &adapter->state))
- queue_delayed_work(adapter->e1000_workqueue,
- &adapter->watchdog_task, 1);
+ mod_delayed_work(adapter->e1000_workqueue,
+ &adapter->watchdog_task, HZ);
}
if (!test_bit(__E1000_DOWN, &adapter->state))
--
2.17.1
On Mon, 2019-07-15 at 20:25 +0800, Kai-Heng Feng wrote:
> After hotplugging an 1Gbps ethernet cable with 1Gbps link partner, the
> MII_BMSR may report 10Mbps, renders the network rather slow.
>
> The issue has much lower fail rate after commit 59653e6497d1 ("e1000e:
> Make watchdog use delayed work"), which essentially introduces some
> delay before running the watchdog task.
>
> But there's still a chance that the hotplugging event and the queued
> watchdog task gets run at the same time, then the original issue can be
> observed once again.
>
> So let's use mod_delayed_work() to add a deterministic 1 second delay
> before running watchdog task, after an interrupt.
>
> Signed-off-by: Kai-Heng Feng <[email protected]>
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
Tested-by: Aaron Brown <[email protected]>