2020-06-24 09:17:29

by Wright Feng

[permalink] [raw]
Subject: [PATCH v2 0/2] brcmfmac: Stability and throughput changes

This series fixes stability issue and enhances throughput.

Changes in v2:
- Add more testing results in commit message

Wright Feng (2):
brcmfmac: set state of hanger slot to FREE when flushing PSQ
brcmfmac: Set pacing shift before transmitting skb to bus

drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 +++
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c | 4 ++++
2 files changed, 7 insertions(+)

--
2.25.0


2020-06-24 09:17:44

by Wright Feng

[permalink] [raw]
Subject: [PATCH v2 1/2] brcmfmac: set state of hanger slot to FREE when flushing PSQ

When USB or SDIO device got abnormal bus disconnection, host driver
tried to clean up the skbs in PSQ and TXQ (The skb's pointer in hanger
slot linked to PSQ and TSQ), so we should set the state of skb hanger slot
to BRCMF_FWS_HANGER_ITEM_STATE_FREE before freeing skb.
In brcmf_fws_bus_txq_cleanup it already sets
BRCMF_FWS_HANGER_ITEM_STATE_FREE before freeing skb, therefore we add the
same thing in brcmf_fws_psq_flush to avoid following warning message.

[ 1580.012880] ------------ [ cut here ]------------
[ 1580.017550] WARNING: CPU: 3 PID: 3065 at
drivers/net/wireless/broadcom/brcm80211/brcmutil/utils.c:49
brcmu_pkt_buf_free_skb+0x21/0x30 [brcmutil]
[ 1580.184017] Call Trace:
[ 1580.186514] brcmf_fws_cleanup+0x14e/0x190 [brcmfmac]
[ 1580.191594] brcmf_fws_del_interface+0x70/0x90 [brcmfmac]
[ 1580.197029] brcmf_proto_bcdc_del_if+0xe/0x10 [brcmfmac]
[ 1580.202418] brcmf_remove_interface+0x69/0x190 [brcmfmac]
[ 1580.207888] brcmf_detach+0x90/0xe0 [brcmfmac]
[ 1580.212385] brcmf_usb_disconnect+0x76/0xb0 [brcmfmac]
[ 1580.217557] usb_unbind_interface+0x72/0x260
[ 1580.221857] device_release_driver_internal+0x141/0x200
[ 1580.227152] device_release_driver+0x12/0x20
[ 1580.231460] bus_remove_device+0xfd/0x170
[ 1580.235504] device_del+0x1d9/0x300
[ 1580.239041] usb_disable_device+0x9e/0x270
[ 1580.243160] usb_disconnect+0x94/0x270
[ 1580.246980] hub_event+0x76d/0x13b0
[ 1580.250499] process_one_work+0x144/0x360
[ 1580.254564] worker_thread+0x4d/0x3c0
[ 1580.258247] kthread+0x109/0x140
[ 1580.261515] ? rescuer_thread+0x340/0x340
[ 1580.265543] ? kthread_park+0x60/0x60
[ 1580.269237] ? SyS_exit_group+0x14/0x20
[ 1580.273118] ret_from_fork+0x25/0x30
[ 1580.300446] ------------ [ cut here ]------------

Acked-by: Arend van Spriel <[email protected]>
Signed-off-by: Wright Feng <[email protected]>
Signed-off-by: Chi-hsien Lin <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
index 09701262330d..babaac682f13 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
@@ -621,6 +621,7 @@ static inline int brcmf_fws_hanger_poppkt(struct brcmf_fws_hanger *h,
static void brcmf_fws_psq_flush(struct brcmf_fws_info *fws, struct pktq *q,
int ifidx)
{
+ struct brcmf_fws_hanger_item *hi;
bool (*matchfn)(struct sk_buff *, void *) = NULL;
struct sk_buff *skb;
int prec;
@@ -632,6 +633,9 @@ static void brcmf_fws_psq_flush(struct brcmf_fws_info *fws, struct pktq *q,
skb = brcmu_pktq_pdeq_match(q, prec, matchfn, &ifidx);
while (skb) {
hslot = brcmf_skb_htod_tag_get_field(skb, HSLOT);
+ hi = &fws->hanger.items[hslot];
+ WARN_ON(skb != hi->pkt);
+ hi->state = BRCMF_FWS_HANGER_ITEM_STATE_FREE;
brcmf_fws_hanger_poppkt(&fws->hanger, hslot, &skb,
true);
brcmu_pkt_buf_free_skb(skb);
--
2.25.0

2020-06-24 09:18:02

by Wright Feng

[permalink] [raw]
Subject: [PATCH v2 2/2] brcmfmac: set pacing shift before transmitting skb to bus

Linux 3.6 introduces TSQ which has a per socket threshold for TCP Tx
packet to reduce latency. In flow control mode, host driver enqueues skb
in hanger and TCP doesn't push new skb frees until host frees the skb when
receiving fwstatus event. So set pacing shift 8 to send them as a single
large aggregate frame to the bus layer.

43455 TX TCP throughput in different FC modes on Linux 5.4.18

sk_pacing_shift : Throughput (fcmode=0)
10: 245 Mbps
9: 245 Mbps
8: 246 Mbps
7: 246 Mbps

sk_pacing_shift : Throughput (fcmode=1)
10: 182 Mbps
9: 197 Mbps
8: 206 Mbps
7: 207 Mbps

sk_pacing_shift : Throughput (fcmode=2)
10: 180 Mbps
9: 197 Mbps
8: 206 Mbps
7: 207 Mbps

Signed-off-by: Wright Feng <[email protected]>
Signed-off-by: Chi-hsien Lin <[email protected]>
---
v2:
- Add more testing results

drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index c88655acc78c..f89010a81ffb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -352,6 +352,9 @@ static netdev_tx_t brcmf_netdev_start_xmit(struct sk_buff *skb,
if ((skb->priority == 0) || (skb->priority > 7))
skb->priority = cfg80211_classify8021d(skb, NULL);

+ /* set pacing shift for packet aggregation */
+ sk_pacing_shift_update(skb->sk, 8);
+
ret = brcmf_proto_tx_queue_data(drvr, ifp->ifidx, skb);
if (ret < 0)
brcmf_txfinalize(ifp, skb, false);
--
2.25.0

2020-07-14 09:51:42

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] brcmfmac: set state of hanger slot to FREE when flushing PSQ

Wright Feng <[email protected]> wrote:

> When USB or SDIO device got abnormal bus disconnection, host driver
> tried to clean up the skbs in PSQ and TXQ (The skb's pointer in hanger
> slot linked to PSQ and TSQ), so we should set the state of skb hanger slot
> to BRCMF_FWS_HANGER_ITEM_STATE_FREE before freeing skb.
> In brcmf_fws_bus_txq_cleanup it already sets
> BRCMF_FWS_HANGER_ITEM_STATE_FREE before freeing skb, therefore we add the
> same thing in brcmf_fws_psq_flush to avoid following warning message.
>
> [ 1580.012880] ------------ [ cut here ]------------
> [ 1580.017550] WARNING: CPU: 3 PID: 3065 at
> drivers/net/wireless/broadcom/brcm80211/brcmutil/utils.c:49
> brcmu_pkt_buf_free_skb+0x21/0x30 [brcmutil]
> [ 1580.184017] Call Trace:
> [ 1580.186514] brcmf_fws_cleanup+0x14e/0x190 [brcmfmac]
> [ 1580.191594] brcmf_fws_del_interface+0x70/0x90 [brcmfmac]
> [ 1580.197029] brcmf_proto_bcdc_del_if+0xe/0x10 [brcmfmac]
> [ 1580.202418] brcmf_remove_interface+0x69/0x190 [brcmfmac]
> [ 1580.207888] brcmf_detach+0x90/0xe0 [brcmfmac]
> [ 1580.212385] brcmf_usb_disconnect+0x76/0xb0 [brcmfmac]
> [ 1580.217557] usb_unbind_interface+0x72/0x260
> [ 1580.221857] device_release_driver_internal+0x141/0x200
> [ 1580.227152] device_release_driver+0x12/0x20
> [ 1580.231460] bus_remove_device+0xfd/0x170
> [ 1580.235504] device_del+0x1d9/0x300
> [ 1580.239041] usb_disable_device+0x9e/0x270
> [ 1580.243160] usb_disconnect+0x94/0x270
> [ 1580.246980] hub_event+0x76d/0x13b0
> [ 1580.250499] process_one_work+0x144/0x360
> [ 1580.254564] worker_thread+0x4d/0x3c0
> [ 1580.258247] kthread+0x109/0x140
> [ 1580.261515] ? rescuer_thread+0x340/0x340
> [ 1580.265543] ? kthread_park+0x60/0x60
> [ 1580.269237] ? SyS_exit_group+0x14/0x20
> [ 1580.273118] ret_from_fork+0x25/0x30
> [ 1580.300446] ------------ [ cut here ]------------
>
> Acked-by: Arend van Spriel <[email protected]>
> Signed-off-by: Wright Feng <[email protected]>
> Signed-off-by: Chi-hsien Lin <[email protected]>

2 patches applied to wireless-drivers-next.git, thanks.

fcdd7a875def brcmfmac: set state of hanger slot to FREE when flushing PSQ
2fa8085fc6da brcmfmac: set pacing shift before transmitting skb to bus

--
https://patchwork.kernel.org/patch/11622711/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches