Hi Daniel,
> >> /* 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
You are right. I overlooked it.
> case we bail early. I actually wonder why this didn't hit us earlier,
> but I might miss a detail.
I guess, in your case, the interrupt comes in at line 363 where you have passed the int_status or RX_RCVD checking but the mwifiex_processing flag is still true.
361 if ((adapter->int_status) || IS_CARD_RX_RCVD(adapter))
362 goto process_start;
363
364 spin_lock_irqsave(&adapter->main_proc_lock, flags);
365 adapter->mwifiex_processing = false;
366 spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
The interrupt thread exits because mwifiex_processing is true.
Therefore the mwifiex_main_process misses this interrupt.
>
> 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.
No. Your original patch is fine.
Could you resend it as [PATCH 3.12]? I will ACK in that thread.
Thanks,
Bing
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
