| Message ID | aa7d7ed8f3a817525ab8c7b736f1ce52490a835e.1658176763.git.sean.wang@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | [1/3] mt76: mt7921s: fix the deadlock caused by sdio->stat_work | expand |
> From: Sean Wang <sean.wang@mediatek.com> > > Because wake_work and sdio->stat_work share the same workqueue mt76->wq, > if sdio->stat_work cannot acquire the mutex lock such as that was possibly > held up by mt7921_mutex_acquire, we should exit immediately and schedule > another stat_work to avoid blocking the mt7921_mutex_acquire. > > Also, if mt7921_mutex_acquire was called by sdio->stat_work self, the wake > would be blocked by itself, so we have to changing into an unblocking wake > (directly wakeup via mt7921_mcu_drv_pmctrl, not via the wake_work) in the > context. Hi Sean, it seems to me we are missing some logic here (e.g cancelling ps_work as we do in mt76_connac_pm_wake()). Is it enough to move mt7921_usb_sdio_tx_status_data on mac80211 workqueue? Can you see any performance issue? (same for mt7663). Regards, Lorenzo > > Fixes: 48fab5bbef40 ("mt76: mt7921: introduce mt7921s support") > Co-developed-by: YN Chen <YN.Chen@mediatek.com> > Signed-off-by: YN Chen <YN.Chen@mediatek.com> > Signed-off-by: Sean Wang <sean.wang@mediatek.com> > --- > .../net/wireless/mediatek/mt76/mt7921/mac.c | 22 +++++++++++++++++-- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c > index 6bd9fc9228a2..75e719175e92 100644 > --- a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c > +++ b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c > @@ -1080,10 +1080,28 @@ bool mt7921_usb_sdio_tx_status_data(struct mt76_dev *mdev, u8 *update) > { > struct mt7921_dev *dev = container_of(mdev, struct mt7921_dev, mt76); > > - mt7921_mutex_acquire(dev); > + if (!mutex_trylock(&mdev->mutex)) { > + /* Because wake_work and stat_work share the same workqueue > + * mt76->wq, if sdio->stat_work cannot acquire the mutex lock, > + * we should exit immediately and schedule another stat_work > + * to avoid blocking the wake_work. > + */ > + struct work_struct *stat_work; > + > + stat_work = mt76_is_sdio(mdev) ? &mdev->sdio.stat_work : > + &mdev->usb.stat_work; > + queue_work(dev->mt76.wq, stat_work); > + > + goto out; > + } > + > + mt7921_mcu_drv_pmctrl(dev); > mt7921_mac_sta_poll(dev); > - mt7921_mutex_release(dev); > + mt76_connac_power_save_sched(&mdev->phy, &dev->pm); > > + mutex_unlock(&mdev->mutex); > + > +out: > return false; > } > EXPORT_SYMBOL_GPL(mt7921_usb_sdio_tx_status_data); > -- > 2.25.1 >
diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c index 6bd9fc9228a2..75e719175e92 100644 --- a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c +++ b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c @@ -1080,10 +1080,28 @@ bool mt7921_usb_sdio_tx_status_data(struct mt76_dev *mdev, u8 *update) { struct mt7921_dev *dev = container_of(mdev, struct mt7921_dev, mt76); - mt7921_mutex_acquire(dev); + if (!mutex_trylock(&mdev->mutex)) { + /* Because wake_work and stat_work share the same workqueue + * mt76->wq, if sdio->stat_work cannot acquire the mutex lock, + * we should exit immediately and schedule another stat_work + * to avoid blocking the wake_work. + */ + struct work_struct *stat_work; + + stat_work = mt76_is_sdio(mdev) ? &mdev->sdio.stat_work : + &mdev->usb.stat_work; + queue_work(dev->mt76.wq, stat_work); + + goto out; + } + + mt7921_mcu_drv_pmctrl(dev); mt7921_mac_sta_poll(dev); - mt7921_mutex_release(dev); + mt76_connac_power_save_sched(&mdev->phy, &dev->pm); + mutex_unlock(&mdev->mutex); + +out: return false; } EXPORT_SYMBOL_GPL(mt7921_usb_sdio_tx_status_data);