On 4 February 2015 at 22:11, Eric Dumazet <eric.dumazet@xxxxxxxxx> wrote: > I do not see how a TSO patch could hurt a flow not using TSO/GSO. > > This makes no sense. > > ath10k tx completions being batched/deferred to a tasklet might increase > probability to hit this condition in tcp_wfree() : > > /* If this softirq is serviced by ksoftirqd, we are likely under stress. > * Wait until our queues (qdisc + devices) are drained. > * This gives : > * - less callbacks to tcp_write_xmit(), reducing stress (batches) > * - chance for incoming ACK (processed by another cpu maybe) > * to migrate this flow (skb->ooo_okay will be eventually set) > */ > if (wmem >= SKB_TRUESIZE(1) && this_cpu_ksoftirqd() == current) > goto out; > > Meaning tcp stack waits all skbs left qdisc/NIC queues before queuing > additional packets. > > I would try to call skb_orphan() in ath10k if you really want to keep > these batches. > > I have hard time to understand why tx completed packets go through > ath10k_htc_rx_completion_handler().. anyway... There's a couple of layers for host-firmware communication. The transport layer (e.g. PCI) delivers HTC packets. These contain WMI (configuration stuff) or HTT (traffic stuff). HTT can contain different events (tx complete, rx complete, etc). HTT Tx completion contains a list of ids which refer to frames that have been completed (either sent or dropped). I've tried reverting tx/rx tasklet batching. No change in throughput. I can get tcpdump if you're interested. > Most conservative patch would be : > > diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c > index 9c782a42665e1aaf43bfbca441631ee58da50c09..6a36317d6bb0447202dee15528130bd5e21248c4 100644 > --- a/drivers/net/wireless/ath/ath10k/htt_rx.c > +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c > @@ -1642,6 +1642,7 @@ void ath10k_htt_t2h_msg_handler(struct ath10k *ar, struct sk_buff *skb) > break; > } > case HTT_T2H_MSG_TYPE_TX_COMPL_IND: > + skb_orphan(skb); > spin_lock_bh(&htt->tx_lock); > __skb_queue_tail(&htt->tx_compl_q, skb); > spin_unlock_bh(&htt->tx_lock); I suppose you want to call skb_orphan() on actual data packets, right? This skb is just a host-firmware communication buffer. Michał -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
