Re: [RFC] mt76: fix tx hung regression on MT7630E

[Stanislaw Gruszka <sgruszka@redhat.com>]

On Mon, Aug 05, 2019 at 01:27:19PM +0200, Lorenzo Bianconi wrote:
> > ... but I think we have bug when do mt76_txq_schedule_all() in
> > tx_tasklet, because we can schedule on queues that are stoped.
> > So reverting 41634aa8d6db and then optimize by removing tx_tasklet
> > for mmio and remove not needed mt76_txq_schedule_all() calls looks
> > more reasoneble to me.
> 
> schedule a stopped queue seems not harmful at a first glance since we do not
> copy pending skbs if we have not enough room in the dma ring.

mac80211 stop queues for various other reasons than 
IEEE80211_QUEUE_STOP_REASON_DRIVER .
 
> Maybe we can be
> more conservative doing something like:
>
> diff --git a/drivers/net/wireless/mediatek/mt76/dma.c b/drivers/net/wireless/mediatek/mt76/dma.c
> index d8f61e540bfd..c6482155e5e4 100644
> --- a/drivers/net/wireless/mediatek/mt76/dma.c
> +++ b/drivers/net/wireless/mediatek/mt76/dma.c
> @@ -346,6 +346,11 @@ mt76_dma_tx_queue_skb(struct mt76_dev *dev, enum mt76_txq_id qid,
>  		goto unmap;
>  
>  	if (q->queued + (tx_info.nbuf + 1) / 2 >= q->ndesc - 1) {
> +		if (!q->stopped) {
> +			ieee80211_stop_queue(dev->hw,
> +					     skb_get_queue_mapping(skb));
> +			q->stopped = true;
> +		}
>  		ret = -ENOMEM;
>  		goto unmap;
>  	}
> diff --git a/drivers/net/wireless/mediatek/mt76/tx.c b/drivers/net/wireless/mediatek/mt76/tx.c
> index 5397827668b9..bd2d34c4f326 100644
> --- a/drivers/net/wireless/mediatek/mt76/tx.c
> +++ b/drivers/net/wireless/mediatek/mt76/tx.c
> @@ -495,6 +495,9 @@ mt76_txq_schedule_list(struct mt76_dev *dev, enum mt76_txq_id qid)
>  	while (1) {
>  		bool empty = false;
>  
> +		if (hwq->stopped)
> +			break;
> +
>  		if (sq->swq_queued >= 4)
>  			break;
> 
> Does it fix the issue you are facing?

I'll not be able to test this patch this week. Will have access to
the hardware next week. 

I checeked before, if
'q->queued + (tx_info.nbuf + 1) / 2 >= q->ndesc - 1' is triggered
when MT7630E hangs and it is not. But maybe second part of the patch
will help.

Stanislaw