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

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

[Arvind Yadav]

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

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 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

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

[Yuval Shaia]

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 <arvind.yadav.cs@gmail.com>
> ---
>  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 <yuval.shaia@oracle.com>

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

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

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1444 bytes --]

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 <arvind.yadav.cs@gmail.com>
> ---
>  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
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Joe Perches]

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.

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

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1293 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Joe Perches]

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.

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

[Doug Ledford]

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 <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

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

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 762 bytes --]

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 <dledford@redhat.com>
>     GPG KeyID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]