From: Hayes Wang <hidden> Date: 2017-01-25 02:51:24
Scheduling the napi during the following periods would let it be ignored.
And the events wouldn't be handled until next napi_schedule() is called.
1. after napi_disable and before napi_enable().
2. after all actions of napi function is completed and before calling
napi_complete().
If no next napi_schedule() is called, tx or rx would stop working.
In order to avoid these situations, the followings solutions are applied.
1. prevent start_xmit() from calling napi_schedule() during runtime suspend
or after napi_disable().
2. re-schedule the napi for tx if it is necessary.
3. check if any rx is finished or not after napi_enable().
Hayes Wang (4):
r8152: avoid start_xmit to call napi_schedule during autosuspend
r8152: avoid start_xmit to schedule napi when napi is disabled
r8152: re-schedule napi for tx
r8152: check rx after napi is enabled
drivers/net/usb/r8152.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
--
2.7.4
From: Hayes Wang <hidden> Date: 2017-01-25 02:51:27
Adjust the setting of the flag of SELECTIVE_SUSPEND to prevent start_xmit()
from calling napi_schedule() directly during runtime suspend.
After calling napi_disable() or clearing the flag of WORK_ENABLE,
scheduling the napi is useless.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
From: Hayes Wang <hidden> Date: 2017-01-25 02:51:41
Schedule the napi after napi_enable() for rx, if it is necessary.
If the rx is completed when napi is disabled, the sheduling of napi
would be lost. Then, no one handles the rx packet until next napi
is scheduled.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
@@ -32,7 +32,7 @@#define NETNEXT_VERSION "08"/* Information for net */-#define NET_VERSION "7"+#define NET_VERSION "8"#define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION#define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_swsd@realtek.com>"
@@ -3561,6 +3561,9 @@ static int rtl8152_post_reset(struct usb_interface *intf)netif_wake_queue(netdev);usb_submit_urb(tp->intr_urb,GFP_KERNEL);+if(!list_empty(&tp->rx_done))+napi_schedule(&tp->napi);+return0;}
@@ -3696,6 +3699,8 @@ static int rtl8152_resume(struct usb_interface *intf)rtl_start_rx(tp);napi_enable(&tp->napi);clear_bit(SELECTIVE_SUSPEND,&tp->flags);+if(!list_empty(&tp->rx_done))+napi_schedule(&tp->napi);}else{tp->rtl_ops.up(tp);netif_carrier_off(tp->netdev);
From: Hayes Wang <hidden> Date: 2017-01-25 02:51:49
Stop the tx when the napi is disabled to prevent napi_schedule() is
called.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
@@ -3515,12 +3519,12 @@ static int rtl8152_pre_reset(struct usb_interface *intf)if(!netif_running(netdev))return0;+netif_stop_queue(netdev);napi_disable(&tp->napi);clear_bit(WORK_ENABLE,&tp->flags);usb_kill_urb(tp->intr_urb);cancel_delayed_work_sync(&tp->schedule);if(netif_carrier_ok(netdev)){-netif_stop_queue(netdev);mutex_lock(&tp->control);tp->rtl_ops.disable(tp);mutex_unlock(&tp->control);
@@ -3548,10 +3552,10 @@ static int rtl8152_post_reset(struct usb_interface *intf)rtl_start_rx(tp);rtl8152_set_rx_mode(netdev);mutex_unlock(&tp->control);-netif_wake_queue(netdev);}napi_enable(&tp->napi);+netif_wake_queue(netdev);usb_submit_urb(tp->intr_urb,GFP_KERNEL);return0;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Hayes Wang <hidden> Date: 2017-01-25 02:52:10
Re-schedule napi after napi_complete() for tx, if it is necessay.
In r8152_poll(), if the tx is completed after tx_bottom() and before
napi_complete(), the scheduling of napi would be lost. Then, no
one handles the next tx until the next napi_schedule() is called.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 3 +++
1 file changed, 3 insertions(+)
From: Stephen Hemminger <stephen@networkplumber.org> Date: 2017-01-25 05:02:56
On Wed, 25 Jan 2017 10:50:51 +0800
Hayes Wang [off-list ref] wrote:
quoted hunk
Adjust the setting of the flag of SELECTIVE_SUSPEND to prevent start_xmit()
from calling napi_schedule() directly during runtime suspend.
After calling napi_disable() or clearing the flag of WORK_ENABLE,
scheduling the napi is useless.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
From: Hayes Wang <hidden> Date: 2017-01-25 08:13:39
v2:
Add smp_mb__after_atomic() for patch #1.
v1:
Scheduling the napi during the following periods would let it be ignored.
And the events wouldn't be handled until next napi_schedule() is called.
1. after napi_disable and before napi_enable().
2. after all actions of napi function is completed and before calling
napi_complete().
If no next napi_schedule() is called, tx or rx would stop working.
In order to avoid these situations, the followings solutions are applied.
1. prevent start_xmit() from calling napi_schedule() during runtime suspend
or after napi_disable().
2. re-schedule the napi for tx if it is necessary.
3. check if any rx is finished or not after napi_enable().
Hayes Wang (4):
r8152: avoid start_xmit to call napi_schedule during autosuspend
r8152: avoid start_xmit to schedule napi when napi is disabled
r8152: re-schedule napi for tx
r8152: check rx after napi is enabled
drivers/net/usb/r8152.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
--
2.7.4
From: Hayes Wang <hidden> Date: 2017-01-25 08:13:46
Stop the tx when the napi is disabled to prevent napi_schedule() is
called.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
From: Hayes Wang <hidden> Date: 2017-01-25 08:13:52
Adjust the setting of the flag of SELECTIVE_SUSPEND to prevent start_xmit()
from calling napi_schedule() directly during runtime suspend.
After calling napi_disable() or clearing the flag of WORK_ENABLE,
scheduling the napi is useless.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
From: Hayes Wang <hidden> Date: 2017-01-25 08:13:55
Re-schedule napi after napi_complete() for tx, if it is necessay.
In r8152_poll(), if the tx is completed after tx_bottom() and before
napi_complete(), the scheduling of napi would be lost. Then, no
one handles the next tx until the next napi_schedule() is called.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 3 +++
1 file changed, 3 insertions(+)
@@ -1936,6 +1936,9 @@ static int r8152_poll(struct napi_struct *napi, int budget)napi_complete(napi);if(!list_empty(&tp->rx_done))napi_schedule(napi);+elseif(!skb_queue_empty(&tp->tx_queue)&&+!list_empty(&tp->tx_free))+napi_schedule(&tp->napi);}returnwork_done;
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Hayes Wang <hidden> Date: 2017-01-25 08:13:57
Schedule the napi after napi_enable() for rx, if it is necessary.
If the rx is completed when napi is disabled, the sheduling of napi
would be lost. Then, no one handles the rx packet until next napi
is scheduled.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
@@ -32,7 +32,7 @@#define NETNEXT_VERSION "08"/* Information for net */-#define NET_VERSION "7"+#define NET_VERSION "8"#define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION#define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_swsd@realtek.com>"
@@ -3561,6 +3561,9 @@ static int rtl8152_post_reset(struct usb_interface *intf)netif_wake_queue(netdev);usb_submit_urb(tp->intr_urb,GFP_KERNEL);+if(!list_empty(&tp->rx_done))+napi_schedule(&tp->napi);+return0;}
@@ -3700,6 +3703,8 @@ static int rtl8152_resume(struct usb_interface *intf)napi_enable(&tp->napi);clear_bit(SELECTIVE_SUSPEND,&tp->flags);smp_mb__after_atomic();+if(!list_empty(&tp->rx_done))+napi_schedule(&tp->napi);}else{tp->rtl_ops.up(tp);netif_carrier_off(tp->netdev);
From: Eric Dumazet <hidden> Date: 2017-01-25 13:57:22
On Wed, 2017-01-25 at 16:13 +0800, Hayes Wang wrote:
quoted hunk
Re-schedule napi after napi_complete() for tx, if it is necessay.
In r8152_poll(), if the tx is completed after tx_bottom() and before
napi_complete(), the scheduling of napi would be lost. Then, no
one handles the next tx until the next napi_schedule() is called.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 3 +++
1 file changed, 3 insertions(+)
From: David Miller <davem@davemloft.net> Date: 2017-01-25 19:31:27
From: Hayes Wang <redacted>
Date: Wed, 25 Jan 2017 16:13:17 +0800
v2:
Add smp_mb__after_atomic() for patch #1.
v1:
Scheduling the napi during the following periods would let it be ignored.
And the events wouldn't be handled until next napi_schedule() is called.
1. after napi_disable and before napi_enable().
2. after all actions of napi function is completed and before calling
napi_complete().
If no next napi_schedule() is called, tx or rx would stop working.
In order to avoid these situations, the followings solutions are applied.
1. prevent start_xmit() from calling napi_schedule() during runtime suspend
or after napi_disable().
2. re-schedule the napi for tx if it is necessary.
3. check if any rx is finished or not after napi_enable().
I think the fundamental issue is that since you can't stop URBs from
queueing up, you cannot properly synchronize NAPI and schedule polling
properly.
From my perspective what happened here is you want GRO support, but it
comes at the expense of this extremely racey NAPI support which does
not at all achieve one of the main advantages of NAPI which is
interrupt mitigation.
It would have been so much better to implement a generic way for
drivers to get GRO support without NAPI, especially if their packet
feeding engine works the way that the USB networking drivers's do.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Hayes Wang <hidden> Date: 2017-01-26 01:38:56
Adjust the setting of the flag of SELECTIVE_SUSPEND to prevent start_xmit()
from calling napi_schedule() directly during runtime suspend.
After calling napi_disable() or clearing the flag of WORK_ENABLE,
scheduling the napi is useless.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
From: Hayes Wang <hidden> Date: 2017-01-26 01:39:05
Stop the tx when the napi is disabled to prevent napi_schedule() is
called.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
From: Hayes Wang <hidden> Date: 2017-01-26 01:39:09
Re-schedule napi after napi_complete() for tx, if it is necessay.
In r8152_poll(), if the tx is completed after tx_bottom() and before
napi_complete(), the scheduling of napi would be lost. Then, no
one handles the next tx until the next napi_schedule() is called.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 3 +++
1 file changed, 3 insertions(+)
From: Hayes Wang <hidden> Date: 2017-01-26 01:39:18
Schedule the napi after napi_enable() for rx, if it is necessary.
If the rx is completed when napi is disabled, the sheduling of napi
would be lost. Then, no one handles the rx packet until next napi
is scheduled.
Signed-off-by: Hayes Wang <redacted>
---
drivers/net/usb/r8152.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
@@ -32,7 +32,7 @@#define NETNEXT_VERSION "08"/* Information for net */-#define NET_VERSION "7"+#define NET_VERSION "8"#define DRIVER_VERSION "v1." NETNEXT_VERSION "." NET_VERSION#define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_swsd@realtek.com>"
@@ -3561,6 +3561,9 @@ static int rtl8152_post_reset(struct usb_interface *intf)netif_wake_queue(netdev);usb_submit_urb(tp->intr_urb,GFP_KERNEL);+if(!list_empty(&tp->rx_done))+napi_schedule(&tp->napi);+return0;}
@@ -3700,6 +3703,8 @@ static int rtl8152_resume(struct usb_interface *intf)napi_enable(&tp->napi);clear_bit(SELECTIVE_SUSPEND,&tp->flags);smp_mb__after_atomic();+if(!list_empty(&tp->rx_done))+napi_schedule(&tp->napi);}else{tp->rtl_ops.up(tp);netif_carrier_off(tp->netdev);
From: Hayes Wang <hidden> Date: 2017-01-26 01:40:00
v3:
simply the argument for patch #3. Replace &tp->napi with napi.
v2:
Add smp_mb__after_atomic() for patch #1.
v1:
Scheduling the napi during the following periods would let it be ignored.
And the events wouldn't be handled until next napi_schedule() is called.
1. after napi_disable and before napi_enable().
2. after all actions of napi function is completed and before calling
napi_complete().
If no next napi_schedule() is called, tx or rx would stop working.
In order to avoid these situations, the followings solutions are applied.
1. prevent start_xmit() from calling napi_schedule() during runtime suspend
or after napi_disable().
2. re-schedule the napi for tx if it is necessary.
3. check if any rx is finished or not after napi_enable().
Hayes Wang (4):
r8152: avoid start_xmit to call napi_schedule during autosuspend
r8152: avoid start_xmit to schedule napi when napi is disabled
r8152: re-schedule napi for tx
r8152: check rx after napi is enabled
drivers/net/usb/r8152.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
--
2.7.4
From: Hayes Wang <hidden> Date: 2017-01-26 03:04:58
David Miller [mailto:davem@davemloft.net]
Sent: Thursday, January 26, 2017 3:31 AM
[...]
I think the fundamental issue is that since you can't stop URBs from
queueing up, you cannot properly synchronize NAPI and schedule polling
properly.
From my perspective what happened here is you want GRO support, but it
comes at the expense of this extremely racey NAPI support which does
not at all achieve one of the main advantages of NAPI which is
interrupt mitigation.
May you apply these patches first, until I find another way to replace
current one? The driver uses NAPI now and I have no idea to find better
way to replace current one. I think it would take me long time to find
out the solution. And the issue is still there until I finish this work.
If now I give up the NAPI and its advantages except for the interrupt
mitigation, some things would become worse.
Our hw supports packet aggregation. The purpose is interrupt mitigation,
too. I wouldn't say it is better than what the NAPI does. However, I
could say we try to improve it. If the interrupt could be disabled, I
would be happy to do it. However, it is the limitation of USB devices.
That is why I still use NAPI even though the interrupt cannot be disabled
for USB devices. Because one of the advantages of NAPI couldn't be
satisfied, I must not use the NAPI. Doesn't it seem too strict?
Best Regards,
Hayes
From: David Miller <davem@davemloft.net> Date: 2017-01-26 03:47:52
From: Hayes Wang <redacted>
Date: Thu, 26 Jan 2017 09:38:30 +0800
v3:
simply the argument for patch #3. Replace &tp->napi with napi.
v2:
Add smp_mb__after_atomic() for patch #1.
v1:
Scheduling the napi during the following periods would let it be ignored.
And the events wouldn't be handled until next napi_schedule() is called.
1. after napi_disable and before napi_enable().
2. after all actions of napi function is completed and before calling
napi_complete().
If no next napi_schedule() is called, tx or rx would stop working.
In order to avoid these situations, the followings solutions are applied.
1. prevent start_xmit() from calling napi_schedule() during runtime suspend
or after napi_disable().
2. re-schedule the napi for tx if it is necessary.
3. check if any rx is finished or not after napi_enable().