Re: [PATCH 1/1] mwifiex: queue main work from main process when bailing on races

[Daniel Mack <zonque@gmail.com>]

Hi Bing,

On 16.09.2013 23:14, Bing Zhao wrote:
>> 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);
> 
> This is specific to SDIO interface,

Is it really? By checking adapter->mwifiex_processing, the driver seems
to expect mwifiex_main_process() to be called from multiple execution
paths, and in that case, we will always loose one execution cycle in
case we bail early. I actually wonder why this didn't hit us earlier,
but I might miss a detail.

OTOH, the worst thing that can happen if the function is executed too
often is that it exits early and does nothing.

> +               if (adapter->iface_type == MWIFIEX_SDIO)
> +                       queue_work(adapter->workqueue, &adapter->main_work);

I can of course add this, but I don't fully understand why the driver
takes care of concurrently running executing paths and then just bails
silently in case a race is detected.


Best regards,
Daniel