[PATCH] RDMA/siw: Print error code while kthread_create failed

[PATCH] RDMA/siw: Print error code while kthread_create failed

[YueHaibing]

In iw_create_tx_threads(), if we failed to create kthread,
we should print the 'rv', this fix gcc warning:

drivers/infiniband/sw/siw/siw_main.c: In function 'siw_create_tx_threads':
drivers/infiniband/sw/siw/siw_main.c:91:11: warning:
 variable 'rv' set but not used [-Wunused-but-set-variable]

Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/infiniband/sw/siw/siw_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/sw/siw/siw_main.c b/drivers/infiniband/sw/siw/siw_main.c
index fd2552a..2a70830d 100644
--- a/drivers/infiniband/sw/siw/siw_main.c
+++ b/drivers/infiniband/sw/siw/siw_main.c
@@ -101,7 +101,8 @@ static int siw_create_tx_threads(void)
 		if (IS_ERR(siw_tx_thread[cpu])) {
 			rv = PTR_ERR(siw_tx_thread[cpu]);
 			siw_tx_thread[cpu] = NULL;
-			pr_info("Creating TX thread for CPU %d failed", cpu);
+			pr_info("Creating TX thread for CPU%d failed %d\n",
+				cpu, rv);
 			continue;
 		}
 		kthread_bind(siw_tx_thread[cpu], cpu);
-- 
2.7.4

Re: [PATCH] RDMA/siw: Print error code while kthread_create failed

[Leon Romanovsky]

On Wed, Jul 10, 2019 at 09:50:09AM +0800, YueHaibing wrote:
> In iw_create_tx_threads(), if we failed to create kthread,
> we should print the 'rv', this fix gcc warning:
>
> drivers/infiniband/sw/siw/siw_main.c: In function 'siw_create_tx_threads':
> drivers/infiniband/sw/siw/siw_main.c:91:11: warning:
>  variable 'rv' set but not used [-Wunused-but-set-variable]
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/infiniband/sw/siw/siw_main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/sw/siw/siw_main.c b/drivers/infiniband/sw/siw/siw_main.c
> index fd2552a..2a70830d 100644
> --- a/drivers/infiniband/sw/siw/siw_main.c
> +++ b/drivers/infiniband/sw/siw/siw_main.c
> @@ -101,7 +101,8 @@ static int siw_create_tx_threads(void)
>  		if (IS_ERR(siw_tx_thread[cpu])) {
>  			rv = PTR_ERR(siw_tx_thread[cpu]);
>  			siw_tx_thread[cpu] = NULL;
> -			pr_info("Creating TX thread for CPU %d failed", cpu);
> +			pr_info("Creating TX thread for CPU%d failed %d\n",
> +				cpu, rv);

Delete this print together with variable, failure to create kthread
is basic failure, which affect performance only. The whole kthread
creation spam in this driver looked suspicious during submission
and it continues to be.

Thanks

>  			continue;
>  		}
>  		kthread_bind(siw_tx_thread[cpu], cpu);
> --
> 2.7.4
>
>

[PATCH] RDMA/siw: remove unnecessary print in iw_create_tx_threads

[YueHaibing]

In iw_create_tx_threads(), failure to create kthread
is basic failure, which affect performance only. The
whole kthread creation spam in this driver looked
suspicious during submission and it continues to be.
This patch remove the failed print to fix gcc warning:

drivers/infiniband/sw/siw/siw_main.c: In function 'siw_create_tx_threads':
drivers/infiniband/sw/siw/siw_main.c:91:11: warning:
 variable 'rv' set but not used [-Wunused-but-set-variable]

Reported-by: Hulk Robot <hulkci@huawei.com>
Suggested-by: Leon Romanovsky <leon@kernel.org>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/infiniband/sw/siw/siw_main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_main.c b/drivers/infiniband/sw/siw/siw_main.c
index fd2552a..f55c4e8 100644
--- a/drivers/infiniband/sw/siw/siw_main.c
+++ b/drivers/infiniband/sw/siw/siw_main.c
@@ -88,7 +88,7 @@ static void siw_device_cleanup(struct ib_device *base_dev)
 
 static int siw_create_tx_threads(void)
 {
-	int cpu, rv, assigned = 0;
+	int cpu, assigned = 0;
 
 	for_each_online_cpu(cpu) {
 		/* Skip HT cores */
@@ -99,9 +99,7 @@ static int siw_create_tx_threads(void)
 			kthread_create(siw_run_sq, (unsigned long *)(long)cpu,
 				       "siw_tx/%d", cpu);
 		if (IS_ERR(siw_tx_thread[cpu])) {
-			rv = PTR_ERR(siw_tx_thread[cpu]);
 			siw_tx_thread[cpu] = NULL;
-			pr_info("Creating TX thread for CPU %d failed", cpu);
 			continue;
 		}
 		kthread_bind(siw_tx_thread[cpu], cpu);
-- 
2.7.4

Re: Re: [PATCH] RDMA/siw: Print error code while kthread_create failed

[Bernard Metzler]

-----"Leon Romanovsky" <leon@kernel.org> wrote: -----

>To: "YueHaibing" <yuehaibing@huawei.com>
>From: "Leon Romanovsky" <leon@kernel.org>
>Date: 07/10/2019 06:36AM
>Cc: bmt@zurich.ibm.com, dledford@redhat.com, jgg@ziepe.ca,
>linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org
>Subject: [EXTERNAL] Re: [PATCH] RDMA/siw: Print error code while
>kthread_create failed
>
>On Wed, Jul 10, 2019 at 09:50:09AM +0800, YueHaibing wrote:
>> In iw_create_tx_threads(), if we failed to create kthread,
>> we should print the 'rv', this fix gcc warning:
>>
>> drivers/infiniband/sw/siw/siw_main.c: In function
>'siw_create_tx_threads':
>> drivers/infiniband/sw/siw/siw_main.c:91:11: warning:
>>  variable 'rv' set but not used [-Wunused-but-set-variable]
>>
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>> ---
>>  drivers/infiniband/sw/siw/siw_main.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/sw/siw/siw_main.c
>b/drivers/infiniband/sw/siw/siw_main.c
>> index fd2552a..2a70830d 100644
>> --- a/drivers/infiniband/sw/siw/siw_main.c
>> +++ b/drivers/infiniband/sw/siw/siw_main.c
>> @@ -101,7 +101,8 @@ static int siw_create_tx_threads(void)
>>  		if (IS_ERR(siw_tx_thread[cpu])) {
>>  			rv = PTR_ERR(siw_tx_thread[cpu]);
>>  			siw_tx_thread[cpu] = NULL;
>> -			pr_info("Creating TX thread for CPU %d failed", cpu);
>> +			pr_info("Creating TX thread for CPU%d failed %d\n",
>> +				cpu, rv);
>
>Delete this print together with variable, failure to create kthread
>is basic failure, which affect performance only. The whole kthread
>creation spam in this driver looked suspicious during submission
>and it continues to be.
>
>Thanks

Right, I agree with Leon. Better remove all those printouts. We
already have a warning if we cannot start any thread. Also
stopping those threads is not worth spamming the console. I just
forgot to remove after Leon's comment. Would it be possible
to apply the following?

Thanks a lot!
Bernard.

From e4ca3d4dec86bb5731f8e3cb0cdd01e84b315d80 Mon Sep 17 00:00:00 2001
From: Bernard Metzler <bmt@zurich.ibm.com>
Date: Wed, 10 Jul 2019 10:03:17 +0200
Subject: [PATCH] remove kthread create/destroy printouts

Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
---
 drivers/infiniband/sw/siw/siw_main.c  | 4 +---
 drivers/infiniband/sw/siw/siw_qp_tx.c | 4 ----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/infiniband/sw/siw/siw_main.c b/drivers/infiniband/sw/siw/siw_main.c
index fd2552a9091d..f55c4e80aea4 100644
--- a/drivers/infiniband/sw/siw/siw_main.c
+++ b/drivers/infiniband/sw/siw/siw_main.c
@@ -88,7 +88,7 @@ static void siw_device_cleanup(struct ib_device *base_dev)
 
 static int siw_create_tx_threads(void)
 {
-	int cpu, rv, assigned = 0;
+	int cpu, assigned = 0;
 
 	for_each_online_cpu(cpu) {
 		/* Skip HT cores */
@@ -99,9 +99,7 @@ static int siw_create_tx_threads(void)
 			kthread_create(siw_run_sq, (unsigned long *)(long)cpu,
 				       "siw_tx/%d", cpu);
 		if (IS_ERR(siw_tx_thread[cpu])) {
-			rv = PTR_ERR(siw_tx_thread[cpu]);
 			siw_tx_thread[cpu] = NULL;
-			pr_info("Creating TX thread for CPU %d failed", cpu);
 			continue;
 		}
 		kthread_bind(siw_tx_thread[cpu], cpu);
diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw/siw/siw_qp_tx.c
index 2c3d250ee57c..fff02b56d38a 100644
--- a/drivers/infiniband/sw/siw/siw_qp_tx.c
+++ b/drivers/infiniband/sw/siw/siw_qp_tx.c
@@ -1200,8 +1200,6 @@ int siw_run_sq(void *data)
 	init_llist_head(&tx_task->active);
 	init_waitqueue_head(&tx_task->waiting);
 
-	pr_info("Started siw TX thread on CPU %u\n", nr_cpu);
-
 	while (1) {
 		struct llist_node *fifo_list = NULL;
 
@@ -1239,8 +1237,6 @@ int siw_run_sq(void *data)
 			siw_sq_resume(qp);
 		}
 	}
-	pr_info("Stopped siw TX thread on CPU %u\n", nr_cpu);
-
 	return 0;
 }
 
-- 
2.17.2


 

Re: Re: [PATCH] RDMA/siw: Print error code while kthread_create failed

[Jason Gunthorpe]

On Wed, Jul 10, 2019 at 08:38:00AM +0000, Bernard Metzler wrote:

> Right, I agree with Leon. Better remove all those printouts. We
> already have a warning if we cannot start any thread. Also
> stopping those threads is not worth spamming the console. I just
> forgot to remove after Leon's comment. Would it be possible
> to apply the following?
> 
> Thanks a lot!
> Bernard.
> 
> From e4ca3d4dec86bb5731f8e3cb0cdd01e84b315d80 Mon Sep 17 00:00:00 2001
> From: Bernard Metzler <bmt@zurich.ibm.com>
> Date: Wed, 10 Jul 2019 10:03:17 +0200
> Subject: [PATCH] remove kthread create/destroy printouts
> 
> Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
>  drivers/infiniband/sw/siw/siw_main.c  | 4 +---
>  drivers/infiniband/sw/siw/siw_qp_tx.c | 4 ----
>  2 files changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/siw/siw_main.c b/drivers/infiniband/sw/siw/siw_main.c
> index fd2552a9091d..f55c4e80aea4 100644
> +++ b/drivers/infiniband/sw/siw/siw_main.c
> @@ -88,7 +88,7 @@ static void siw_device_cleanup(struct ib_device *base_dev)
>  
>  static int siw_create_tx_threads(void)
>  {
> -	int cpu, rv, assigned = 0;
> +	int cpu, assigned = 0;
>  
>  	for_each_online_cpu(cpu) {
>  		/* Skip HT cores */
> @@ -99,9 +99,7 @@ static int siw_create_tx_threads(void)
>  			kthread_create(siw_run_sq, (unsigned long *)(long)cpu,
>  				       "siw_tx/%d", cpu);
>  		if (IS_ERR(siw_tx_thread[cpu])) {
> -			rv = PTR_ERR(siw_tx_thread[cpu]);
>  			siw_tx_thread[cpu] = NULL;
> -			pr_info("Creating TX thread for CPU %d failed", cpu);
>  			continue;
>  		}
>  		kthread_bind(siw_tx_thread[cpu], cpu);
> diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw/siw/siw_qp_tx.c
> index 2c3d250ee57c..fff02b56d38a 100644
> +++ b/drivers/infiniband/sw/siw/siw_qp_tx.c
> @@ -1200,8 +1200,6 @@ int siw_run_sq(void *data)
>  	init_llist_head(&tx_task->active);
>  	init_waitqueue_head(&tx_task->waiting);
>  
> -	pr_info("Started siw TX thread on CPU %u\n", nr_cpu);
> -
>  	while (1) {
>  		struct llist_node *fifo_list = NULL;
>  
> @@ -1239,8 +1237,6 @@ int siw_run_sq(void *data)
>  			siw_sq_resume(qp);
>  		}
>  	}
> -	pr_info("Stopped siw TX thread on CPU %u\n", nr_cpu);
> -
>  	return 0;
>  }

Okay, I took this patch to for-next, thanks

Jason