Re: [PATCH] mt76: usb: reduce locking in mt76u_tx_tasklet

[Lorenzo Bianconi <lorenzo.bianconi@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]

> On Sun, Mar 24, 2019 at 03:51:43PM +0100, Lorenzo Bianconi wrote:
> > +		int idx;
> > +
> >  		sq = &dev->q_tx[i];
> >  		q = sq->q;
> >  
> > -		spin_lock_bh(&q->lock);
> > -		while (true) {
> > -			if (!q->entry[q->head].done || !q->queued)
> > +		while (q->queued > n_queued) {
> > +			if (!q->entry[q->head].done)
> >  				break;
> If you place done = false here you will not need additional idx
> variable.

As Felix suggested, I would set done to false at the end of the loop, after
tx_complete_skb

> 
> >  			dev->drv->tx_complete_skb(dev, i, &entry);
> > -			spin_lock_bh(&q->lock);
> > +			q->entry[idx].done = false;
> >  		}
> >  
> > +		spin_lock_bh(&q->lock);
> This patch does not apply for me as there is missing
> mt76_txq_schedule(dev, sq);

Sorry I forgot to mention this patch is based on
https://patchwork.kernel.org/patch/10856027/. Have you applied it?

> 
> > +
> > +		sq->swq_queued -= n_sw_queued;
> > +		q->queued -= n_queued;
> > +
> Naming is confusing, it should rather be n_dequeued, n_sw_dequeued.

I just followed dma counterpart naming convention, but I can modify it.

Regards,
Lorenzo

> 
> Stanislaw

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]