On 04.01.23 15:20, Thorsten Leemhuis wrote: > 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 There is another report about an issue with mediatek wifi in 6.2-rc: https://bugzilla.kernel.org/show_bug.cgi?id=216901 To me this looks like a duplicate of the report that started this thread. (side note: there was another, earlier report that might be a dupe, too: https://bugzilla.kernel.org/show_bug.cgi?id=216829 ) > 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. No reply. :-(( That lack of feedback is another reason why I'm CCing the network maintainers now, as the mediatek wifi issues in 6.2-rc (this one) and 6.1 ([1]) are already hitting a nerve here because the fixes are progressing so slowly. I known, it was holiday season, but seems quite a few people ran into these regressions already, hence we IMHO should really try to aim fixing both this week. [1] see https://lore.kernel.org/all/ac023262-c6cb-01ad-aeee-2dbf379f4c37@xxxxxxxxxxxxx/ Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- 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 > -- > 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); >>
