| Message ID | 20230823021306.170901-1-lizhijian@fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] RDMA/rxe: add missing newline to rxe_set_mtu message | expand |
On Wed, Aug 23, 2023 11:13 AM Li Zhijian wrote: > > A newline help flushing message out. > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > drivers/infiniband/sw/rxe/rxe.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c > index 54c723a6edda..cb2c0d54aae1 100644 > --- a/drivers/infiniband/sw/rxe/rxe.c > +++ b/drivers/infiniband/sw/rxe/rxe.c > @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) > port->attr.active_mtu = mtu; > port->mtu_cap = ib_mtu_enum_to_int(mtu); > > - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); > + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); > } > > /* called by ifc layer to create new rxe device. > -- > 2.29.2 Reviewed-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com>
Daisuke Thanks for your reviewing, beside this place, i just noticed that there are so many places missing newline in rxe driver except rxe_err() rxe_info() rxe_dbg() already auto added newline in the macros, other macros should append newline by hand when being used. A roughly count: # git grep -n -e rxe_info -e rxe_err -e rxe_dbg | grep -v '#define' | grep -v '\\n' | wc -l 146 i will do a big cleanup for all of them later. Thanks Zhijian On 23/08/2023 10:43, Matsuda, Daisuke/松田 大輔 wrote: > On Wed, Aug 23, 2023 11:13 AM Li Zhijian wrote: >> >> A newline help flushing message out. >> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> drivers/infiniband/sw/rxe/rxe.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c >> index 54c723a6edda..cb2c0d54aae1 100644 >> --- a/drivers/infiniband/sw/rxe/rxe.c >> +++ b/drivers/infiniband/sw/rxe/rxe.c >> @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) >> port->attr.active_mtu = mtu; >> port->mtu_cap = ib_mtu_enum_to_int(mtu); >> >> - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); >> + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); >> } >> >> /* called by ifc layer to create new rxe device. >> -- >> 2.29.2 > > Reviewed-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com> >
On Wed, Aug 23, 2023 12:05 PM Li, Zhijian wrote: > > Daisuke > > > Thanks for your reviewing, beside this place, i just noticed that > there are so many places missing newline in rxe driver > > except rxe_err() rxe_info() rxe_dbg() already auto added newline in the macros, These built-in newline characters should be removed. In the kernel, "*fmt" is expected to include a newline character. We should remove '\n' from the definitions of these functions, and add '\n' to each caller. > other macros should append newline by hand when being used. Yes, they also need to be fixed. Daisuke > > A roughly count: > # git grep -n -e rxe_info -e rxe_err -e rxe_dbg | grep -v '#define' | grep -v '\\n' | wc -l > 146 > > > i will do a big cleanup for all of them later. > > Thanks > Zhijian > > > > On 23/08/2023 10:43, Matsuda, Daisuke/松田 大輔 wrote: > > On Wed, Aug 23, 2023 11:13 AM Li Zhijian wrote: > >> > >> A newline help flushing message out. > >> > >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > >> --- > >> drivers/infiniband/sw/rxe/rxe.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c > >> index 54c723a6edda..cb2c0d54aae1 100644 > >> --- a/drivers/infiniband/sw/rxe/rxe.c > >> +++ b/drivers/infiniband/sw/rxe/rxe.c > >> @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) > >> port->attr.active_mtu = mtu; > >> port->mtu_cap = ib_mtu_enum_to_int(mtu); > >> > >> - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); > >> + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); > >> } > >> > >> /* called by ifc layer to create new rxe device. > >> -- > >> 2.29.2 > > > > Reviewed-by: Daisuke Matsuda <matsuda-daisuke@fujitsu.com> > >
在 2023/8/23 10:13, Li Zhijian 写道: > A newline help flushing message out. rxe_info_dev will finally call printk to output information. In this link https://github.com/torvalds/linux/blob/master/Documentation/core-api/printk-basics.rst, " All printk() messages are printed to the kernel log buffer, which is a ring buffer exported to userspace through /dev/kmsg. The usual way to read it is using dmesg. " Do you mean that a new line will help the kernel log buffer flush message out? Zhu Yanjun > > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > drivers/infiniband/sw/rxe/rxe.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c > index 54c723a6edda..cb2c0d54aae1 100644 > --- a/drivers/infiniband/sw/rxe/rxe.c > +++ b/drivers/infiniband/sw/rxe/rxe.c > @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) > port->attr.active_mtu = mtu; > port->mtu_cap = ib_mtu_enum_to_int(mtu); > > - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); > + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); > } > > /* called by ifc layer to create new rxe device.
On 23/08/2023 14:12, Zhu Yanjun wrote: > 在 2023/8/23 10:13, Li Zhijian 写道: >> A newline help flushing message out. > > rxe_info_dev will finally call printk to output information. > > In this link https://github.com/torvalds/linux/blob/master/Documentation/core-api/printk-basics.rst, > " > All printk() messages are printed to the kernel log buffer, which is a ring buffer exported to userspace through /dev/kmsg. The usual way to read it is using dmesg. > " > Do you mean that a new line will help the kernel log buffer flush message out? Yeah, the message will be buffered until it is full or it meets a newline. > > Zhu Yanjun >> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> drivers/infiniband/sw/rxe/rxe.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c >> index 54c723a6edda..cb2c0d54aae1 100644 >> --- a/drivers/infiniband/sw/rxe/rxe.c >> +++ b/drivers/infiniband/sw/rxe/rxe.c >> @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) >> port->attr.active_mtu = mtu; >> port->mtu_cap = ib_mtu_enum_to_int(mtu); >> - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); >> + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); >> } >> /* called by ifc layer to create new rxe device. >
On Wed, Aug 23, 2023 at 2:25 PM Zhijian Li (Fujitsu) <lizhijian@fujitsu.com> wrote: > > > > On 23/08/2023 14:12, Zhu Yanjun wrote: > > 在 2023/8/23 10:13, Li Zhijian 写道: > >> A newline help flushing message out. > > > > rxe_info_dev will finally call printk to output information. > > > > In this link https://github.com/torvalds/linux/blob/master/Documentation/core-api/printk-basics.rst, > > " > > All printk() messages are printed to the kernel log buffer, which is a ring buffer exported to userspace through /dev/kmsg. The usual way to read it is using dmesg. > > " > > Do you mean that a new line will help the kernel log buffer flush message out? > > Yeah, the message will be buffered until it is full or it meets a newline. Add PRINTK reviewers: Petr Mladek <pmladek@suse.com> Sergey Senozhatsky <senozhatsky@chromium.org> Steven Rostedt <rostedt@goodmis.org> John Ogness <john.ogness@linutronix.de> This is about printk. They can decide this commit. Zhu Yanjun > > > > > > > Zhu Yanjun > >> > >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > >> --- > >> drivers/infiniband/sw/rxe/rxe.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c > >> index 54c723a6edda..cb2c0d54aae1 100644 > >> --- a/drivers/infiniband/sw/rxe/rxe.c > >> +++ b/drivers/infiniband/sw/rxe/rxe.c > >> @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) > >> port->attr.active_mtu = mtu; > >> port->mtu_cap = ib_mtu_enum_to_int(mtu); > >> - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); > >> + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); > >> } > >> /* called by ifc layer to create new rxe device. > >
On 23/08/2023 14:35, Zhu Yanjun wrote: > On Wed, Aug 23, 2023 at 2:25 PM Zhijian Li (Fujitsu) > <lizhijian@fujitsu.com> wrote: >> >> >> >> On 23/08/2023 14:12, Zhu Yanjun wrote: >>> 在 2023/8/23 10:13, Li Zhijian 写道: >>>> A newline help flushing message out. >>> >>> rxe_info_dev will finally call printk to output information. >>> >>> In this link https://github.com/torvalds/linux/blob/master/Documentation/core-api/printk-basics.rst, >>> " >>> All printk() messages are printed to the kernel log buffer, which is a ring buffer exported to userspace through /dev/kmsg. The usual way to read it is using dmesg. >>> " >>> Do you mean that a new line will help the kernel log buffer flush message out? >> >> Yeah, the message will be buffered until it is full or it meets a newline. > > Add PRINTK reviewers: > > Petr Mladek <pmladek@suse.com> > Sergey Senozhatsky <senozhatsky@chromium.org> > Steven Rostedt <rostedt@goodmis.org> > John Ogness <john.ogness@linutronix.de> > > This is about printk. They can decide this commit. I don't think it's a printk stuff. In general, when developers add some printk()/pr_info() to print some message in the kernel, they expect this message will be printed in time. So most of the printk()/pr_info() calls in current kernel accompany a '\n' at the end. And printk() will also print message to 'console' by default, console could be a serial port(ttyS0) or tty1 etc. Thanks Zhijian > > Zhu Yanjun > >> >> >> >>> >>> Zhu Yanjun >>>> >>>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >>>> --- >>>> drivers/infiniband/sw/rxe/rxe.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c >>>> index 54c723a6edda..cb2c0d54aae1 100644 >>>> --- a/drivers/infiniband/sw/rxe/rxe.c >>>> +++ b/drivers/infiniband/sw/rxe/rxe.c >>>> @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) >>>> port->attr.active_mtu = mtu; >>>> port->mtu_cap = ib_mtu_enum_to_int(mtu); >>>> - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); >>>> + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); >>>> } >>>> /* called by ifc layer to create new rxe device. >>>
在 2023/8/23 14:47, Zhijian Li (Fujitsu) 写道: > > On 23/08/2023 14:35, Zhu Yanjun wrote: >> On Wed, Aug 23, 2023 at 2:25 PM Zhijian Li (Fujitsu) >> <lizhijian@fujitsu.com> wrote: >>> >>> >>> On 23/08/2023 14:12, Zhu Yanjun wrote: >>>> 在 2023/8/23 10:13, Li Zhijian 写道: >>>>> A newline help flushing message out. >>>> rxe_info_dev will finally call printk to output information. >>>> >>>> In this link https://github.com/torvalds/linux/blob/master/Documentation/core-api/printk-basics.rst, >>>> " >>>> All printk() messages are printed to the kernel log buffer, which is a ring buffer exported to userspace through /dev/kmsg. The usual way to read it is using dmesg. >>>> " >>>> Do you mean that a new line will help the kernel log buffer flush message out? >>> Yeah, the message will be buffered until it is full or it meets a newline. >> Add PRINTK reviewers: >> >> Petr Mladek <pmladek@suse.com> >> Sergey Senozhatsky <senozhatsky@chromium.org> >> Steven Rostedt <rostedt@goodmis.org> >> John Ogness <john.ogness@linutronix.de> >> >> This is about printk. They can decide this commit. > I don't think it's a printk stuff. Do you get me? I mean, prinkt reviewer will check the statement "the message will be buffered until it is full or it meets a newline." correct or not. Zhu Yanjun > > In general, when developers add some printk()/pr_info() to print some message in the kernel, they expect this message will be printed in time. > So most of the printk()/pr_info() calls in current kernel accompany a '\n' at the end. > > And printk() will also print message to 'console' by default, console could be a serial port(ttyS0) or tty1 etc. > > Thanks > Zhijian > > >> Zhu Yanjun >> >>> >>> >>>> Zhu Yanjun >>>>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >>>>> --- >>>>> drivers/infiniband/sw/rxe/rxe.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c >>>>> index 54c723a6edda..cb2c0d54aae1 100644 >>>>> --- a/drivers/infiniband/sw/rxe/rxe.c >>>>> +++ b/drivers/infiniband/sw/rxe/rxe.c >>>>> @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) >>>>> port->attr.active_mtu = mtu; >>>>> port->mtu_cap = ib_mtu_enum_to_int(mtu); >>>>> - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); >>>>> + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); >>>>> } >>>>> /* called by ifc layer to create new rxe device. >>> >
On 23/08/2023 14:59, Zhu Yanjun wrote: > > 在 2023/8/23 14:47, Zhijian Li (Fujitsu) 写道: >> >> On 23/08/2023 14:35, Zhu Yanjun wrote: >>> On Wed, Aug 23, 2023 at 2:25 PM Zhijian Li (Fujitsu) >>> <lizhijian@fujitsu.com> wrote: >>>> >>>> >>>> On 23/08/2023 14:12, Zhu Yanjun wrote: >>>>> 在 2023/8/23 10:13, Li Zhijian 写道: >>>>>> A newline help flushing message out. >>>>> rxe_info_dev will finally call printk to output information. >>>>> >>>>> In this link https://github.com/torvalds/linux/blob/master/Documentation/core-api/printk-basics.rst, >>>>> " >>>>> All printk() messages are printed to the kernel log buffer, which is a ring buffer exported to userspace through /dev/kmsg. The usual way to read it is using dmesg. >>>>> " >>>>> Do you mean that a new line will help the kernel log buffer flush message out? >>>> Yeah, the message will be buffered until it is full or it meets a newline. >>> Add PRINTK reviewers: >>> >>> Petr Mladek <pmladek@suse.com> >>> Sergey Senozhatsky <senozhatsky@chromium.org> >>> Steven Rostedt <rostedt@goodmis.org> >>> John Ogness <john.ogness@linutronix.de> >>> >>> This is about printk. They can decide this commit. >> I don't think it's a printk stuff. > Do you get me? > > I mean, prinkt reviewer will check the statement "the message will be buffered until it is full or it meets a newline." correct or not. I'm sorry, i get confused. I thought you were talking about "this commit" And i have post 2nd revision, please take a look. [PATCH v2 1/2] RDMA/rxe: Improve newline in printing messages Thanks Zhijian > > Zhu Yanjun > >> >> In general, when developers add some printk()/pr_info() to print some message in the kernel, they expect this message will be printed in time. >> So most of the printk()/pr_info() calls in current kernel accompany a '\n' at the end. >> >> And printk() will also print message to 'console' by default, console could be a serial port(ttyS0) or tty1 etc. >> >> Thanks >> Zhijian >> >> >>> Zhu Yanjun >>> >>>> >>>> >>>>> Zhu Yanjun >>>>>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >>>>>> --- >>>>>> drivers/infiniband/sw/rxe/rxe.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c >>>>>> index 54c723a6edda..cb2c0d54aae1 100644 >>>>>> --- a/drivers/infiniband/sw/rxe/rxe.c >>>>>> +++ b/drivers/infiniband/sw/rxe/rxe.c >>>>>> @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) >>>>>> port->attr.active_mtu = mtu; >>>>>> port->mtu_cap = ib_mtu_enum_to_int(mtu); >>>>>> - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); >>>>>> + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); >>>>>> } >>>>>> /* called by ifc layer to create new rxe device. >>>> >
diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c index 54c723a6edda..cb2c0d54aae1 100644 --- a/drivers/infiniband/sw/rxe/rxe.c +++ b/drivers/infiniband/sw/rxe/rxe.c @@ -161,7 +161,7 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu) port->attr.active_mtu = mtu; port->mtu_cap = ib_mtu_enum_to_int(mtu); - rxe_info_dev(rxe, "Set mtu to %d", port->mtu_cap); + rxe_info_dev(rxe, "Set mtu to %d\n", port->mtu_cap); } /* called by ifc layer to create new rxe device.
A newline help flushing message out. Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> --- drivers/infiniband/sw/rxe/rxe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)