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 <[email protected]>
Signed-off-by: YueHaibing <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: YueHaibing <[email protected]>
> ---
> 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
>
>
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 <[email protected]>
Suggested-by: Leon Romanovsky <[email protected]>
Signed-off-by: YueHaibing <[email protected]>
---
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
-----"Leon Romanovsky" <[email protected]> wrote: -----
>To: "YueHaibing" <[email protected]>
>From: "Leon Romanovsky" <[email protected]>
>Date: 07/10/2019 06:36AM
>Cc: [email protected], [email protected], [email protected],
>[email protected], [email protected]
>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 <[email protected]>
>> Signed-off-by: YueHaibing <[email protected]>
>> ---
>> 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 <[email protected]>
Date: Wed, 10 Jul 2019 10:03:17 +0200
Subject: [PATCH] remove kthread create/destroy printouts
Signed-off-by: Bernard Metzler <[email protected]>
---
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
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 <[email protected]>
> Date: Wed, 10 Jul 2019 10:03:17 +0200
> Subject: [PATCH] remove kthread create/destroy printouts
>
> Signed-off-by: Bernard Metzler <[email protected]>
> 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