Re: [6.2][regression] after commit cd372b8c99c5a5cf6a464acebb7e4a79af7ec8ae stopping working wifi mt7921e
From: Thorsten Leemhuis
Date: Wed Jan 04 2023 - 09:20:29 EST
Hi, this is your Linux kernel regression tracker. Top-posting for once,
to make this easily accessible to everyone.
Felix, Lorenzo, did below fix for the regression Mikhail reported make
any progress to get mainlined? It doesn't look like it from here, but I
suspect I missed something, that's why I'm asking.
Ciao, Thorsten
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
On 21.12.22 18:17, Felix Fietkau wrote:
> On 21.12.22 17:46, Mikhail Gavrilov wrote:
>> On Wed, Dec 21, 2022 at 7:12 PM Felix Fietkau <nbd@xxxxxxxx> wrote:
>>>
>>> Thanks! I guess I focused on the wrong part of your kernel log
>>> initially. After more code review, I found that there is in fact a DMA
>>> related bug in the commit that your bisection pointed to, which happened
>>> to uncover and trigger the deadlock fixed by my other patch.
>>>
>>> So here's my fix for the DMA issue:
>>> ---
>> [cutted]
>>> qbuf.skip_unmap = false;
>>> - if (mt76_dma_add_buf(dev, q, &qbuf, 1, 0, buf, t) < 0) {
>>> + if (mt76_dma_add_rx_buf(dev, q, &qbuf, buf) < 0) {
>>> dma_unmap_single(dev->dma_dev, addr, len,
>>> DMA_FROM_DEVICE);
>>> skb_free_frag(buf);
>>>
>>
>> Sorry for stupid question.
>>
>> Do you have a separate branch?
>> I see that the code is differ between master branch and the patch.
>>
>> For example in patch the line:
>> - if (mt76_dma_add_buf(dev, q, &qbuf, 1, 0, buf, t) < 0) {
>> replaced by the line:
>> + if (mt76_dma_add_rx_buf(dev, q, &qbuf, buf) < 0) {
>>
>> But in master branch
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/net/wireless/mediatek/mt76/dma.c?id=b6bb9676f2165d518b35ba3bea5f1fcfc0d969bf#n604
>> after line:
>> qbuf.skip_unmap = false;
>> followed the line:
>> mt76_dma_add_buf(dev, q, &qbuf, 1, 0, buf, t);
>> without if condition.
>>
>> So I'm stuck applying the patch :(
> Sorry, I worked on a tree that had other pending fixes applied.
> Please try this:
>
>
> --- a/drivers/net/wireless/mediatek/mt76/dma.c
> +++ b/drivers/net/wireless/mediatek/mt76/dma.c
> @@ -205,6 +205,52 @@ mt76_dma_queue_reset(struct mt76_dev *dev, struct
> mt76_queue *q)
> mt76_dma_sync_idx(dev, q);
> }
>
> +static int
> +mt76_dma_add_rx_buf(struct mt76_dev *dev, struct mt76_queue *q,
> + struct mt76_queue_buf *buf, void *data)
> +{
> + struct mt76_desc *desc = &q->desc[q->head];
> + struct mt76_queue_entry *entry = &q->entry[q->head];
> + struct mt76_txwi_cache *txwi = NULL;
> + u32 buf1 = 0, ctrl;
> + int idx = q->head;
> + int rx_token;
> +
> + ctrl = FIELD_PREP(MT_DMA_CTL_SD_LEN0, buf[0].len);
> +
> + if ((q->flags & MT_QFLAG_WED) &&
> + FIELD_GET(MT_QFLAG_WED_TYPE, q->flags) == MT76_WED_Q_RX) {
> + txwi = mt76_get_rxwi(dev);
> + if (!txwi)
> + return -ENOMEM;
> +
> + rx_token = mt76_rx_token_consume(dev, data, txwi, buf->addr);
> + if (rx_token < 0) {
> + mt76_put_rxwi(dev, txwi);
> + return -ENOMEM;
> + }
> +
> + buf1 |= FIELD_PREP(MT_DMA_CTL_TOKEN, rx_token);
> + ctrl |= MT_DMA_CTL_TO_HOST;
> + }
> +
> + WRITE_ONCE(desc->buf0, cpu_to_le32(buf->addr));
> + WRITE_ONCE(desc->buf1, cpu_to_le32(buf1));
> + WRITE_ONCE(desc->ctrl, cpu_to_le32(ctrl));
> + WRITE_ONCE(desc->info, 0);
> +
> + entry->dma_addr[0] = buf->addr;
> + entry->dma_len[0] = buf->len;
> + entry->txwi = txwi;
> + entry->buf = data;
> + entry->wcid = 0xffff;
> + entry->skip_buf1 = true;
> + q->head = (q->head + 1) % q->ndesc;
> + q->queued++;
> +
> + return idx;
> +}
> +
> static int
> mt76_dma_add_buf(struct mt76_dev *dev, struct mt76_queue *q,
> struct mt76_queue_buf *buf, int nbufs, u32 info,
> @@ -212,65 +258,51 @@ mt76_dma_add_buf(struct mt76_dev *dev, struct
> mt76_queue *q,
> {
> struct mt76_queue_entry *entry;
> struct mt76_desc *desc;
> - u32 ctrl;
> int i, idx = -1;
> + u32 ctrl, next;
> +
> + if (txwi) {
> + q->entry[q->head].txwi = DMA_DUMMY_DATA;
> + q->entry[q->head].skip_buf0 = true;
> + }
>
> for (i = 0; i < nbufs; i += 2, buf += 2) {
> u32 buf0 = buf[0].addr, buf1 = 0;
>
> idx = q->head;
> - q->head = (q->head + 1) % q->ndesc;
> + next = (q->head + 1) % q->ndesc;
>
> desc = &q->desc[idx];
> entry = &q->entry[idx];
>
> - if ((q->flags & MT_QFLAG_WED) &&
> - FIELD_GET(MT_QFLAG_WED_TYPE, q->flags) == MT76_WED_Q_RX) {
> - struct mt76_txwi_cache *t = txwi;
> - int rx_token;
> -
> - if (!t)
> - return -ENOMEM;
> -
> - rx_token = mt76_rx_token_consume(dev, (void *)skb, t,
> - buf[0].addr);
> - buf1 |= FIELD_PREP(MT_DMA_CTL_TOKEN, rx_token);
> - ctrl = FIELD_PREP(MT_DMA_CTL_SD_LEN0, buf[0].len) |
> - MT_DMA_CTL_TO_HOST;
> - } else {
> - if (txwi) {
> - q->entry[q->head].txwi = DMA_DUMMY_DATA;
> - q->entry[q->head].skip_buf0 = true;
> - }
> -
> - if (buf[0].skip_unmap)
> - entry->skip_buf0 = true;
> - entry->skip_buf1 = i == nbufs - 1;
> -
> - entry->dma_addr[0] = buf[0].addr;
> - entry->dma_len[0] = buf[0].len;
> -
> - ctrl = FIELD_PREP(MT_DMA_CTL_SD_LEN0, buf[0].len);
> - if (i < nbufs - 1) {
> - entry->dma_addr[1] = buf[1].addr;
> - entry->dma_len[1] = buf[1].len;
> - buf1 = buf[1].addr;
> - ctrl |= FIELD_PREP(MT_DMA_CTL_SD_LEN1, buf[1].len);
> - if (buf[1].skip_unmap)
> - entry->skip_buf1 = true;
> - }
> -
> - if (i == nbufs - 1)
> - ctrl |= MT_DMA_CTL_LAST_SEC0;
> - else if (i == nbufs - 2)
> - ctrl |= MT_DMA_CTL_LAST_SEC1;
> + if (buf[0].skip_unmap)
> + entry->skip_buf0 = true;
> + entry->skip_buf1 = i == nbufs - 1;
> +
> + entry->dma_addr[0] = buf[0].addr;
> + entry->dma_len[0] = buf[0].len;
> +
> + ctrl = FIELD_PREP(MT_DMA_CTL_SD_LEN0, buf[0].len);
> + if (i < nbufs - 1) {
> + entry->dma_addr[1] = buf[1].addr;
> + entry->dma_len[1] = buf[1].len;
> + buf1 = buf[1].addr;
> + ctrl |= FIELD_PREP(MT_DMA_CTL_SD_LEN1, buf[1].len);
> + if (buf[1].skip_unmap)
> + entry->skip_buf1 = true;
> }
>
> + if (i == nbufs - 1)
> + ctrl |= MT_DMA_CTL_LAST_SEC0;
> + else if (i == nbufs - 2)
> + ctrl |= MT_DMA_CTL_LAST_SEC1;
> +
> WRITE_ONCE(desc->buf0, cpu_to_le32(buf0));
> WRITE_ONCE(desc->buf1, cpu_to_le32(buf1));
> WRITE_ONCE(desc->info, cpu_to_le32(info));
> WRITE_ONCE(desc->ctrl, cpu_to_le32(ctrl));
>
> + q->head = next;
> q->queued++;
> }
>
> @@ -577,17 +609,9 @@ mt76_dma_rx_fill(struct mt76_dev *dev, struct
> mt76_queue *q)
> spin_lock_bh(&q->lock);
>
> while (q->queued < q->ndesc - 1) {
> - struct mt76_txwi_cache *t = NULL;
> struct mt76_queue_buf qbuf;
> void *buf = NULL;
>
> - if ((q->flags & MT_QFLAG_WED) &&
> - FIELD_GET(MT_QFLAG_WED_TYPE, q->flags) == MT76_WED_Q_RX) {
> - t = mt76_get_rxwi(dev);
> - if (!t)
> - break;
> - }
> -
> buf = page_frag_alloc(rx_page, q->buf_size, GFP_ATOMIC);
> if (!buf)
> break;
> @@ -601,7 +625,12 @@ mt76_dma_rx_fill(struct mt76_dev *dev, struct
> mt76_queue *q)
> qbuf.addr = addr + offset;
> qbuf.len = len - offset;
> qbuf.skip_unmap = false;
> - mt76_dma_add_buf(dev, q, &qbuf, 1, 0, buf, t);
> + if (mt76_dma_add_rx_buf(dev, q, &qbuf, buf) < 0) {
> + dma_unmap_single(dev->dma_dev, addr, len,
> + DMA_FROM_DEVICE);
> + skb_free_frag(buf);
> + break;
> + }
> frames++;
> }
>
>
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mmio.c
> @@ -653,6 +653,13 @@ static u32 mt7915_mmio_wed_init_rx_buf(struct
> mtk_wed_device *wed, int size)
>
> desc->buf0 = cpu_to_le32(phy_addr);
> token = mt76_rx_token_consume(&dev->mt76, ptr, t, phy_addr);
> + if (token < 0) {
> + dma_unmap_single(dev->mt76.dma_dev, phy_addr,
> + wed->wlan.rx_size, DMA_TO_DEVICE);
> + skb_free_frag(ptr);
> + goto unmap;
> + }
> +
> desc->token |= cpu_to_le32(FIELD_PREP(MT_DMA_CTL_TOKEN,
> token));
> desc++;
>
> --- a/drivers/net/wireless/mediatek/mt76/tx.c
> +++ b/drivers/net/wireless/mediatek/mt76/tx.c
> @@ -764,11 +764,12 @@ int mt76_rx_token_consume(struct mt76_dev *dev,
> void *ptr,
> spin_lock_bh(&dev->rx_token_lock);
> token = idr_alloc(&dev->rx_token, t, 0, dev->rx_token_size,
> GFP_ATOMIC);
> + if (token >= 0) {
> + t->ptr = ptr;
> + t->dma_addr = phys;
> + }
> spin_unlock_bh(&dev->rx_token_lock);
>
> - t->ptr = ptr;
> - t->dma_addr = phys;
> -
> return token;
> }
> EXPORT_SYMBOL_GPL(mt76_rx_token_consume);
>