| Message ID | 20190710015009.57120-1-yuehaibing@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RDMA/siw: Print error code while kthread_create failed | expand |
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 > >
-----"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; }
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
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);
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(-)