2019-11-05 14:38:40

by Mao Wenan

[permalink] [raw]
Subject: [PATCH -next] crypto: hisilicon: move label err to #ifdef CONFIG_NUMA

If CONFIG_NUMA is not set, there is error in function
find_zip_device:
drivers/crypto/hisilicon/zip/zip_main.c:154:13: error:
head undeclared (first use in this function)
free_list(&head);

This is because CONFIG_NUMA is not defined, it should move
label err to #ifdef CONFIG_NUMA.

Fixes: 700f7d0d29c7 ("crypto: hisilicon - fix to return sub-optimal device when best device has no qps")
Signed-off-by: Mao Wenan <[email protected]>
---
drivers/crypto/hisilicon/zip/zip_main.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/hisilicon/zip/zip_main.c b/drivers/crypto/hisilicon/zip/zip_main.c
index 255b63c..0504fb2 100644
--- a/drivers/crypto/hisilicon/zip/zip_main.c
+++ b/drivers/crypto/hisilicon/zip/zip_main.c
@@ -150,10 +150,12 @@ struct hisi_zip *find_zip_device(int node)

return ret;

+#ifdef CONFIG_NUMA
err:
free_list(&head);
mutex_unlock(&hisi_zip_list_lock);
return NULL;
+#endif
}

struct hisi_zip_hw_error {
--
2.7.4


2019-11-06 00:50:54

by Zhou Wang

[permalink] [raw]
Subject: Re: [PATCH -next] crypto: hisilicon: move label err to #ifdef CONFIG_NUMA

On 2019/11/5 22:56, Dan Carpenter wrote:
> The ifdefs in this function were pretty ugly before but this makes it
> super extra ugly... :/ There are bunch of ways to fix this nicely
> but my favourite is this:
>
> Feel free to give me a Suggested-by tag.
>
> diff --git a/drivers/crypto/hisilicon/zip/zip_main.c b/drivers/crypto/hisilicon/zip/zip_main.c
> index 255b63cfbe1d..1b22f0ead56e 100644
> --- a/drivers/crypto/hisilicon/zip/zip_main.c
> +++ b/drivers/crypto/hisilicon/zip/zip_main.c
> @@ -105,20 +105,27 @@ static void free_list(struct list_head *head)
> struct hisi_zip *find_zip_device(int node)
> {
> struct hisi_zip *ret = NULL;
> -#ifdef CONFIG_NUMA
> struct hisi_zip_resource *res, *tmp;
> struct hisi_zip *hisi_zip;
> struct list_head *n;
> struct device *dev;
> LIST_HEAD(head);
>
> + if (!IS_ENABLED(CONFIG_NUMA)) {
> + mutex_lock(&hisi_zip_list_lock);
> + ret = list_first_entry(&hisi_zip_list, struct hisi_zip, list);
> + mutex_unlock(&hisi_zip_list_lock);
> + return ret;
> + }
> +
> mutex_lock(&hisi_zip_list_lock);
>
> list_for_each_entry(hisi_zip, &hisi_zip_list, list) {
> res = kzalloc(sizeof(*res), GFP_KERNEL);
> - if (!res)
> - goto err;
> -
> + if (!res) {
> + ret = NULL;
> + goto done;
> + }
> dev = &hisi_zip->qm.pdev->dev;
> res->hzip = hisi_zip;
> res->distance = node_distance(dev->numa_node, node);
> @@ -140,20 +147,10 @@ struct hisi_zip *find_zip_device(int node)
> }
> }
>
> +done:
> free_list(&head);
> -#else
> - mutex_lock(&hisi_zip_list_lock);
> -
> - ret = list_first_entry(&hisi_zip_list, struct hisi_zip, list);
> -#endif
> mutex_unlock(&hisi_zip_list_lock);
> -
> return ret;
> -
> -err:
> - free_list(&head);
> - mutex_unlock(&hisi_zip_list_lock);
> - return NULL;
> }
>
> struct hisi_zip_hw_error {
>
>
> .
>

Hi Mao and Dan,

I already posted a patch to fix this problem:
[PATCH] crypto: hisilicon - replace #ifdef with IS_ENABLED for CONFIG_NUMA

Many thanks for your help,
Zhou