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

[Wright Feng <wright.feng@cypress.com>]

Arend Van Spriel 於 3/19/2020 4:55 PM 寫道:
> + Tejun - regarding alloc_workqueue usage
> 
> On 3/19/2020 8:53 AM, Wright Feng wrote:
>> 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 <wright.feng@cypress.com>
>> Signed-off-by: Chi-hsien Lin <chi-hsien.lin@cypress.com>
>> ---
>>   .../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(-)
>>
> 
> [...]
> 
>> 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
> 
>> +    } 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;
>