Re: [PATCH 02/12] net: hns3: Destroy a mutex after initialisation failure in hclge_init_ad_dev()

Re: [PATCH 02/12] net: hns3: Destroy a mutex after initialisation failure in hclge_init_ad_dev()

[Markus Elfring]

> Add a mutex destroy call in hclge_init_ae_dev() when fails.

How do you think about a wording variant like the following?

   Change description:
   The function “mutex_init” was called before a call of
   the function “hclge_pci_init”.
   But the function “mutex_destroy” was not called after initialisation
   steps failed.
   Thus add the missed function call for the completion of
   the exception handling.


Would you like to add the tag “Fixes” to the commit message?


…
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> @@ -10108,6 +10108,7 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
>  	pci_release_regions(pdev);
>  	pci_disable_device(pdev);
>  out:
> +	mutex_destroy(&hdev->vport_lock);
>  	return ret;
>  }

How do you think about to use the label “destroy_mutex” instead?

Regards,
Markus

Re: [PATCH 02/12] net: hns3: Destroy a mutex after initialisation failure in hclge_init_ad_dev()

[tanhuazhong]

On 2020/5/29 2:42, Markus Elfring wrote:
>> Add a mutex destroy call in hclge_init_ae_dev() when fails.
> 
> How do you think about a wording variant like the following?
> 
>     Change description:
>     The function “mutex_init” was called before a call of
>     the function “hclge_pci_init”.
>     But the function “mutex_destroy” was not called after initialisation
>     steps failed.
>     Thus add the missed function call for the completion of
>     the exception handling.
> 

It looks better. I will try to improve the skill of patch description
and make as many as people can understand the patch.

Thanks for help.

> 
> Would you like to add the tag “Fixes” to the commit message?
> 

Since it seems not a very urgent issue, so i send it to the -next
and make it as a code optimization.

Thanks:)

> 
> …
>> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
>> @@ -10108,6 +10108,7 @@ static int hclge_init_ae_dev(struct hnae3_ae_dev *ae_dev)
>>   	pci_release_regions(pdev);
>>   	pci_disable_device(pdev);
>>   out:
>> +	mutex_destroy(&hdev->vport_lock);
>>   	return ret;
>>   }
> 
> How do you think about to use the label “destroy_mutex” instead?

Will use label 'destroy_mutex‘ instead if there is another patch need to 
modify this code, which is more readable.

Thanks for your comments.

> 
> Regards,
> Markus
> 
> 

Re: [02/12] net: hns3: Destroy a mutex after initialisation failure in hclge_init_ae_dev()

[Markus Elfring]

>>> Add a mutex destroy call in hclge_init_ae_dev() when fails.
>>
>> How do you think about a wording variant like the following?
…
> It looks better. I will try to improve the skill of patch description
> and make as many as people can understand the patch.

Thanks for your positive feedback.

I suggest to avoid also a typo in the function name of the patch subject.

Regards,
Markus

Re: [02/12] net: hns3: Destroy a mutex after initialisation failure in hclge_init_ae_dev()

[Markus Elfring]

>> Would you like to add the tag “Fixes” to the commit message?
>
> Since it seems not a very urgent issue, so i send it to the -next

I suggest to take another look at the prioritisation for another
completion of the exception handling.


> and make it as a code optimization.

I propose to reconsider also such a view.

Regards,
Markus