diff mbox series

net: microchip: sparx5: Fix missing destroy_workqueue of mact_queue

Message ID 20221201134717.25750-1-linqiheng@huawei.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: microchip: sparx5: Fix missing destroy_workqueue of mact_queue | expand

Checks

Context Check Description
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Qiheng Lin Dec. 1, 2022, 1:47 p.m. UTC
The mchp_sparx5_probe() won't destroy workqueue created by
create_singlethread_workqueue() in sparx5_start() when later
inits failed. Add destroy_workqueue in the cleanup_ports case,
also add it in mchp_sparx5_remove()

Signed-off-by: Qiheng Lin <linqiheng@huawei.com>
---
 drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Pavan Chebbi Dec. 2, 2022, 8:06 a.m. UTC | #1
On Thu, Dec 1, 2022 at 6:57 PM Qiheng Lin <linqiheng@huawei.com> wrote:
>
> The mchp_sparx5_probe() won't destroy workqueue created by
> create_singlethread_workqueue() in sparx5_start() when later
> inits failed. Add destroy_workqueue in the cleanup_ports case,
> also add it in mchp_sparx5_remove()
>
> Signed-off-by: Qiheng Lin <linqiheng@huawei.com>
> ---
>  drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> index eeac04b84638..b6bbb3c9bd7a 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> @@ -887,6 +887,8 @@ static int mchp_sparx5_probe(struct platform_device *pdev)
>
>  cleanup_ports:
>         sparx5_cleanup_ports(sparx5);
> +       if (sparx5->mact_queue)
> +               destroy_workqueue(sparx5->mact_queue);

Would be better if you destroy inside sparx5_start() before returning failure.

>  cleanup_config:
>         kfree(configs);
>  cleanup_pnode:
> @@ -911,6 +913,7 @@ static int mchp_sparx5_remove(struct platform_device *pdev)
>         sparx5_cleanup_ports(sparx5);
>         /* Unregister netdevs */
>         sparx5_unregister_notifier_blocks(sparx5);
> +       destroy_workqueue(sparx5->mact_queue);
>
>         return 0;
>  }
> --
> 2.32.0
>
Pavan Chebbi Dec. 2, 2022, 10:02 a.m. UTC | #2
On Fri, Dec 2, 2022 at 1:36 PM Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:
>
> On Thu, Dec 1, 2022 at 6:57 PM Qiheng Lin <linqiheng@huawei.com> wrote:
> >
> > The mchp_sparx5_probe() won't destroy workqueue created by
> > create_singlethread_workqueue() in sparx5_start() when later
> > inits failed. Add destroy_workqueue in the cleanup_ports case,
> > also add it in mchp_sparx5_remove()
> >
> > Signed-off-by: Qiheng Lin <linqiheng@huawei.com>
> > ---
> >  drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> > index eeac04b84638..b6bbb3c9bd7a 100644
> > --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> > +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> > @@ -887,6 +887,8 @@ static int mchp_sparx5_probe(struct platform_device *pdev)
> >
> >  cleanup_ports:
> >         sparx5_cleanup_ports(sparx5);
> > +       if (sparx5->mact_queue)
> > +               destroy_workqueue(sparx5->mact_queue);
>
> Would be better if you destroy inside sparx5_start() before returning failure.
>

Alternatively you could add the destroy inside sparx5_cleanup_ports()
that will cover all error exits?

> >  cleanup_config:
> >         kfree(configs);
> >  cleanup_pnode:
> > @@ -911,6 +913,7 @@ static int mchp_sparx5_remove(struct platform_device *pdev)
> >         sparx5_cleanup_ports(sparx5);
> >         /* Unregister netdevs */
> >         sparx5_unregister_notifier_blocks(sparx5);
> > +       destroy_workqueue(sparx5->mact_queue);
> >
> >         return 0;
> >  }
> > --
> > 2.32.0
> >
Qiheng Lin Dec. 2, 2022, 10:34 a.m. UTC | #3
在 2022/12/2 18:02, Pavan Chebbi 写道:
> On Fri, Dec 2, 2022 at 1:36 PM Pavan Chebbi <pavan.chebbi@broadcom.com> wrote:
>>
>> On Thu, Dec 1, 2022 at 6:57 PM Qiheng Lin <linqiheng@huawei.com> wrote:
>>>
>>> The mchp_sparx5_probe() won't destroy workqueue created by
>>> create_singlethread_workqueue() in sparx5_start() when later
>>> inits failed. Add destroy_workqueue in the cleanup_ports case,
>>> also add it in mchp_sparx5_remove()
>>>
>>> Signed-off-by: Qiheng Lin <linqiheng@huawei.com>
>>> ---
>>>   drivers/net/ethernet/microchip/sparx5/sparx5_main.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
>>> index eeac04b84638..b6bbb3c9bd7a 100644
>>> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
>>> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
>>> @@ -887,6 +887,8 @@ static int mchp_sparx5_probe(struct platform_device *pdev)
>>>
>>>   cleanup_ports:
>>>          sparx5_cleanup_ports(sparx5);
>>> +       if (sparx5->mact_queue)
>>> +               destroy_workqueue(sparx5->mact_queue);
>>
>> Would be better if you destroy inside sparx5_start() before returning failure.
>>
> 
> Alternatively you could add the destroy inside sparx5_cleanup_ports()
> that will cover all error exits?

That works functionally, I have considered this modification as well. 
Since I'm not quite sure on the naming, destroying the mact_queue 
belongs to sparx5_cleanup_ports, which they don't contain now.

> 
>>>   cleanup_config:
>>>          kfree(configs);
>>>   cleanup_pnode:
>>> @@ -911,6 +913,7 @@ static int mchp_sparx5_remove(struct platform_device *pdev)
>>>          sparx5_cleanup_ports(sparx5);
>>>          /* Unregister netdevs */
>>>          sparx5_unregister_notifier_blocks(sparx5);
>>> +       destroy_workqueue(sparx5->mact_queue);
>>>
>>>          return 0;
>>>   }
>>> --
>>> 2.32.0
>>>
Jakub Kicinski Dec. 3, 2022, 4:59 a.m. UTC | #4
On Thu, 1 Dec 2022 21:47:17 +0800 Qiheng Lin wrote:
> The mchp_sparx5_probe() won't destroy workqueue created by
> create_singlethread_workqueue() in sparx5_start() when later
> inits failed. Add destroy_workqueue in the cleanup_ports case,
> also add it in mchp_sparx5_remove()

This is a fix so it needs a Fixes tag.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
index eeac04b84638..b6bbb3c9bd7a 100644
--- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
+++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
@@ -887,6 +887,8 @@  static int mchp_sparx5_probe(struct platform_device *pdev)
 
 cleanup_ports:
 	sparx5_cleanup_ports(sparx5);
+	if (sparx5->mact_queue)
+		destroy_workqueue(sparx5->mact_queue);
 cleanup_config:
 	kfree(configs);
 cleanup_pnode:
@@ -911,6 +913,7 @@  static int mchp_sparx5_remove(struct platform_device *pdev)
 	sparx5_cleanup_ports(sparx5);
 	/* Unregister netdevs */
 	sparx5_unregister_notifier_blocks(sparx5);
+	destroy_workqueue(sparx5->mact_queue);
 
 	return 0;
 }