[PATCH] mellanox: mlx5: Use logging functions to reduce text ~10k/5%

Subsystems: mellanox mlx5 core vpi driver, networking drivers, the rest

STALE3702d

7 messages, 4 authors, 2016-06-23 · open the first message on its own page

[PATCH] mellanox: mlx5: Use logging functions to reduce text ~10k/5%

From: Joe Perches <joe@perches.com>
Date: 2016-06-22 18:24:06

The logging macros create a bit of duplicated code/text.

Use specialized functions to reduce the duplication.

(defconfig/x86-64)
$ size drivers/net/ethernet/mellanox/mlx5/core/built-in.o*
   text	   data	    bss	    dec	    hex	filename
 178634	   2059	     16	 180709	  2c1e5	drivers/net/ethernet/mellanox/mlx5/core/built-in.o.new
 188679	   2059	     16	 190754	  2e922	drivers/net/ethernet/mellanox/mlx5/core/built-in.o.old

The output changes now do not include line #,
but do include the function offset.

Signed-off-by: Joe Perches <redacted>
---
 drivers/net/ethernet/mellanox/mlx5/core/main.c     | 34 ++++++++++++++++++++++
 .../net/ethernet/mellanox/mlx5/core/mlx5_core.h    | 13 +++------
 2 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index a19b593..34cbaf0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c

@@ -1557,3 +1557,37 @@ static void __exit cleanup(void)

 
 module_init(init);
 module_exit(cleanup);
+
+void mlx5_core_err(struct mlx5_core_dev *dev, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	dev_err(&dev->pdev->dev, "%s:%pS:(pid %d): %pV",
+		dev->priv.name, __builtin_return_address(0), current->pid,
+		&vaf);
+
+	va_end(args);
+}
+
+void mlx5_core_warn(struct mlx5_core_dev *dev, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	dev_warn(&dev->pdev->dev, "%s:%pS:(pid %d): %pV",
+		 dev->priv.name, __builtin_return_address(0), current->pid,
+		 &vaf);
+
+	va_end(args);
+}

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
index 2f86ec6..31430a7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h

@@ -57,15 +57,10 @@ do {									\

 		mlx5_core_dbg(__dev, format, ##__VA_ARGS__);		\
 } while (0)
 
-#define mlx5_core_err(__dev, format, ...)				\
-	dev_err(&(__dev)->pdev->dev, "%s:%s:%d:(pid %d): " format,	\
-	       (__dev)->priv.name, __func__, __LINE__, current->pid,	\
-	       ##__VA_ARGS__)
-
-#define mlx5_core_warn(__dev, format, ...)				\
-	dev_warn(&(__dev)->pdev->dev, "%s:%s:%d:(pid %d): " format,	\
-		(__dev)->priv.name, __func__, __LINE__, current->pid,	\
-		##__VA_ARGS__)
+__printf(2, 3)
+void mlx5_core_err(struct mlx5_core_dev *dev, const char *fmt, ...);
+__printf(2, 3)
+void mlx5_core_warn(struct mlx5_core_dev *dev, const char *fmt, ...);
 
 #define mlx5_core_info(__dev, format, ...)				\
 	dev_info(&(__dev)->pdev->dev, format, ##__VA_ARGS__)


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

Re: [PATCH] mellanox: mlx5: Use logging functions to reduce text ~10k/5%

From: Jason Gunthorpe <hidden>
Date: 2016-06-22 20:41:04

On Wed, Jun 22, 2016 at 11:23:59AM -0700, Joe Perches wrote:

The output changes now do not include line #, but do include the
function offset.

I've been using a technique like this in some code with good results:

struct source_location
{
   const char *file;
   const char *func;
   const char *format;
   uint16_t line;
};
#define _LOCATION(format) ({static const source_location __location__ = {\
             __FILE__,__PRETTY_FUNCTION__,format,__LINE__};\
	     &__location__;})

void _mlx5_core_err(const struct source_location *loc,struct mlx5_core_dev *dev, ...);
#define mlx5_core_err(dev,format,...) _mlx_core_err(_LOCATION(format),dev,__VA_ARGS__)

The call site .text overhead is the about same as what you have, but
this still retains the function and line number information in
.rodata.

Jason

Re: [PATCH] mellanox: mlx5: Use logging functions to reduce text ~10k/5%

From: Joe Perches <joe@perches.com>
Date: 2016-06-22 22:20:48

On Wed, 2016-06-22 at 14:40 -0600, Jason Gunthorpe wrote:

On Wed, Jun 22, 2016 at 11:23:59AM -0700, Joe Perches wrote:

quoted

The output changes now do not include line #, but do include the
function offset.

I've been using a technique like this in some code with good results:

struct source_location
{
   const char *file;
   const char *func;
   const char *format;
   uint16_t line;
};
#define _LOCATION(format) ({static const source_location __location__
= {\
             __FILE__,__PRETTY_FUNCTION__,format,__LINE__};\
	     &__location__;})

void _mlx5_core_err(const struct source_location *loc,struct
mlx5_core_dev *dev, ...);
#define mlx5_core_err(dev,format,...)
_mlx_core_err(_LOCATION(format),dev,__VA_ARGS__)

The call site .text overhead is the about same as what you have, but
this still retains the function and line number information in
.rodata.

Hello Jason.

As far as I know, no kernel code currently uses a _LOCATION
like macro.

I think your proposal is nearly identical code size to the
existing call.  Also, compiler format/argument checking is
eliminated and I think that is a significant negative.

Using the kernel vsprintf %pS or %ps extension is pretty common.

Using printk("%pS", __builtin_return_address(0)); in the called
function is no overhead at all and returns almost exactly
the same information.

Using more expressive messages is generally better than using
printk("%d", __LINE__);

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

Re: [PATCH] mellanox: mlx5: Use logging functions to reduce text ~10k/5%

From: Leon Romanovsky <hidden>
Date: 2016-06-23 07:00:51

On Wed, Jun 22, 2016 at 11:23:59AM -0700, Joe Perches wrote:

The logging macros create a bit of duplicated code/text.

Use specialized functions to reduce the duplication.

(defconfig/x86-64)
$ size drivers/net/ethernet/mellanox/mlx5/core/built-in.o*
   text	   data	    bss	    dec	    hex	filename
 178634	   2059	     16	 180709	  2c1e5	drivers/net/ethernet/mellanox/mlx5/core/built-in.o.new
 188679	   2059	     16	 190754	  2e922	drivers/net/ethernet/mellanox/mlx5/core/built-in.o.old

The output changes now do not include line #,
but do include the function offset.

Signed-off-by: Joe Perches <redacted>

As far as I see all these functions are used in error paths, so no
implication on performance is expected.

And I'm fine with function offsets.

Saeed,
What do you think?

Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

Re: [PATCH] mellanox: mlx5: Use logging functions to reduce text ~10k/5%

From: Leon Romanovsky <hidden>
Date: 2016-06-23 07:12:22

On Thu, Jun 23, 2016 at 08:27:01AM +0300, Leon Romanovsky wrote:

On Wed, Jun 22, 2016 at 11:23:59AM -0700, Joe Perches wrote:

quoted

The logging macros create a bit of duplicated code/text.

Use specialized functions to reduce the duplication.

(defconfig/x86-64)
$ size drivers/net/ethernet/mellanox/mlx5/core/built-in.o*
   text	   data	    bss	    dec	    hex	filename
 178634	   2059	     16	 180709	  2c1e5	drivers/net/ethernet/mellanox/mlx5/core/built-in.o.new
 188679	   2059	     16	 190754	  2e922	drivers/net/ethernet/mellanox/mlx5/core/built-in.o.old

The output changes now do not include line #,
but do include the function offset.

Signed-off-by: Joe Perches <redacted>

As far as I see all these functions are used in error paths, so no
implication on performance is expected.

And I'm fine with function offsets.

Saeed,
What do you think?

Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

I continued to play with this patch and it doesn't pass checkpatch.
It looks like corrupted file.

➜  linux-rdma git:(master) ./scripts/checkpatch.pl
~/Downloads/mellanox-mlx5-Use-logging-functions-to-reduce-text-10k-5.patch
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#21: 
 178634    2059      16  180709   2c1e5
drivers/net/ethernet/mellanox/mlx5/core/built-in.o.new

ERROR: patch seems to be corrupt (line wrapped?)
#46: FILE: drivers/net/ethernet/mellanox/mlx5/core/main.c:1556:

CHECK: Alignment should match open parenthesis
#78: FILE: drivers/net/ethernet/mellanox/mlx5/core/main.c:1586:
+       dev_warn(&dev->pdev->dev, "%s:%pS:(pid %d): %pV",
+                dev->priv.name, __builtin_return_address(0), current->pid,

ERROR: space required before that '&' (ctx:VxV)
#79: FILE: drivers/net/ethernet/mellanox/mlx5/core/main.c:1587:
+                &vaf);
                  ^
total: 2 errors, 1 warnings, 1 checks, 58 lines  checked

Re: [PATCH] mellanox: mlx5: Use logging functions to reduce text ~10k/5%

From: Saeed Mahameed <hidden>
Date: 2016-06-23 08:10:25

On Wed, Jun 22, 2016 at 9:23 PM, Joe Perches [off-list ref] wrote:
[...]

quoted hunk

--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c

@@ -1557,3 +1557,37 @@ static void __exit cleanup(void)

 module_init(init);
 module_exit(cleanup);
+
+void mlx5_core_err(struct mlx5_core_dev *dev, const char *fmt, ...)
+{
+       struct va_format vaf;
+       va_list args;
+
+       va_start(args, fmt);
+
+       vaf.fmt = fmt;
+       vaf.va = &args;
+
+       dev_err(&dev->pdev->dev, "%s:%pS:(pid %d): %pV",
+               dev->priv.name, __builtin_return_address(0), current->pid,
+               &vaf);
+
+       va_end(args);
+}
+
+void mlx5_core_warn(struct mlx5_core_dev *dev, const char *fmt, ...)
+{
+       struct va_format vaf;
+       va_list args;
+
+       va_start(args, fmt);
+
+       vaf.fmt = fmt;
+       vaf.va = &args;
+
+       dev_warn(&dev->pdev->dev, "%s:%pS:(pid %d): %pV",
+                dev->priv.name, __builtin_return_address(0), current->pid,
+                &vaf);
+
+       va_end(args);
+}

Hi Joe,

I like to keep the file organized in a bottom-up fashion.  Those
functions need to appear as early as possible in the file, just move
them up to appear after the MACROs defines and static fields
declarations.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mellanox: mlx5: Use logging functions to reduce text ~10k/5%

From: Saeed Mahameed <hidden>
Date: 2016-06-23 08:13:16

On Thu, Jun 23, 2016 at 8:27 AM, Leon Romanovsky [off-list ref] wrote:

On Wed, Jun 22, 2016 at 11:23:59AM -0700, Joe Perches wrote:

quoted

The logging macros create a bit of duplicated code/text.

Use specialized functions to reduce the duplication.

(defconfig/x86-64)
$ size drivers/net/ethernet/mellanox/mlx5/core/built-in.o*
   text          data     bss     dec     hex filename
 178634          2059      16  180709   2c1e5 drivers/net/ethernet/mellanox/mlx5/core/built-in.o.new
 188679          2059      16  190754   2e922 drivers/net/ethernet/mellanox/mlx5/core/built-in.o.old

The output changes now do not include line #,
but do include the function offset.

Signed-off-by: Joe Perches <redacted>

As far as I see all these functions are used in error paths, so no
implication on performance is expected.

And I'm fine with function offsets.

Saeed,
What do you think?

Fine with me, need to fix my comment on functions placement, an your
comment on checkpatch.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help