2013-09-17 18:25:49

by Daniel Mack

[permalink] [raw]
Subject: [PATCH 3.12] mwifiex: queue main work from main process when bailing on races

Queue main_work in case mwifiex_main_process() bails due to an already
processed transaction. This is particularly necessary because
mwifiex_main_process() is called from both the SDIO interrupt handler
and the workqueue. In case an interrupt occurs while the main process
is currently executed from the workqueue, the interrupt is lost,
resulting in a command timeout and consequently a card reset.

I'm marking this for stable kernel in version 3.7+, because on our
platform, the issue appears since 601216e12c ("mwifiex: process RX
packets in SDIO IRQ thread directly") went in.

Signed-off-by: Daniel Mack <[email protected]>
Reported-by: Sven Neumann <[email protected]>
Reported-by: Andreas Fenkart <[email protected]>
Cc: Bing Zhao <[email protected]>
Cc: <[email protected]> [v3.7+]
---
drivers/net/wireless/mwifiex/main.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index ff4ed96..0700bc2 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -235,6 +235,7 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
/* Check if already processing */
if (adapter->mwifiex_processing) {
spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
+ queue_work(adapter->workqueue, &adapter->main_work);
goto exit_main_proc;
} else {
adapter->mwifiex_processing = true;
--
1.8.3.1



2013-09-25 14:23:37

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 3.12] mwifiex: queue main work from main process when bailing on races

On 17.09.2013 21:15, Bing Zhao wrote:
>> Queue main_work in case mwifiex_main_process() bails due to an already
>> processed transaction. This is particularly necessary because
>> mwifiex_main_process() is called from both the SDIO interrupt handler and
>> the workqueue. In case an interrupt occurs while the main process is
>> currently executed from the workqueue, the interrupt is lost, resulting in a
>> command timeout and consequently a card reset.
>>
>> I'm marking this for stable kernel in version 3.7+, because on our platform,
>> the issue appears since 601216e12c ("mwifiex: process RX packets in SDIO
>> IRQ thread directly") went in.
>>
>> Signed-off-by: Daniel Mack <[email protected]>
>> Reported-by: Sven Neumann <[email protected]>
>> Reported-by: Andreas Fenkart <[email protected]>
>> Cc: Bing Zhao <[email protected]>
>> Cc: <[email protected]> [v3.7+]
>
> Acked-by: Bing Zhao <[email protected]>

John, could you pick this one?


Many thanks,
Daniel


>
> Thanks,
> Bing
>
>> ---
>> drivers/net/wireless/mwifiex/main.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/wireless/mwifiex/main.c
>> b/drivers/net/wireless/mwifiex/main.c
>> index ff4ed96..0700bc2 100644
>> --- a/drivers/net/wireless/mwifiex/main.c
>> +++ b/drivers/net/wireless/mwifiex/main.c
>> @@ -235,6 +235,7 @@ int mwifiex_main_process(struct mwifiex_adapter
>> *adapter)
>> /* Check if already processing */
>> if (adapter->mwifiex_processing) {
>> spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
>> + queue_work(adapter->workqueue, &adapter->main_work);
>> goto exit_main_proc;
>> } else {
>> adapter->mwifiex_processing = true;
>> --
>> 1.8.3.1
>


2013-09-25 16:20:41

by Bing Zhao

[permalink] [raw]
Subject: RE: [PATCH 3.12] mwifiex: queue main work from main process when bailing on races

Hi Daniel,

> > Acked-by: Bing Zhao <[email protected]>
>
> John, could you pick this one?

We found that this patch causes CPU utilization issue on Chromebooks.
Could you please try attached patch on your platform? Basically this patch reverts your change and add main_proc_lock protection for int_status and IS_CARD_RX_RCVD access to avoid the missing interrupt handling.

Thanks,
Bing


Attachments:
0001-mwifiex-avoid-redundant-main-process-queueing.patch (1.92 kB)
0001-mwifiex-avoid-redundant-main-process-queueing.patch

2013-09-17 19:16:19

by Bing Zhao

[permalink] [raw]
Subject: RE: [PATCH 3.12] mwifiex: queue main work from main process when bailing on races

Hi Daniel,

> Queue main_work in case mwifiex_main_process() bails due to an already
> processed transaction. This is particularly necessary because
> mwifiex_main_process() is called from both the SDIO interrupt handler and
> the workqueue. In case an interrupt occurs while the main process is
> currently executed from the workqueue, the interrupt is lost, resulting in a
> command timeout and consequently a card reset.
>
> I'm marking this for stable kernel in version 3.7+, because on our platform,
> the issue appears since 601216e12c ("mwifiex: process RX packets in SDIO
> IRQ thread directly") went in.
>
> Signed-off-by: Daniel Mack <[email protected]>
> Reported-by: Sven Neumann <[email protected]>
> Reported-by: Andreas Fenkart <[email protected]>
> Cc: Bing Zhao <[email protected]>
> Cc: <[email protected]> [v3.7+]

Acked-by: Bing Zhao <[email protected]>

Thanks,
Bing

> ---
> drivers/net/wireless/mwifiex/main.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/wireless/mwifiex/main.c
> b/drivers/net/wireless/mwifiex/main.c
> index ff4ed96..0700bc2 100644
> --- a/drivers/net/wireless/mwifiex/main.c
> +++ b/drivers/net/wireless/mwifiex/main.c
> @@ -235,6 +235,7 @@ int mwifiex_main_process(struct mwifiex_adapter
> *adapter)
> /* Check if already processing */
> if (adapter->mwifiex_processing) {
> spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
> + queue_work(adapter->workqueue, &adapter->main_work);
> goto exit_main_proc;
> } else {
> adapter->mwifiex_processing = true;
> --
> 1.8.3.1


2013-09-25 16:37:40

by Daniel Mack

[permalink] [raw]
Subject: Re: [PATCH 3.12] mwifiex: queue main work from main process when bailing on races

Hi Bing,

On 25.09.2013 18:19, Bing Zhao wrote:
>>> Acked-by: Bing Zhao <[email protected]>
>>
>> John, could you pick this one?
>
> We found that this patch causes CPU utilization issue on
> Chromebooks. Could you please try attached patch on your platform?
> Basically this patch reverts your change and add main_proc_lock
> protection for int_status and IS_CARD_RX_RCVD access to avoid the
> missing interrupt handling.

Ok, that seems to work fine for me as well. Thanks for the update :)

Fell free to add my

Tested-by: Daniel Mack <[email protected]>

if you need it.


Thanks,
Daniel