* [PATCH net-next 0/2] r8152: adjust r8152_submit_rx @ 2014-12-19 8:55 Hayes Wang 2014-12-19 8:55 ` [PATCH net-next 1/2] r8152: adjust set_carrier Hayes Wang ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Hayes Wang @ 2014-12-19 8:55 UTC (permalink / raw) To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang Avoid r8152_submit_rx() from submitting rx during unexpected moment. This could reduce the time of stopping rx. For patch #1, the tp->speed should be updated early. Then, the patch #2 could use it to check the current linking status. Hayes Wang (2): r8152: adjust set_carrier r8152: check the status before submitting rx drivers/net/usb/r8152.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next 1/2] r8152: adjust set_carrier 2014-12-19 8:55 [PATCH net-next 0/2] r8152: adjust r8152_submit_rx Hayes Wang @ 2014-12-19 8:55 ` Hayes Wang 2014-12-19 8:56 ` [PATCH net-next 2/2] r8152: check the status before submitting rx Hayes Wang 2014-12-22 6:52 ` [PATCH net-next v2 0/2] r8152: adjust r8152_submit_rx Hayes Wang 2 siblings, 0 replies; 9+ messages in thread From: Hayes Wang @ 2014-12-19 8:55 UTC (permalink / raw) To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang Update the tp->speed at the beginning of the function. Then, the other fucntion could use it for checking linking status. Signed-off-by: Hayes Wang <hayeswang@realtek.com> --- drivers/net/usb/r8152.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 2d1c77e..59b70c5 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -2848,26 +2848,25 @@ static void rtl8153_down(struct r8152 *tp) static void set_carrier(struct r8152 *tp) { struct net_device *netdev = tp->netdev; - u8 speed; + u8 old_speed = tp->speed; clear_bit(RTL8152_LINK_CHG, &tp->flags); - speed = rtl8152_get_speed(tp); + tp->speed = rtl8152_get_speed(tp); - if (speed & LINK_STATUS) { - if (!(tp->speed & LINK_STATUS)) { + if (tp->speed & LINK_STATUS) { + if (!(old_speed & LINK_STATUS)) { tp->rtl_ops.enable(tp); set_bit(RTL8152_SET_RX_MODE, &tp->flags); netif_carrier_on(netdev); } } else { - if (tp->speed & LINK_STATUS) { + if (old_speed & LINK_STATUS) { netif_carrier_off(netdev); tasklet_disable(&tp->tl); tp->rtl_ops.disable(tp); tasklet_enable(&tp->tl); } } - tp->speed = speed; } static void rtl_work_func_t(struct work_struct *work) -- 2.1.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next 2/2] r8152: check the status before submitting rx 2014-12-19 8:55 [PATCH net-next 0/2] r8152: adjust r8152_submit_rx Hayes Wang 2014-12-19 8:55 ` [PATCH net-next 1/2] r8152: adjust set_carrier Hayes Wang @ 2014-12-19 8:56 ` Hayes Wang 2014-12-19 20:44 ` David Miller 2014-12-22 6:52 ` [PATCH net-next v2 0/2] r8152: adjust r8152_submit_rx Hayes Wang 2 siblings, 1 reply; 9+ messages in thread From: Hayes Wang @ 2014-12-19 8:56 UTC (permalink / raw) To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang Don't submit the rx if the device is unplugged, linking down, or stopped. Signed-off-by: Hayes Wang <hayeswang@realtek.com> --- drivers/net/usb/r8152.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 59b70c5..b39b2e4 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags) { int ret; + /* The rx would be stopped, so skip submitting */ + if (test_bit(RTL8152_UNPLUG, &tp->flags) || + !test_bit(WORK_ENABLE, &tp->flags) || !(tp->speed & LINK_STATUS)) + return 0; + usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1), agg->head, agg_buf_sz, (usb_complete_t)read_bulk_callback, agg); -- 2.1.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 2/2] r8152: check the status before submitting rx 2014-12-19 8:56 ` [PATCH net-next 2/2] r8152: check the status before submitting rx Hayes Wang @ 2014-12-19 20:44 ` David Miller 2014-12-22 2:53 ` [PATCH net-next 2/2] r8152: check the status before submittingrx Hayes Wang 0 siblings, 1 reply; 9+ messages in thread From: David Miller @ 2014-12-19 20:44 UTC (permalink / raw) To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb From: Hayes Wang <hayeswang@realtek.com> Date: Fri, 19 Dec 2014 16:56:00 +0800 > Don't submit the rx if the device is unplugged, linking down, > or stopped. ... > @@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags) > { > int ret; > > + /* The rx would be stopped, so skip submitting */ > + if (test_bit(RTL8152_UNPLUG, &tp->flags) || > + !test_bit(WORK_ENABLE, &tp->flags) || !(tp->speed & LINK_STATUS)) > + return 0; > + I think netif_carrier_off() should always be true in all three of those situations, and would be a much simpler test than what you've coded here. ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH net-next 2/2] r8152: check the status before submittingrx 2014-12-19 20:44 ` David Miller @ 2014-12-22 2:53 ` Hayes Wang 2014-12-22 5:22 ` David Miller 0 siblings, 1 reply; 9+ messages in thread From: Hayes Wang @ 2014-12-22 2:53 UTC (permalink / raw) To: David Miller; +Cc: netdev, nic_swsd, linux-kernel, linux-usb David Miller [mailto:davem@davemloft.net] > Sent: Saturday, December 20, 2014 4:44 AM [...] > > Don't submit the rx if the device is unplugged, linking down, > > or stopped. > ... > > @@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152 > *tp, struct rx_agg *agg, gfp_t mem_flags) > > { > > int ret; > > > > + /* The rx would be stopped, so skip submitting */ > > + if (test_bit(RTL8152_UNPLUG, &tp->flags) || > > + !test_bit(WORK_ENABLE, &tp->flags) || !(tp->speed & LINK_STATUS)) > > + return 0; > > + > > I think netif_carrier_off() should always be true in all three of those > situations, and would be a much simpler test than what you've coded > here. When the device is unplugged or stopped, the linking status may be true, so I add additional checks to avoid the submission. Besides, in set_carrier() I set netif_carrier_on() after ops.enable() to avoid any transmission before I finish starting the tx/rx. tp->rtl_ops.enable(tp); set_bit(RTL8152_SET_RX_MODE, &tp->flags); netif_carrier_on(netdev); However, the r8152_submit_rx() would be called in ops.enable(), and the check of netif_carrier_ok() would be always false. That is why I use tp->speed, not netif_carrier_ok(), to check the linking stauts. Best Regards, Hayes ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net-next 2/2] r8152: check the status before submittingrx 2014-12-22 2:53 ` [PATCH net-next 2/2] r8152: check the status before submittingrx Hayes Wang @ 2014-12-22 5:22 ` David Miller 0 siblings, 0 replies; 9+ messages in thread From: David Miller @ 2014-12-22 5:22 UTC (permalink / raw) To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb From: Hayes Wang <hayeswang@realtek.com> Date: Mon, 22 Dec 2014 02:53:42 +0000 > David Miller [mailto:davem@davemloft.net] >> Sent: Saturday, December 20, 2014 4:44 AM > [...] >> > Don't submit the rx if the device is unplugged, linking down, >> > or stopped. >> ... >> > @@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152 >> *tp, struct rx_agg *agg, gfp_t mem_flags) >> > { >> > int ret; >> > >> > + /* The rx would be stopped, so skip submitting */ >> > + if (test_bit(RTL8152_UNPLUG, &tp->flags) || >> > + !test_bit(WORK_ENABLE, &tp->flags) || !(tp->speed & LINK_STATUS)) >> > + return 0; >> > + >> >> I think netif_carrier_off() should always be true in all three of those >> situations, and would be a much simpler test than what you've coded >> here. > > When the device is unplugged or stopped, the linking status > may be true, so I add additional checks to avoid the submission. > > Besides, in set_carrier() I set netif_carrier_on() after > ops.enable() to avoid any transmission before I finish > starting the tx/rx. > > tp->rtl_ops.enable(tp); > set_bit(RTL8152_SET_RX_MODE, &tp->flags); > netif_carrier_on(netdev); > > However, the r8152_submit_rx() would be called in ops.enable(), > and the check of netif_carrier_ok() would be always false. That > is why I use tp->speed, not netif_carrier_ok(), to check the > linking stauts. I stil think your check is way too complicated for this fast path so I would ask that you arrange things such that the simpler netif_carrier_off() test works. Especially because that is what the core networking stack uses to decide whether to send packets to us as well. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v2 0/2] r8152: adjust r8152_submit_rx 2014-12-19 8:55 [PATCH net-next 0/2] r8152: adjust r8152_submit_rx Hayes Wang 2014-12-19 8:55 ` [PATCH net-next 1/2] r8152: adjust set_carrier Hayes Wang 2014-12-19 8:56 ` [PATCH net-next 2/2] r8152: check the status before submitting rx Hayes Wang @ 2014-12-22 6:52 ` Hayes Wang 2014-12-22 6:52 ` [PATCH net-next v2 1/2] r8152: call rtl_start_rx after netif_carrier_on Hayes Wang 2014-12-22 6:52 ` [PATCH net-next v2 2/2] r8152: check the status before submitting rx Hayes Wang 2 siblings, 2 replies; 9+ messages in thread From: Hayes Wang @ 2014-12-22 6:52 UTC (permalink / raw) To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb v2: Replace the patch #1 with "call rtl_start_rx after netif_carrier_on". For patch #2, replace checking tp->speed with netif_carrier_ok. v1: Avoid r8152_submit_rx() from submitting rx during unexpected moment. This could reduce the time of stopping rx. For patch #1, the tp->speed should be updated early. Then, the patch #2 could use it to check the current linking status. Hayes Wang (2): r8152: call rtl_start_rx after netif_carrier_on r8152: check the status before submitting rx drivers/net/usb/r8152.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH net-next v2 1/2] r8152: call rtl_start_rx after netif_carrier_on 2014-12-22 6:52 ` [PATCH net-next v2 0/2] r8152: adjust r8152_submit_rx Hayes Wang @ 2014-12-22 6:52 ` Hayes Wang 2014-12-22 6:52 ` [PATCH net-next v2 2/2] r8152: check the status before submitting rx Hayes Wang 1 sibling, 0 replies; 9+ messages in thread From: Hayes Wang @ 2014-12-22 6:52 UTC (permalink / raw) To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb Remove rtl_start_rx() from rtl_enable() and put it after calling netif_carrier_on(). Signed-off-by: Hayes Wang <hayeswang@realtek.com> --- drivers/net/usb/r8152.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index 2d1c77e..cbe450c 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -2043,7 +2043,7 @@ static int rtl_enable(struct r8152 *tp) rxdy_gated_en(tp, false); - return rtl_start_rx(tp); + return 0; } static int rtl8152_enable(struct r8152 *tp) @@ -2858,6 +2858,7 @@ static void set_carrier(struct r8152 *tp) tp->rtl_ops.enable(tp); set_bit(RTL8152_SET_RX_MODE, &tp->flags); netif_carrier_on(netdev); + rtl_start_rx(tp); } } else { if (tp->speed & LINK_STATUS) { -- 2.1.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH net-next v2 2/2] r8152: check the status before submitting rx 2014-12-22 6:52 ` [PATCH net-next v2 0/2] r8152: adjust r8152_submit_rx Hayes Wang 2014-12-22 6:52 ` [PATCH net-next v2 1/2] r8152: call rtl_start_rx after netif_carrier_on Hayes Wang @ 2014-12-22 6:52 ` Hayes Wang 1 sibling, 0 replies; 9+ messages in thread From: Hayes Wang @ 2014-12-22 6:52 UTC (permalink / raw) To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb Don't submit the rx if the device is unplugged, stopped, or linking down. Signed-off-by: Hayes Wang <hayeswang@realtek.com> --- drivers/net/usb/r8152.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c index cbe450c..8ecc2df 100644 --- a/drivers/net/usb/r8152.c +++ b/drivers/net/usb/r8152.c @@ -1789,6 +1789,11 @@ int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags) { int ret; + /* The rx would be stopped, so skip submitting */ + if (test_bit(RTL8152_UNPLUG, &tp->flags) || + !test_bit(WORK_ENABLE, &tp->flags) || !netif_carrier_ok(tp->netdev)) + return 0; + usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1), agg->head, agg_buf_sz, (usb_complete_t)read_bulk_callback, agg); -- 2.1.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-12-22 6:53 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-12-19 8:55 [PATCH net-next 0/2] r8152: adjust r8152_submit_rx Hayes Wang 2014-12-19 8:55 ` [PATCH net-next 1/2] r8152: adjust set_carrier Hayes Wang 2014-12-19 8:56 ` [PATCH net-next 2/2] r8152: check the status before submitting rx Hayes Wang 2014-12-19 20:44 ` David Miller 2014-12-22 2:53 ` [PATCH net-next 2/2] r8152: check the status before submittingrx Hayes Wang 2014-12-22 5:22 ` David Miller 2014-12-22 6:52 ` [PATCH net-next v2 0/2] r8152: adjust r8152_submit_rx Hayes Wang 2014-12-22 6:52 ` [PATCH net-next v2 1/2] r8152: call rtl_start_rx after netif_carrier_on Hayes Wang 2014-12-22 6:52 ` [PATCH net-next v2 2/2] r8152: check the status before submitting rx Hayes Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).