[PATCH net-next] r8152: divide the tx and rx bottom functions

[PATCH net-next] r8152: divide the tx and rx bottom functions

[Hayes Wang]

Move the tx bottom function from NAPI to a new tasklet. Then, for
multi-cores, the bottom functions of tx and rx may be run at same
time with different cores. This is used to improve performance.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 40d18e866269..3ed9f8e082c9 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -619,7 +619,7 @@ enum rtl8152_flags {
 	RTL8152_LINK_CHG,
 	SELECTIVE_SUSPEND,
 	PHY_RESET,
-	SCHEDULE_NAPI,
+	SCHEDULE_TASKLET,
 	GREEN_ETHERNET,
 	DELL_TB_RX_AGG_BUG,
 };
@@ -733,6 +733,7 @@ struct r8152 {
 #ifdef CONFIG_PM_SLEEP
 	struct notifier_block pm_notifier;
 #endif
+	struct tasklet_struct tx_tl;
 
 	struct rtl_ops {
 		void (*init)(struct r8152 *);
@@ -1401,7 +1402,7 @@ static void write_bulk_callback(struct urb *urb)
 		return;
 
 	if (!skb_queue_empty(&tp->tx_queue))
-		napi_schedule(&tp->napi);
+		tasklet_schedule(&tp->tx_tl);
 }
 
 static void intr_callback(struct urb *urb)
@@ -2179,8 +2180,12 @@ static void tx_bottom(struct r8152 *tp)
 	} while (res == 0);
 }
 
-static void bottom_half(struct r8152 *tp)
+static void bottom_half(unsigned long data)
 {
+	struct r8152 *tp;
+
+	tp = (struct r8152 *)data;
+
 	if (test_bit(RTL8152_UNPLUG, &tp->flags))
 		return;
 
@@ -2192,7 +2197,7 @@ static void bottom_half(struct r8152 *tp)
 	if (!netif_carrier_ok(tp->netdev))
 		return;
 
-	clear_bit(SCHEDULE_NAPI, &tp->flags);
+	clear_bit(SCHEDULE_TASKLET, &tp->flags);
 
 	tx_bottom(tp);
 }
@@ -2203,16 +2208,12 @@ static int r8152_poll(struct napi_struct *napi, int budget)
 	int work_done;
 
 	work_done = rx_bottom(tp, budget);
-	bottom_half(tp);
 
 	if (work_done < budget) {
 		if (!napi_complete_done(napi, work_done))
 			goto out;
 		if (!list_empty(&tp->rx_done))
 			napi_schedule(napi);
-		else if (!skb_queue_empty(&tp->tx_queue) &&
-			 !list_empty(&tp->tx_free))
-			napi_schedule(napi);
 	}
 
 out:
@@ -2366,11 +2367,11 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
 
 	if (!list_empty(&tp->tx_free)) {
 		if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
-			set_bit(SCHEDULE_NAPI, &tp->flags);
+			set_bit(SCHEDULE_TASKLET, &tp->flags);
 			schedule_delayed_work(&tp->schedule, 0);
 		} else {
 			usb_mark_last_busy(tp->udev);
-			napi_schedule(&tp->napi);
+			tasklet_schedule(&tp->tx_tl);
 		}
 	} else if (skb_queue_len(&tp->tx_queue) > tp->tx_qlen) {
 		netif_stop_queue(netdev);
@@ -4020,9 +4021,11 @@ static void set_carrier(struct r8152 *tp)
 	} else {
 		if (netif_carrier_ok(netdev)) {
 			netif_carrier_off(netdev);
+			tasklet_disable(&tp->tx_tl);
 			napi_disable(napi);
 			tp->rtl_ops.disable(tp);
 			napi_enable(napi);
+			tasklet_enable(&tp->tx_tl);
 			netif_info(tp, link, netdev, "carrier off\n");
 		}
 	}
@@ -4055,10 +4058,10 @@ static void rtl_work_func_t(struct work_struct *work)
 	if (test_and_clear_bit(RTL8152_SET_RX_MODE, &tp->flags))
 		_rtl8152_set_rx_mode(tp->netdev);
 
-	/* don't schedule napi before linking */
-	if (test_and_clear_bit(SCHEDULE_NAPI, &tp->flags) &&
+	/* don't schedule tasket before linking */
+	if (test_and_clear_bit(SCHEDULE_TASKLET, &tp->flags) &&
 	    netif_carrier_ok(tp->netdev))
-		napi_schedule(&tp->napi);
+		tasklet_schedule(&tp->tx_tl);
 
 	mutex_unlock(&tp->control);
 
@@ -4144,6 +4147,7 @@ static int rtl8152_open(struct net_device *netdev)
 		goto out_unlock;
 	}
 	napi_enable(&tp->napi);
+	tasklet_enable(&tp->tx_tl);
 
 	mutex_unlock(&tp->control);
 
@@ -4171,6 +4175,7 @@ static int rtl8152_close(struct net_device *netdev)
 #ifdef CONFIG_PM_SLEEP
 	unregister_pm_notifier(&tp->pm_notifier);
 #endif
+	tasklet_disable(&tp->tx_tl);
 	if (!test_bit(RTL8152_UNPLUG, &tp->flags))
 		napi_disable(&tp->napi);
 	clear_bit(WORK_ENABLE, &tp->flags);
@@ -4440,6 +4445,7 @@ static int rtl8152_pre_reset(struct usb_interface *intf)
 		return 0;
 
 	netif_stop_queue(netdev);
+	tasklet_disable(&tp->tx_tl);
 	napi_disable(&tp->napi);
 	clear_bit(WORK_ENABLE, &tp->flags);
 	usb_kill_urb(tp->intr_urb);
@@ -4483,6 +4489,7 @@ static int rtl8152_post_reset(struct usb_interface *intf)
 	}
 
 	napi_enable(&tp->napi);
+	tasklet_enable(&tp->tx_tl);
 	netif_wake_queue(netdev);
 	usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
@@ -4636,10 +4643,12 @@ static int rtl8152_system_suspend(struct r8152 *tp)
 
 		clear_bit(WORK_ENABLE, &tp->flags);
 		usb_kill_urb(tp->intr_urb);
+		tasklet_disable(&tp->tx_tl);
 		napi_disable(napi);
 		cancel_delayed_work_sync(&tp->schedule);
 		tp->rtl_ops.down(tp);
 		napi_enable(napi);
+		tasklet_enable(&tp->tx_tl);
 	}
 
 	return 0;
@@ -5499,6 +5508,8 @@ static int rtl8152_probe(struct usb_interface *intf,
 	mutex_init(&tp->control);
 	INIT_DELAYED_WORK(&tp->schedule, rtl_work_func_t);
 	INIT_DELAYED_WORK(&tp->hw_phy_work, rtl_hw_phy_work_func_t);
+	tasklet_init(&tp->tx_tl, bottom_half, (unsigned long)tp);
+	tasklet_disable(&tp->tx_tl);
 
 	netdev->netdev_ops = &rtl8152_netdev_ops;
 	netdev->watchdog_timeo = RTL8152_TX_TIMEOUT;
@@ -5585,6 +5596,7 @@ static int rtl8152_probe(struct usb_interface *intf,
 
 out1:
 	netif_napi_del(&tp->napi);
+	tasklet_kill(&tp->tx_tl);
 	usb_set_intfdata(intf, NULL);
 out:
 	free_netdev(netdev);
@@ -5601,6 +5613,7 @@ static void rtl8152_disconnect(struct usb_interface *intf)
 
 		netif_napi_del(&tp->napi);
 		unregister_netdev(tp->netdev);
+		tasklet_kill(&tp->tx_tl);
 		cancel_delayed_work_sync(&tp->hw_phy_work);
 		tp->rtl_ops.unload(tp);
 		free_netdev(tp->netdev);
-- 
2.21.0

Re: [PATCH net-next] r8152: divide the tx and rx bottom functions

[David Miller]

From: Hayes Wang <hayeswang@realtek.com>
Date: Wed, 14 Aug 2019 16:30:17 +0800

> Move the tx bottom function from NAPI to a new tasklet. Then, for
> multi-cores, the bottom functions of tx and rx may be run at same
> time with different cores. This is used to improve performance.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>

Theoretically, yes.

But do you have actual performance numbers showing this to be worth
the change?

Always provide performance numbers with changes that are supposed to
improve performance.

RE: [PATCH net-next] r8152: divide the tx and rx bottom functions

[Hayes Wang]

David Miller [mailto:davem@davemloft.net]
> Sent: Friday, August 16, 2019 4:59 AM
[...]
> Theoretically, yes.
> 
> But do you have actual performance numbers showing this to be worth
> the change?
> 
> Always provide performance numbers with changes that are supposed to
> improve performance.

On x86, they are almost the same.
Tx/Rx: 943/943 Mbits/sec -> 945/944

For arm platform,
Tx/Rx: 917/917 Mbits/sec -> 933/933
Improve about 1.74%.

Best Regards,
Hayes

Re: [PATCH net-next] r8152: divide the tx and rx bottom functions

[David Miller]

From: Hayes Wang <hayeswang@realtek.com>
Date: Fri, 16 Aug 2019 02:59:16 +0000

> David Miller [mailto:davem@davemloft.net]
>> Sent: Friday, August 16, 2019 4:59 AM
> [...]
>> Theoretically, yes.
>> 
>> But do you have actual performance numbers showing this to be worth
>> the change?
>> 
>> Always provide performance numbers with changes that are supposed to
>> improve performance.
> 
> On x86, they are almost the same.
> Tx/Rx: 943/943 Mbits/sec -> 945/944
> 
> For arm platform,
> Tx/Rx: 917/917 Mbits/sec -> 933/933
> Improve about 1.74%.

Belongs in the commit message.

Re: [PATCH net-next] r8152: divide the tx and rx bottom functions

[Eric Dumazet]

On 8/14/19 10:30 AM, Hayes Wang wrote:
> Move the tx bottom function from NAPI to a new tasklet. Then, for
> multi-cores, the bottom functions of tx and rx may be run at same
> time with different cores. This is used to improve performance.
> 
>  

tasklet and NAPI are scheduled on the same core (the current
cpu calling napi_schedule() or tasklet_schedule())

I would rather not add this dubious tasklet, and instead try to understand
what is wrong in this driver ;)

The various napi_schedule() calls are suspect IMO.

Also rtl8152_start_xmit() uses skb_queue_tail(&tp->tx_queue, skb);

But I see nothing really kicking the transmit if tx_free is empty ?

tx_bottom() is only called from bottom_half() (called from r8152_poll())

RE: [PATCH net-next] r8152: divide the tx and rx bottom functions

[Hayes Wang]

Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Friday, August 16, 2019 2:40 PM
[...]
> tasklet and NAPI are scheduled on the same core (the current
> cpu calling napi_schedule() or tasklet_schedule())
> 
> I would rather not add this dubious tasklet, and instead try to understand
> what is wrong in this driver ;)
> 
> The various napi_schedule() calls are suspect IMO.

The original method as following.

static int r8152_poll(struct napi_struct *napi, int budget)
{
	struct r8152 *tp = container_of(napi, struct r8152, napi);
	int work_done;

	work_done = rx_bottom(tp, budget); <-- RX
	bottom_half(tp); <-- Tx (tx_bottom)
	[...]

The rx_bottom and tx_bottom would only be called in r8152_poll.
That is, tx_bottom wouldn't be run unless rx_bottom is finished.
And, rx_bottom would be called if tx_bottom is running.

If the traffic is busy. rx_bottom or tx_bottom may take a lot
of time to deal with the packets. And the one would increase
the latency time for the other one.

Therefore, when I separate the tx_bottom and rx_bottom to
different tasklet and napi, the callback functions of tx and
rx may schedule the tasklet and napi to different cpu. Then,
the rx_bottom and tx_bottom may be run at the same time.

Take our arm platform for example. There are five cpus to
handle the interrupt of USB host controller. When the rx is
completed, cpu #1 may handle the interrupt and napi would
be scheduled. When the tx is finished, cpu #2 may handle
the interrupt and the tasklet is scheduled. Then, napi is
run on cpu #1, and tasklet is run on cpu #2.

> Also rtl8152_start_xmit() uses skb_queue_tail(&tp->tx_queue, skb);
> 
> But I see nothing really kicking the transmit if tx_free is empty ?

Tx callback function "write_bulk_callback" would deal with it.
The callback function would check if there are packets waiting
to be sent.


Best Regards,
Hayes

Re: [PATCH net-next] r8152: divide the tx and rx bottom functions

[Eric Dumazet]

On 8/16/19 10:10 AM, Hayes Wang wrote:
> Eric Dumazet [mailto:eric.dumazet@gmail.com]
>> Sent: Friday, August 16, 2019 2:40 PM
> [...]
>> tasklet and NAPI are scheduled on the same core (the current
>> cpu calling napi_schedule() or tasklet_schedule())
>>
>> I would rather not add this dubious tasklet, and instead try to understand
>> what is wrong in this driver ;)
>>
>> The various napi_schedule() calls are suspect IMO.
> 
> The original method as following.
> 
> static int r8152_poll(struct napi_struct *napi, int budget)
> {
> 	struct r8152 *tp = container_of(napi, struct r8152, napi);
> 	int work_done;
> 
> 	work_done = rx_bottom(tp, budget); <-- RX
> 	bottom_half(tp); <-- Tx (tx_bottom)
> 	[...]
> 
> The rx_bottom and tx_bottom would only be called in r8152_poll.
> That is, tx_bottom wouldn't be run unless rx_bottom is finished.
> And, rx_bottom would be called if tx_bottom is running.
> 
> If the traffic is busy. rx_bottom or tx_bottom may take a lot
> of time to deal with the packets. And the one would increase
> the latency time for the other one.
> 
> Therefore, when I separate the tx_bottom and rx_bottom to
> different tasklet and napi, the callback functions of tx and
> rx may schedule the tasklet and napi to different cpu. Then,
> the rx_bottom and tx_bottom may be run at the same time.


Your patch makes absolutely no guarantee of doing what you
want, I am afraid.

> 
> Take our arm platform for example. There are five cpus to
> handle the interrupt of USB host controller. When the rx is
> completed, cpu #1 may handle the interrupt and napi would
> be scheduled. When the tx is finished, cpu #2 may handle
> the interrupt and the tasklet is scheduled. Then, napi is
> run on cpu #1, and tasklet is run on cpu #2.
> 
>> Also rtl8152_start_xmit() uses skb_queue_tail(&tp->tx_queue, skb);
>>
>> But I see nothing really kicking the transmit if tx_free is empty ?
> 
> Tx callback function "write_bulk_callback" would deal with it.
> The callback function would check if there are packets waiting
> to be sent.

Which callback ?

After an idle period (no activity, no prior packets being tx-completed ...),
a packet is sent by the upper stack, enters the ndo_start_xmit() of a network driver.

This driver ndo_start_xmit() simply adds an skb to a local list, and returns.

Where/how is scheduled this callback ?

Some kind of timer ?
An (unrelated) incoming packet ?

> 
> 
> Best Regards,
> Hayes
> 
> 

RE: [PATCH net-next] r8152: divide the tx and rx bottom functions

[Hayes Wang]

Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Friday, August 16, 2019 4:20 PM
[...]
> Which callback ?

The USB device has two endpoints for Tx and Rx.
If I submit tx or rx URB to the USB host controller,
the relative callback functions would be called, when
they are finished. For rx, it is read_bulk_callback.
For tx, it is write_bulk_callback.

> After an idle period (no activity, no prior packets being tx-completed ...),
> a packet is sent by the upper stack, enters the ndo_start_xmit() of a network
> driver.
> 
> This driver ndo_start_xmit() simply adds an skb to a local list, and returns.

Base on the current method (without tasklet), when
ndo_start_xmit() is called, napi_schedule is called only
if there is at least one free buffer (!list_empty(&tp->tx_free))
to transmit the packet. Then, the flow would be as following.

    - Call r8152_poll
     -- Call bottom_half
      --- Call tx_bottom
       ---- Call r8152_tx_agg_fill
        ----- submit tx urb

    - Call write_bulk_callback if tx is completed

When the tx transfer is completed, write_bulk_callback would
be called. And it would check if there is any tx packet
in &tp->tx_queue and determine whether it is necessary to
schedule the napi again or not.

> Where/how is scheduled this callback ?

For tx, you could find the following code in r8152_tx_agg_fill().

	usb_fill_bulk_urb(agg->urb, tp->udev, usb_sndbulkpipe(tp->udev, 2),
			  agg->head, (int)(tx_data - (u8 *)agg->head),
			  (usb_complete_t)write_bulk_callback, agg);
	ret = usb_submit_urb(agg->urb, GFP_ATOMIC);

For rx you could find the following code in r8152_submit_rx().

	usb_fill_bulk_urb(agg->urb, tp->udev, usb_rcvbulkpipe(tp->udev, 1),
			  agg->buffer, tp->rx_buf_sz,
			  (usb_complete_t)read_bulk_callback, agg);

	ret = usb_submit_urb(agg->urb, mem_flags);

> Some kind of timer ?
> An (unrelated) incoming packet ?

When the rx or tx is completed, a interrupt of USB
host controller would be triggered. Then, the callback
functions would be called.

Best Regards,
Hayes

Re: [PATCH net-next] r8152: divide the tx and rx bottom functions

[Eric Dumazet]

On 8/16/19 11:08 AM, Hayes Wang wrote:
> Eric Dumazet [mailto:eric.dumazet@gmail.com]
>> Sent: Friday, August 16, 2019 4:20 PM
> [...]
>> Which callback ?
> 
> The USB device has two endpoints for Tx and Rx.
> If I submit tx or rx URB to the USB host controller,
> the relative callback functions would be called, when
> they are finished. For rx, it is read_bulk_callback.
> For tx, it is write_bulk_callback.
> 
>> After an idle period (no activity, no prior packets being tx-completed ...),
>> a packet is sent by the upper stack, enters the ndo_start_xmit() of a network
>> driver.
>>
>> This driver ndo_start_xmit() simply adds an skb to a local list, and returns.
> 
> Base on the current method (without tasklet), when
> ndo_start_xmit() is called, napi_schedule is called only
> if there is at least one free buffer (!list_empty(&tp->tx_free))
> to transmit the packet. Then, the flow would be as following.

Very uncommon naming conventions really :/


Maybe you would avoid messing with a tasklet (we really try to get rid
of tasklets in general) using two NAPI, one for TX, one for RX.

Some drivers already use two NAPI, it is fine.

This might avoid the ugly dance in r8152_poll(),
calling napi_schedule(napi) after napi_complete_done() !

RE: [PATCH net-next] r8152: divide the tx and rx bottom functions

[Hayes Wang]

Eric Dumazet [mailto:eric.dumazet@gmail.com]
> Sent: Friday, August 16, 2019 5:27 PM
[...]
> Maybe you would avoid messing with a tasklet (we really try to get rid
> of tasklets in general) using two NAPI, one for TX, one for RX.
> 
> Some drivers already use two NAPI, it is fine.
> 
> This might avoid the ugly dance in r8152_poll(),
> calling napi_schedule(napi) after napi_complete_done() !

The reason is that the USB device couldn't control
the interrupt of USB controller. That is, I couldn't
disable the interrupt before napi_schedule and
enable it after napi_complete_done. If the callback
function occurs during the following timing, it is
possible no one would schedule the napi again.

static int r8152_poll(struct napi_struct *napi, int budget)
{
	struct r8152 *tp = container_of(napi, struct r8152, napi);
	int work_done;

	work_done = rx_bottom(tp, budget);
	bottom_half(tp);

	--> callback occurs here and try to call napi_schedule

	napi_complete_done(napi, work_done)

That is, no tx or rx could be handled unless
something trigger napi_schedule.


Best Regards,
Hayes

[PATCH net-next v2] r8152: divide the tx and rx bottom functions

[Hayes Wang]

Move the tx bottom function from NAPI to a new tasklet. Then, for
multi-cores, the bottom functions of tx and rx may be run at same
time with different cores. This is used to improve performance.

On x86, Tx/Rx 943/943 Mbits/sec -> 945/944.
For arm platform, Tx/Rx: 917/917 Mbits/sec -> 933/933.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
v2: add the performance number in the commit message.
---
 drivers/net/usb/r8152.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 40d18e866269..3ed9f8e082c9 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -619,7 +619,7 @@ enum rtl8152_flags {
 	RTL8152_LINK_CHG,
 	SELECTIVE_SUSPEND,
 	PHY_RESET,
-	SCHEDULE_NAPI,
+	SCHEDULE_TASKLET,
 	GREEN_ETHERNET,
 	DELL_TB_RX_AGG_BUG,
 };
@@ -733,6 +733,7 @@ struct r8152 {
 #ifdef CONFIG_PM_SLEEP
 	struct notifier_block pm_notifier;
 #endif
+	struct tasklet_struct tx_tl;
 
 	struct rtl_ops {
 		void (*init)(struct r8152 *);
@@ -1401,7 +1402,7 @@ static void write_bulk_callback(struct urb *urb)
 		return;
 
 	if (!skb_queue_empty(&tp->tx_queue))
-		napi_schedule(&tp->napi);
+		tasklet_schedule(&tp->tx_tl);
 }
 
 static void intr_callback(struct urb *urb)
@@ -2179,8 +2180,12 @@ static void tx_bottom(struct r8152 *tp)
 	} while (res == 0);
 }
 
-static void bottom_half(struct r8152 *tp)
+static void bottom_half(unsigned long data)
 {
+	struct r8152 *tp;
+
+	tp = (struct r8152 *)data;
+
 	if (test_bit(RTL8152_UNPLUG, &tp->flags))
 		return;
 
@@ -2192,7 +2197,7 @@ static void bottom_half(struct r8152 *tp)
 	if (!netif_carrier_ok(tp->netdev))
 		return;
 
-	clear_bit(SCHEDULE_NAPI, &tp->flags);
+	clear_bit(SCHEDULE_TASKLET, &tp->flags);
 
 	tx_bottom(tp);
 }
@@ -2203,16 +2208,12 @@ static int r8152_poll(struct napi_struct *napi, int budget)
 	int work_done;
 
 	work_done = rx_bottom(tp, budget);
-	bottom_half(tp);
 
 	if (work_done < budget) {
 		if (!napi_complete_done(napi, work_done))
 			goto out;
 		if (!list_empty(&tp->rx_done))
 			napi_schedule(napi);
-		else if (!skb_queue_empty(&tp->tx_queue) &&
-			 !list_empty(&tp->tx_free))
-			napi_schedule(napi);
 	}
 
 out:
@@ -2366,11 +2367,11 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
 
 	if (!list_empty(&tp->tx_free)) {
 		if (test_bit(SELECTIVE_SUSPEND, &tp->flags)) {
-			set_bit(SCHEDULE_NAPI, &tp->flags);
+			set_bit(SCHEDULE_TASKLET, &tp->flags);
 			schedule_delayed_work(&tp->schedule, 0);
 		} else {
 			usb_mark_last_busy(tp->udev);
-			napi_schedule(&tp->napi);
+			tasklet_schedule(&tp->tx_tl);
 		}
 	} else if (skb_queue_len(&tp->tx_queue) > tp->tx_qlen) {
 		netif_stop_queue(netdev);
@@ -4020,9 +4021,11 @@ static void set_carrier(struct r8152 *tp)
 	} else {
 		if (netif_carrier_ok(netdev)) {
 			netif_carrier_off(netdev);
+			tasklet_disable(&tp->tx_tl);
 			napi_disable(napi);
 			tp->rtl_ops.disable(tp);
 			napi_enable(napi);
+			tasklet_enable(&tp->tx_tl);
 			netif_info(tp, link, netdev, "carrier off\n");
 		}
 	}
@@ -4055,10 +4058,10 @@ static void rtl_work_func_t(struct work_struct *work)
 	if (test_and_clear_bit(RTL8152_SET_RX_MODE, &tp->flags))
 		_rtl8152_set_rx_mode(tp->netdev);
 
-	/* don't schedule napi before linking */
-	if (test_and_clear_bit(SCHEDULE_NAPI, &tp->flags) &&
+	/* don't schedule tasket before linking */
+	if (test_and_clear_bit(SCHEDULE_TASKLET, &tp->flags) &&
 	    netif_carrier_ok(tp->netdev))
-		napi_schedule(&tp->napi);
+		tasklet_schedule(&tp->tx_tl);
 
 	mutex_unlock(&tp->control);
 
@@ -4144,6 +4147,7 @@ static int rtl8152_open(struct net_device *netdev)
 		goto out_unlock;
 	}
 	napi_enable(&tp->napi);
+	tasklet_enable(&tp->tx_tl);
 
 	mutex_unlock(&tp->control);
 
@@ -4171,6 +4175,7 @@ static int rtl8152_close(struct net_device *netdev)
 #ifdef CONFIG_PM_SLEEP
 	unregister_pm_notifier(&tp->pm_notifier);
 #endif
+	tasklet_disable(&tp->tx_tl);
 	if (!test_bit(RTL8152_UNPLUG, &tp->flags))
 		napi_disable(&tp->napi);
 	clear_bit(WORK_ENABLE, &tp->flags);
@@ -4440,6 +4445,7 @@ static int rtl8152_pre_reset(struct usb_interface *intf)
 		return 0;
 
 	netif_stop_queue(netdev);
+	tasklet_disable(&tp->tx_tl);
 	napi_disable(&tp->napi);
 	clear_bit(WORK_ENABLE, &tp->flags);
 	usb_kill_urb(tp->intr_urb);
@@ -4483,6 +4489,7 @@ static int rtl8152_post_reset(struct usb_interface *intf)
 	}
 
 	napi_enable(&tp->napi);
+	tasklet_enable(&tp->tx_tl);
 	netif_wake_queue(netdev);
 	usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
@@ -4636,10 +4643,12 @@ static int rtl8152_system_suspend(struct r8152 *tp)
 
 		clear_bit(WORK_ENABLE, &tp->flags);
 		usb_kill_urb(tp->intr_urb);
+		tasklet_disable(&tp->tx_tl);
 		napi_disable(napi);
 		cancel_delayed_work_sync(&tp->schedule);
 		tp->rtl_ops.down(tp);
 		napi_enable(napi);
+		tasklet_enable(&tp->tx_tl);
 	}
 
 	return 0;
@@ -5499,6 +5508,8 @@ static int rtl8152_probe(struct usb_interface *intf,
 	mutex_init(&tp->control);
 	INIT_DELAYED_WORK(&tp->schedule, rtl_work_func_t);
 	INIT_DELAYED_WORK(&tp->hw_phy_work, rtl_hw_phy_work_func_t);
+	tasklet_init(&tp->tx_tl, bottom_half, (unsigned long)tp);
+	tasklet_disable(&tp->tx_tl);
 
 	netdev->netdev_ops = &rtl8152_netdev_ops;
 	netdev->watchdog_timeo = RTL8152_TX_TIMEOUT;
@@ -5585,6 +5596,7 @@ static int rtl8152_probe(struct usb_interface *intf,
 
 out1:
 	netif_napi_del(&tp->napi);
+	tasklet_kill(&tp->tx_tl);
 	usb_set_intfdata(intf, NULL);
 out:
 	free_netdev(netdev);
@@ -5601,6 +5613,7 @@ static void rtl8152_disconnect(struct usb_interface *intf)
 
 		netif_napi_del(&tp->napi);
 		unregister_netdev(tp->netdev);
+		tasklet_kill(&tp->tx_tl);
 		cancel_delayed_work_sync(&tp->hw_phy_work);
 		tp->rtl_ops.unload(tp);
 		free_netdev(tp->netdev);
-- 
2.21.0

Re: [PATCH net-next v2] r8152: divide the tx and rx bottom functions

[David Miller]

From: Hayes Wang <hayeswang@realtek.com>
Date: Mon, 19 Aug 2019 14:40:36 +0800

> Move the tx bottom function from NAPI to a new tasklet. Then, for
> multi-cores, the bottom functions of tx and rx may be run at same
> time with different cores. This is used to improve performance.
> 
> On x86, Tx/Rx 943/943 Mbits/sec -> 945/944.
> For arm platform, Tx/Rx: 917/917 Mbits/sec -> 933/933.
> 
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
> v2: add the performance number in the commit message.

Applied.