On Mon, 2010-10-11 at 12:56 +0300, Juuso Oikarinen wrote:
> > @@ -537,6 +533,17 @@ static void wl1271_irq_work(struct work_struct *work)
> > (wl->tx_results_count & 0xff))
> > wl1271_tx_complete(wl);
> >
> > + /* Check if any tx blocks were freed */
> > + if ((wl->tx_blocks_available > prev_tx_blocks) &&
> > + !skb_queue_empty(&wl->tx_queue)) {
> > + /*
> > + * In order to avoid starvation of the TX path,
> > + * call the work function directly.
> > + */
> > + cancel_work_sync(&wl->tx_work);
>
> Hmm, isn't this causing a theoretical potential for a dead-lock? The
> tx_work could be waiting in mutex-lock already when cancel_work_sync is
> called, in which case cancel_work_sync would lock forever.
>
> IIRC the irq_work and tx_work currently run in the same queue, so this
> may work with the current driver. Smells like a hazard anyway, and
> changing the workqueues for each work could easily lead to dead locks
> here. So at minimum I'd like to see it documented in the comment why
> this cannot cause a deadlock.
It should still cause a lockdep warning which you may want to avoid ...
and unless I'm looking at the wrong driver, this is in an irqs disabled
section which is wrong as well.
johannes
--
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
