Re: [PATCH] wfx: avoid flush_workqueue(system_highpri_wq) usage

["Jérôme Pouiller" <jerome.pouiller@silabs.com>]

On Sunday 1 May 2022 10:53:57 CEST Kalle Valo wrote:
> Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:
> 
> > Flushing system-wide workqueues is dangerous and will be forbidden.
> > Replace system_highpri_wq with local wfx_wq.
> >
> > While we are at it, add missing spi_unregister_driver() call when
> > sdio_register_driver() failed.
> >
> > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> 
> [...]
> 
> > @@ -473,10 +475,18 @@ static int __init wfx_core_init(void)
> >  {
> >       int ret = 0;
> >
> > +     wfx_wq = alloc_workqueue("wfx_wq", WQ_HIGHPRI, 0);
> > +     if (!wfx_wq)
> > +             return -ENOMEM;
> >       if (IS_ENABLED(CONFIG_SPI))
> >               ret = spi_register_driver(&wfx_spi_driver);
> >       if (IS_ENABLED(CONFIG_MMC) && !ret)
> >               ret = sdio_register_driver(&wfx_sdio_driver);
> > +     if (ret) {
> > +             if (IS_ENABLED(CONFIG_SPI))
> > +                     spi_unregister_driver(&wfx_spi_driver);
> > +             destroy_workqueue(wfx_wq);
> > +     }
> >       return ret;
> >  }
> >  module_init(wfx_core_init);
> 
> So now the thread is created every time the module loaded, even if
> there's no device available. Also I'm not really a fan of global
> variables (wfx_wq). I would rather create a workqueue per device in
> wfx_probe() or use the workqueue provided by mac80211.
> 
> /**
>  * ieee80211_queue_work - add work onto the mac80211 workqueue
>  *
>  * Drivers and mac80211 use this to add work onto the mac80211 workqueue.
>  * This helper ensures drivers are not queueing work when they should not be.
>  *
>  * @hw: the hardware struct for the interface we are adding work for
>  * @work: the work we want to add onto the mac80211 workqueue
>  */
> void ieee80211_queue_work(struct ieee80211_hw *hw, struct work_struct *work);
> 

The last time I have checked if I could use this workqueue, I remember
it was not well suited for wfx (I don't remember why exactly). So I
believe we have to allocate a new workqueue.


-- 
Jérôme Pouiller