Re: [PATCH 3/3] brcmfmac: make setting SDIO workqueue WQ_HIGHPRI a module parameter

Arend Van Spriel 於 3/19/2020 4:55 PM 寫道:

+ Tejun - regarding alloc_workqueue usage

On 3/19/2020 8:53 AM, Wright Feng wrote:

quoted

With setting sdio_wq_highpri=1 in module parameters, tasks submitted to
SDIO workqueue will put at the head of the queue and run immediately.
This parameter is for getting higher TX/RX throughput with SDIO bus.

Signed-off-by: Wright Feng <redacted>
Signed-off-by: Chi-hsien Lin <redacted>
---
  .../wireless/broadcom/brcm80211/brcmfmac/common.c  |  5 +++++
  .../wireless/broadcom/brcm80211/brcmfmac/common.h  |  2 ++
  .../wireless/broadcom/brcm80211/brcmfmac/sdio.c    | 22 
++++++++++++++--------
  3 files changed, 21 insertions(+), 8 deletions(-)

[...]

quoted

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c 

b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 3a08252..885e8bd 100644

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c

@@ -4342,9 +4342,21 @@ struct brcmf_sdio *brcmf_sdio_probe(struct 

brcmf_sdio_dev *sdiodev)
      bus->txminmax = BRCMF_TXMINMAX;
      bus->tx_seq = SDPCM_SEQ_WRAP - 1;
+    /* attempt to attach to the dongle */
+    if (!(brcmf_sdio_probe_attach(bus))) {
+        brcmf_err("brcmf_sdio_probe_attach failed\n");
+        goto fail;
+    }
+
      /* single-threaded workqueue */
-    wq = alloc_ordered_workqueue("brcmf_wq/%s", WQ_MEM_RECLAIM,
-                     dev_name(&sdiodev->func1->dev));
+    if (sdiodev->settings->sdio_wq_highpri) {
+        wq = alloc_workqueue("brcmf_wq/%s",
+                     WQ_HIGHPRI | WQ_MEM_RECLAIM | WQ_UNBOUND,
+                     1, dev_name(&sdiodev->func1->dev));

So two things changed, 1) WQ_HIGHPRI flag added *and* 2) use 
allow_workqueue basically dropping __WQ_ORDERED. Not sure which one 
contributes to the behavior described in the commit message.

The combination of Unbound and max_active==1 implies ordered, so I 
suppose we don't need to set __WQ_ORDERED bit in flags.

Regards,
Arend

quoted

+    } else {
+        wq = alloc_ordered_workqueue("brcmf_wq/%s", WQ_MEM_RECLAIM,
+                         dev_name(&sdiodev->func1->dev));
+    }
      if (!wq) {
          brcmf_err("insufficient memory to create txworkqueue\n");
          goto fail;