2017-09-26 06:51:10

by Arvind Yadav

[permalink] [raw]
Subject: [PATCH] IB/mlx5:: pr_err() and mlx5_ib_dbg() strings should end with newlines

pr_err() and mlx5_ib_dbg( messages should terminated with a new-line to
avoid other messages being concatenated.

Signed-off-by: Arvind Yadav <[email protected]>
---
drivers/infiniband/hw/mlx5/mr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
index 0e2789d..92d643a 100644
--- a/drivers/infiniband/hw/mlx5/mr.c
+++ b/drivers/infiniband/hw/mlx5/mr.c
@@ -1229,13 +1229,13 @@ struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
mr = alloc_mr_from_cache(pd, umem, virt_addr, length, ncont,
page_shift, order, access_flags);
if (PTR_ERR(mr) == -EAGAIN) {
- mlx5_ib_dbg(dev, "cache empty for order %d", order);
+ mlx5_ib_dbg(dev, "cache empty for order %d\n", order);
mr = NULL;
}
} else if (!MLX5_CAP_GEN(dev->mdev, umr_extended_translation_offset)) {
if (access_flags & IB_ACCESS_ON_DEMAND) {
err = -EINVAL;
- pr_err("Got MR registration for ODP MR > 512MB, not supported for Connect-IB");
+ pr_err("Got MR registration for ODP MR > 512MB, not supported for Connect-IB\n");
goto error;
}
use_umr = false;
--
1.9.1


2017-09-26 07:08:16

by Yuval Shaia

[permalink] [raw]
Subject: Re: [PATCH] IB/mlx5:: pr_err() and mlx5_ib_dbg() strings should end with newlines

On Tue, Sep 26, 2017 at 12:20:01PM +0530, Arvind Yadav wrote:
> pr_err() and mlx5_ib_dbg( messages should terminated with a new-line to
> avoid other messages being concatenated.
>
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> drivers/infiniband/hw/mlx5/mr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> index 0e2789d..92d643a 100644
> --- a/drivers/infiniband/hw/mlx5/mr.c
> +++ b/drivers/infiniband/hw/mlx5/mr.c
> @@ -1229,13 +1229,13 @@ struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> mr = alloc_mr_from_cache(pd, umem, virt_addr, length, ncont,
> page_shift, order, access_flags);
> if (PTR_ERR(mr) == -EAGAIN) {
> - mlx5_ib_dbg(dev, "cache empty for order %d", order);
> + mlx5_ib_dbg(dev, "cache empty for order %d\n", order);
> mr = NULL;
> }
> } else if (!MLX5_CAP_GEN(dev->mdev, umr_extended_translation_offset)) {
> if (access_flags & IB_ACCESS_ON_DEMAND) {
> err = -EINVAL;
> - pr_err("Got MR registration for ODP MR > 512MB, not supported for Connect-IB");
> + pr_err("Got MR registration for ODP MR > 512MB, not supported for Connect-IB\n");
> goto error;
> }
> use_umr = false;
> --
> 1.9.1

FWIW,
Reviewed-by: Yuval Shaia <[email protected]>

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2017-09-26 08:39:00

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH] IB/mlx5:: pr_err() and mlx5_ib_dbg() strings should end with newlines

On Tue, Sep 26, 2017 at 12:20:01PM +0530, Arvind Yadav wrote:
> pr_err() and mlx5_ib_dbg( messages should terminated with a new-line to
> avoid other messages being concatenated.
>
> Signed-off-by: Arvind Yadav <[email protected]>
> ---
> drivers/infiniband/hw/mlx5/mr.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)

Did you see it is happening?
It is not needed after 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation lines")

Thanks.

>
> diff --git a/drivers/infiniband/hw/mlx5/mr.c b/drivers/infiniband/hw/mlx5/mr.c
> index 0e2789d..92d643a 100644
> --- a/drivers/infiniband/hw/mlx5/mr.c
> +++ b/drivers/infiniband/hw/mlx5/mr.c
> @@ -1229,13 +1229,13 @@ struct ib_mr *mlx5_ib_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> mr = alloc_mr_from_cache(pd, umem, virt_addr, length, ncont,
> page_shift, order, access_flags);
> if (PTR_ERR(mr) == -EAGAIN) {
> - mlx5_ib_dbg(dev, "cache empty for order %d", order);
> + mlx5_ib_dbg(dev, "cache empty for order %d\n", order);
> mr = NULL;
> }
> } else if (!MLX5_CAP_GEN(dev->mdev, umr_extended_translation_offset)) {
> if (access_flags & IB_ACCESS_ON_DEMAND) {
> err = -EINVAL;
> - pr_err("Got MR registration for ODP MR > 512MB, not supported for Connect-IB");
> + pr_err("Got MR registration for ODP MR > 512MB, not supported for Connect-IB\n");
> goto error;
> }
> use_umr = false;
> --
> 1.9.1
>


Attachments:
(No filename) (1.41 kB)
signature.asc (833.00 B)
Download all attachments

2017-09-26 15:05:44

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] IB/mlx5:: pr_err() and mlx5_ib_dbg() strings should end with newlines

On Tue, 2017-09-26 at 11:38 +0300, Leon Romanovsky wrote:
> On Tue, Sep 26, 2017 at 12:20:01PM +0530, Arvind Yadav wrote:
> > pr_err() and mlx5_ib_dbg( messages should terminated with a new-line to
> > avoid other messages being concatenated.
[]
> Did you see it is happening?
> It is not needed after 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation lines")

Not completely true, and that commit message
itself is incorrect about how KERN_CONT was
ever required or not required.

After that commit, the dmesg output will
eventually be corrected with an appended newline
for sine line format strings without them, but
the printk subsystem has to wait for another
printk to occur before inserting that newline.

The commit message bit that says:

Things get much hairier when you have
multiple threads going on and user level
reading and writing logs too

is correct. That's the actual reason that
the proposed newline additions are reasonable.

2017-09-26 15:50:19

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH] IB/mlx5:: pr_err() and mlx5_ib_dbg() strings should end with newlines

On Tue, Sep 26, 2017 at 08:05:37AM -0700, Joe Perches wrote:
> On Tue, 2017-09-26 at 11:38 +0300, Leon Romanovsky wrote:
> > On Tue, Sep 26, 2017 at 12:20:01PM +0530, Arvind Yadav wrote:
> > > pr_err() and mlx5_ib_dbg( messages should terminated with a new-line to
> > > avoid other messages being concatenated.
> []
> > Did you see it is happening?
> > It is not needed after 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation lines")
>
> Not completely true, and that commit message
> itself is incorrect about how KERN_CONT was
> ever required or not required.
>
> After that commit, the dmesg output will
> eventually be corrected with an appended newline
> for sine line format strings without them, but
> the printk subsystem has to wait for another
> printk to occur before inserting that newline.
>
> The commit message bit that says:
>
> Things get much hairier when you have
> multiple threads going on and user level
> reading and writing logs too
>
> is correct. That's the actual reason that
> the proposed newline additions are reasonable.
>

I asked that question after I tried locally various different options
with/without newline and didn't see any difference.

So how can I reproduce the different output before and after this change?

Thanks


Attachments:
(No filename) (1.26 kB)
signature.asc (833.00 B)
Download all attachments

2017-09-26 19:11:54

by Joe Perches

[permalink] [raw]
Subject: Re: [PATCH] IB/mlx5:: pr_err() and mlx5_ib_dbg() strings should end with newlines

On Tue, 2017-09-26 at 18:50 +0300, Leon Romanovsky wrote:
> So how can I reproduce the different output before and after this change?

Try lib/test_module.c with and without the newline
on "Hello, World" on a quiescent system.

2017-09-27 14:20:43

by Doug Ledford

[permalink] [raw]
Subject: Re: [PATCH] IB/mlx5:: pr_err() and mlx5_ib_dbg() strings should end with newlines

On Tue, 2017-09-26 at 12:11 -0700, Joe Perches wrote:
> On Tue, 2017-09-26 at 18:50 +0300, Leon Romanovsky wrote:
> > So how can I reproduce the different output before and after this
> > change?
>
> Try lib/test_module.c with and without the newline
> on "Hello, World" on a quiescent system.

I agree here. The newlines are still worthwhile as they remove any
unnecessary stalls in the printk output while the printk engine decides
whether or not you are going to do a KERN_CONT print next.

Thanks, applied.

--
Doug Ledford <[email protected]>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD

2017-09-27 14:46:55

by Leon Romanovsky

[permalink] [raw]
Subject: Re: [PATCH] IB/mlx5:: pr_err() and mlx5_ib_dbg() strings should end with newlines

On Wed, Sep 27, 2017 at 10:20:39AM -0400, Doug Ledford wrote:
> On Tue, 2017-09-26 at 12:11 -0700, Joe Perches wrote:
> > On Tue, 2017-09-26 at 18:50 +0300, Leon Romanovsky wrote:
> > > So how can I reproduce the different output before and after this
> > > change?
> >
> > Try lib/test_module.c with and without the newline
> > on "Hello, World" on a quiescent system.
>
> I agree here. The newlines are still worthwhile as they remove any
> unnecessary stalls in the printk output while the printk engine decides
> whether or not you are going to do a KERN_CONT print next.
>
> Thanks, applied.

Thanks

>
> --
> Doug Ledford <[email protected]>
> GPG KeyID: B826A3330E572FDD
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
>


Attachments:
(No filename) (762.00 B)
signature.asc (833.00 B)
Download all attachments