On Fri, Nov 08, 2024 at 02:23:48AM +0000, Ping-Ke Shih wrote:
> Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxx> wrote:
> > rtl_init_core creates a workqueue that is then assigned to rtl_wq.
> > rtl_deinit_core does not destroy it. It is left to rtl_usb_deinit, which
> > must be called in the probe error path.
> >
> > Fixes: 2ca20f79e0d8 ("rtlwifi: Add usb driver")
> > Fixes: 851639fdaeac ("rtlwifi: Modify some USB de-initialize code.")
> > Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@xxxxxxxxxx>
> > ---
> > drivers/net/wireless/realtek/rtlwifi/usb.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/wireless/realtek/rtlwifi/usb.c b/drivers/net/wireless/realtek/rtlwifi/usb.c
> > index 8ec687fab572..0368ecea2e81 100644
> > --- a/drivers/net/wireless/realtek/rtlwifi/usb.c
> > +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
> > @@ -1039,6 +1039,7 @@ int rtl_usb_probe(struct usb_interface *intf,
> > wait_for_completion(&rtlpriv->firmware_loading_complete);
> > rtlpriv->cfg->ops->deinit_sw_vars(hw);
> > error_out:
> > + rtl_usb_deinit(hw);
> > rtl_deinit_core(hw);
> > error_out2:
> > _rtl_usb_io_handler_release(hw);
>
> I think deinit should be in reverse order of init step by step:
Well, I kept the order that they appear in the remove path. Also, I checked
that they are not exactly independent and rtl_usb_deinit does not need to
be called when rtl_init_core fails. I have just checked and it wouldn't
cause any harm either if rtl_usb_deinit is called in case rtl_init_core
fails. So either way should be fine.
Cascardo.
>
> --- a/drivers/net/wireless/realtek/rtlwifi/usb.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/usb.c
> @@ -1017,7 +1017,7 @@ int rtl_usb_probe(struct usb_interface *intf,
> err = rtl_init_core(hw);
> if (err) {
> pr_err("Can't allocate sw for mac80211\n");
> - goto error_out2;
> + goto error_out_usb_deinit;
> }
> if (rtlpriv->cfg->ops->init_sw_vars(hw)) {
> pr_err("Can't init_sw_vars\n");
> @@ -1040,6 +1040,8 @@ int rtl_usb_probe(struct usb_interface *intf,
> rtlpriv->cfg->ops->deinit_sw_vars(hw);
> error_out:
> rtl_deinit_core(hw);
> +error_out_usb_deinit:
> + rtl_usb_deinit(hw);
> error_out2:
> _rtl_usb_io_handler_release(hw);
> usb_put_dev(udev);
>
> Have you also considered PCI? It seems that rtl_pci_deinit() isn't called if
> PCI probe fails.
>
>
