Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 3/3] brcmfmac: make setting SDIO workqueue WQ_HIGHPRI a
 module parameter
To:     Wright Feng <wright.feng@cypress.com>, franky.lin@broadcom.com,
        hante.meuleman@broadcom.com, kvalo@codeaurora.org,
        chi-hsien.lin@cypress.com
Cc:     linux-wireless@vger.kernel.org,
        brcm80211-dev-list.pdl@broadcom.com, tj@kernel.org
References: <1584604406-15452-1-git-send-email-wright.feng@cypress.com>
 <1584604406-15452-4-git-send-email-wright.feng@cypress.com>
 <40e33702-d37f-085d-a5a7-7f09ae9e2629@broadcom.com>
 <241fd81b-a252-ed86-b93e-6796cf1bce23@cypress.com>
From:   Arend Van Spriel <arend.vanspriel@broadcom.com>
Message-ID: <d220ae51-9613-5b0a-2006-e9250603fd42@broadcom.com>
Date:   Fri, 20 Mar 2020 09:18:39 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101
 Thunderbird/68.5.0
MIME-Version: 1.0
In-Reply-To: <241fd81b-a252-ed86-b93e-6796cf1bce23@cypress.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

On 3/20/2020 9:08 AM, Wright Feng wrote:
> 
> 
> 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.

My reason for asking was the idea to only determine flags in the 
if-statement and have following by one alloc_wq call, ie.:

wq_flags = WQ_MEM_RECLAIM;
if (sdio_wq_highpri)
	wq_flags |= WQ_HIGHPRI
wq = alloc_ordered_workqueue(..., wq_flags,...);

Regards,
Arend