A newline help flushing message out.
Signed-off-by: Li Zhijian <[email protected]>
---
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
rxe_set_mtu() will call rxe_info_dev() to print message, and
rxe_info_dev() expects dev_name(rxe->ib_dev->dev) has been assigned.
Previously since dev_name() is not set, when a new rxe link is being
added, 'null' will be used as the dev_name like:
"(null): rxe_set_mtu: Set mtu to 1024"
Move rxe_register_device() earlier to assign the correct dev_name
so that it can be read by rxe_set_mtu() later.
And it's safe to do such change since mtu will not be used during the
rxe_register_device()
After this change, message becomes:
"rxe_eth0: rxe_set_mtu: Set mtu to 4096"
Signed-off-by: Li Zhijian <[email protected]>
---
drivers/infiniband/sw/rxe/rxe.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/sw/rxe/rxe.c b/drivers/infiniband/sw/rxe/rxe.c
index cb2c0d54aae1..d29efce6e5ca 100644
--- a/drivers/infiniband/sw/rxe/rxe.c
+++ b/drivers/infiniband/sw/rxe/rxe.c
@@ -169,10 +169,13 @@ void rxe_set_mtu(struct rxe_dev *rxe, unsigned int ndev_mtu)
*/
int rxe_add(struct rxe_dev *rxe, unsigned int mtu, const char *ibdev_name)
{
+ int ret;
+
rxe_init(rxe);
+ ret = rxe_register_device(rxe, ibdev_name);
rxe_set_mtu(rxe, mtu);
- return rxe_register_device(rxe, ibdev_name);
+ return ret;
}
static int rxe_newlink(const char *ibdev_name, struct net_device *ndev)
--
2.29.2
On 23/08/2023 14:35, Zhu Yanjun wrote:
> On Wed, Aug 23, 2023 at 2:25 PM Zhijian Li (Fujitsu)
> <[email protected]> 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 <[email protected]>
> Sergey Senozhatsky <[email protected]>
> Steven Rostedt <[email protected]>
> John Ogness <[email protected]>
>
> 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 <[email protected]>
>>>> ---
>>>> 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 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 <[email protected]>
> >> ---
> >> 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 <[email protected]>
> >
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 <[email protected]>
>> ---
>> 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)
>>> <[email protected]> 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 <[email protected]>
>>> Sergey Senozhatsky <[email protected]>
>>> Steven Rostedt <[email protected]>
>>> John Ogness <[email protected]>
>>>
>>> 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 <[email protected]>
>>>>>> ---
>>>>>> 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.
>>>> >
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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
On Wed, Aug 23, 2023 11:13 AM Li Zhijian wrote:
>
> A newline help flushing message out.
>
> Signed-off-by: Li Zhijian <[email protected]>
> ---
> 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 <[email protected]>