Messages in this thread | | | Subject | Re: [PATCH net-next] r8152: divide the tx and rx bottom functions | From | Eric Dumazet <> | Date | Fri, 16 Aug 2019 10:19:33 +0200 |
| |
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 > >
| |