* [PATCH] qed: Add and use specific logging functions to reduce object size
@ 2016-07-26 21:25 Joe Perches
2016-07-27 1:24 ` Joe Perches
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Joe Perches @ 2016-07-26 21:25 UTC (permalink / raw)
To: Yuval Mintz, Ariel Elior, everest-linux-l2; +Cc: linux-kernel, netdev
Current DP_<LEVEL> macros generate a lot of code.
Using functions with vsprintf extension %pV helps reduce that size.
$ size drivers/net/ethernet/qlogic/built-in.o* (x86-64)
text data bss dec hex filename
165161 28470 32812 226443 3748b drivers/net/ethernet/qlogic/built-in.o.defconfig.new
190473 28470 32812 251755 3d76b drivers/net/ethernet/qlogic/built-in.o.defconfig.old
1215984 257822 39712 1513518 17182e drivers/net/ethernet/qlogic/built-in.o.allyesconfig.new
1262402 284334 39712 1586448 183510 drivers/net/ethernet/qlogic/built-in.o.allyesconfig.old
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/ethernet/qlogic/qed/Makefile | 2 +-
drivers/net/ethernet/qlogic/qed/qed_util.c | 82 ++++++++++++++++++++++++++++++
include/linux/qed/qed_if.h | 60 +++++++++-------------
3 files changed, 106 insertions(+), 38 deletions(-)
create mode 100644 drivers/net/ethernet/qlogic/qed/qed_util.c
diff --git a/drivers/net/ethernet/qlogic/qed/Makefile b/drivers/net/ethernet/qlogic/qed/Makefile
index d1f157e..9d7e55f 100644
--- a/drivers/net/ethernet/qlogic/qed/Makefile
+++ b/drivers/net/ethernet/qlogic/qed/Makefile
@@ -2,5 +2,5 @@ obj-$(CONFIG_QED) := qed.o
qed-y := qed_cxt.o qed_dev.o qed_hw.o qed_init_fw_funcs.o qed_init_ops.o \
qed_int.o qed_main.o qed_mcp.o qed_sp_commands.o qed_spq.o qed_l2.o \
- qed_selftest.o qed_dcbx.o
+ qed_selftest.o qed_dcbx.o qed_util.o
qed-$(CONFIG_QED_SRIOV) += qed_sriov.o qed_vf.o
diff --git a/drivers/net/ethernet/qlogic/qed/qed_util.c b/drivers/net/ethernet/qlogic/qed/qed_util.c
new file mode 100644
index 0000000..2795e63
--- /dev/null
+++ b/drivers/net/ethernet/qlogic/qed/qed_util.c
@@ -0,0 +1,82 @@
+#include <linux/kernel.h>
+#include <linux/qed/qed_if.h>
+
+#include "qed.h"
+
+void qed_err(const char *name, int line, const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ pr_err("[%ps:%d(%s)] %pV",
+ __builtin_return_address(0), line, name ? name : "",
+ &vaf);
+
+ va_end(args);
+}
+
+void qed_notice(u32 level, const char *name, int line, const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ if (likely(level > QED_LEVEL_NOTICE))
+ return;
+
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ pr_notice("[%ps:%d(%s)] %pV",
+ __builtin_return_address(0), line, name ? name : "",
+ &vaf);
+
+ va_end(args);
+}
+
+void qed_info(u32 level, const char *name, int line, const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ if (likely(level > QED_LEVEL_INFO))
+ return;
+
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ pr_notice("[%ps:%d(%s)] %pV",
+ __builtin_return_address(0), line, name ? name : "",
+ &vaf);
+
+ va_end(args);
+}
+
+void qed_verbose(u32 level, const char *name, int line,
+ const char *fmt, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ if (likely(level > QED_LEVEL_VERBOSE))
+ return;
+
+ va_start(args, fmt);
+
+ vaf.fmt = fmt;
+ vaf.va = &args;
+
+ pr_notice("[%ps:%d(%s)] %pV",
+ __builtin_return_address(0), line, name ? name : "",
+ &vaf);
+
+ va_end(args);
+}
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index b1e3c57..fa3d8c0 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -539,44 +539,30 @@ struct qed_common_ops {
#define GET_FIELD(value, name) \
(((value) >> (name ## _SHIFT)) & name ## _MASK)
-/* Debug print definitions */
-#define DP_ERR(cdev, fmt, ...) \
- pr_err("[%s:%d(%s)]" fmt, \
- __func__, __LINE__, \
- DP_NAME(cdev) ? DP_NAME(cdev) : "", \
- ## __VA_ARGS__) \
-
-#define DP_NOTICE(cdev, fmt, ...) \
- do { \
- if (unlikely((cdev)->dp_level <= QED_LEVEL_NOTICE)) { \
- pr_notice("[%s:%d(%s)]" fmt, \
- __func__, __LINE__, \
- DP_NAME(cdev) ? DP_NAME(cdev) : "", \
- ## __VA_ARGS__); \
- \
- } \
- } while (0)
-
-#define DP_INFO(cdev, fmt, ...) \
- do { \
- if (unlikely((cdev)->dp_level <= QED_LEVEL_INFO)) { \
- pr_notice("[%s:%d(%s)]" fmt, \
- __func__, __LINE__, \
- DP_NAME(cdev) ? DP_NAME(cdev) : "", \
- ## __VA_ARGS__); \
- } \
- } while (0)
+__printf(3, 4)
+void qed_err(const char *name, int line, const char *fmt, ...);
+__printf(4, 5)
+void qed_notice(u32 level, const char *name, int line, const char *fmt, ...);
+__printf(4, 5)
+void qed_info(u32 level, const char *name, int line, const char *fmt, ...);
+__printf(4, 5)
+void qed_verbose(u32 level, const char *name, int line, const char *fmt, ...);
-#define DP_VERBOSE(cdev, module, fmt, ...) \
- do { \
- if (unlikely(((cdev)->dp_level <= QED_LEVEL_VERBOSE) && \
- ((cdev)->dp_module & module))) { \
- pr_notice("[%s:%d(%s)]" fmt, \
- __func__, __LINE__, \
- DP_NAME(cdev) ? DP_NAME(cdev) : "", \
- ## __VA_ARGS__); \
- } \
- } while (0)
+/* Debug print definitions */
+#define DP_ERR(type, fmt, ...) \
+ qed_err(DP_NAME(type), __LINE__, fmt, ##__VA_ARGS__)
+#define DP_NOTICE(type, fmt, ...) \
+ qed_notice((type)->dp_level, DP_NAME(type), __LINE__, \
+ fmt, ##__VA_ARGS__)
+#define DP_INFO(type, fmt, ...) \
+ qed_info((type)->dp_level, DP_NAME(type), __LINE__, \
+ fmt, ##__VA_ARGS__)
+#define DP_VERBOSE(type, module, fmt, ...) \
+do { \
+ if ((type)->dp_module & module) \
+ qed_verbose((type)->dp_level, DP_NAME(type), __LINE__, \
+ fmt, ##__VA_ARGS__); \
+} while (0)
enum DP_LEVEL {
QED_LEVEL_VERBOSE = 0x0,
--
2.8.0.rc4.16.g56331f8
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] qed: Add and use specific logging functions to reduce object size
2016-07-26 21:25 [PATCH] qed: Add and use specific logging functions to reduce object size Joe Perches
@ 2016-07-27 1:24 ` Joe Perches
2016-07-27 7:32 ` Yuval Mintz
2016-07-27 7:24 ` Yuval Mintz
2016-07-31 0:15 ` kbuild test robot
2 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2016-07-27 1:24 UTC (permalink / raw)
To: Yuval Mintz, Ariel Elior, everest-linux-l2; +Cc: linux-kernel, netdev
On Tue, 2016-07-26 at 14:25 -0700, Joe Perches wrote:
> Current DP_ macros generate a lot of code.
> Using functions with vsprintf extension %pV helps reduce that size.
Yuval, I used the same KERN_<LEVEL> output types, but it
is unusual that DP_INFO outputs at KERN_NOTICE.
Was that a copy/paste defect or should it be emitted at
KERN_INFO and DP_VERBOSE be emitted at KERN_DEBUG?
> define DP_INFO(cdev, fmt, ...) \
> - do { \
> - if (unlikely((cdev)->dp_level <= QED_LEVEL_INFO)) { \
> - pr_notice("[%s:%d(%s)]" fmt, \
> - __func__, __LINE__, \
> - DP_NAME(cdev) ? DP_NAME(cdev) : "", \
> - ## __VA_ARGS__); \
> - } \
> - } while (0)
[]
> -#define DP_VERBOSE(cdev, module, fmt, ...) \
> - do { \
> - if (unlikely(((cdev)->dp_level <= QED_LEVEL_VERBOSE) && \
> - ((cdev)->dp_module & module))) { \
> - pr_notice("[%s:%d(%s)]" fmt, \
> - __func__, __LINE__, \
> - DP_NAME(cdev) ? DP_NAME(cdev) : "", \
> - ## __VA_ARGS__); \
> - } \
> - } while (0)
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] qed: Add and use specific logging functions to reduce object size
2016-07-26 21:25 [PATCH] qed: Add and use specific logging functions to reduce object size Joe Perches
2016-07-27 1:24 ` Joe Perches
@ 2016-07-27 7:24 ` Yuval Mintz
2016-07-31 18:04 ` Joe Perches
2016-07-31 0:15 ` kbuild test robot
2 siblings, 1 reply; 6+ messages in thread
From: Yuval Mintz @ 2016-07-27 7:24 UTC (permalink / raw)
To: Joe Perches, Ariel Elior, Dept-Eng Everest Linux L2; +Cc: linux-kernel, netdev
> Current DP_<LEVEL> macros generate a lot of code.
> Using functions with vsprintf extension %pV helps reduce that size.
>
> drivers/net/ethernet/qlogic/qed/Makefile | 2 +-
> drivers/net/ethernet/qlogic/qed/qed_util.c | 82
> ++++++++++++++++++++++++++++++
> include/linux/qed/qed_if.h | 60 +++++++++-------------
> 3 files changed, 106 insertions(+), 38 deletions(-) create mode 100644
> drivers/net/ethernet/qlogic/qed/qed_util.c
This won't compile when CONFIG_QED*=m, as qede can't link to
the new qed_{err, verbose, info, notice} functions.
That was the original reason for putting the macros in the interface
header.
Other alternatives:
- We can EXPORT_SYMBOL() the functions, although we've taken a
strain not adding such to the interface.
- Code duplication between qed/qede [ugly].
- Implementing this in qede via the interface functions with qed;
But the notion of defining an interface between 2 modules passing
va_args seems [to me] like a bad one.
If you have cleaner solutions, I'd be happy to hear those.
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] qed: Add and use specific logging functions to reduce object size
2016-07-27 1:24 ` Joe Perches
@ 2016-07-27 7:32 ` Yuval Mintz
0 siblings, 0 replies; 6+ messages in thread
From: Yuval Mintz @ 2016-07-27 7:32 UTC (permalink / raw)
To: Joe Perches, Ariel Elior, Dept-Eng Everest Linux L2; +Cc: linux-kernel, netdev
> > Current DP_ macros generate a lot of code.
> > Using functions with vsprintf extension %pV helps reduce that size.
>
> Yuval, I used the same KERN_<LEVEL> output types, but it is unusual
> that DP_INFO outputs at KERN_NOTICE.
>
> Was that a copy/paste defect or should it be emitted at KERN_INFO and
> DP_VERBOSE be emitted at KERN_DEBUG?
I agree it's a bit odd, but it's actually by design -
as none of these prints would ever be reached unless user explicitly enable
them [ethtool/module param], the assumption is that NOTICE is good
enough, i.e., it will prevent the need for doing additional configuration of
logging levels by the user.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] qed: Add and use specific logging functions to reduce object size
2016-07-26 21:25 [PATCH] qed: Add and use specific logging functions to reduce object size Joe Perches
2016-07-27 1:24 ` Joe Perches
2016-07-27 7:24 ` Yuval Mintz
@ 2016-07-31 0:15 ` kbuild test robot
2 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2016-07-31 0:15 UTC (permalink / raw)
To: Joe Perches
Cc: kbuild-all, Yuval Mintz, Ariel Elior, everest-linux-l2,
linux-kernel, netdev
[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]
Hi,
[auto build test ERROR on net-next/master]
[also build test ERROR on v4.7 next-20160729]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Joe-Perches/qed-Add-and-use-specific-logging-functions-to-reduce-object-size/20160727-063300
config: i386-randconfig-sb0-07310616 (attached as .config)
compiler: gcc-5 (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
>> ERROR: "qed_err" [drivers/net/ethernet/qlogic/qede/qede.ko] undefined!
>> ERROR: "qed_verbose" [drivers/net/ethernet/qlogic/qede/qede.ko] undefined!
>> ERROR: "qed_info" [drivers/net/ethernet/qlogic/qede/qede.ko] undefined!
>> ERROR: "qed_notice" [drivers/net/ethernet/qlogic/qede/qede.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24308 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] qed: Add and use specific logging functions to reduce object size
2016-07-27 7:24 ` Yuval Mintz
@ 2016-07-31 18:04 ` Joe Perches
0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2016-07-31 18:04 UTC (permalink / raw)
To: Yuval Mintz, Ariel Elior, Dept-Eng Everest Linux L2; +Cc: linux-kernel, netdev
On Wed, 2016-07-27 at 07:24 +0000, Yuval Mintz wrote:
> >
> > Current DP_ macros generate a lot of code.
> > Using functions with vsprintf extension %pV helps reduce that size.
> >
> > drivers/net/ethernet/qlogic/qed/Makefile | 2 +-
> > drivers/net/ethernet/qlogic/qed/qed_util.c | 82 ++++++++++++++++++++++++++++++
> > include/linux/qed/qed_if.h | 60 +++++++++-------------
> > 3 files changed, 106 insertions(+), 38 deletions(-) create mode 100644
> > drivers/net/ethernet/qlogic/qed/qed_util.c
> This won't compile when CONFIG_QED*=m, as qede can't link to
> the new qed_{err, verbose, info, notice} functions.
> That was the original reason for putting the macros in the interface
> header.
>
> Other alternatives:
> - We can EXPORT_SYMBOL() the functions, although we've taken a
> strain not adding such to the interface.
> - Code duplication between qed/qede [ugly].
> - Implementing this in qede via the interface functions with qed;
> But the notion of defining an interface between 2 modules passing
> va_args seems [to me] like a bad one.
>
> If you have cleaner solutions, I'd be happy to hear those.
Hello Yuval.
EXPORT_SYMBOL is probably the simplest solution.
It's pretty commonly used for private logging functions
in drivers/net/, e.g.: drivers/net/wireless/ath/main.c
I'll submit that in awhile.
cheers, Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-31 18:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-26 21:25 [PATCH] qed: Add and use specific logging functions to reduce object size Joe Perches
2016-07-27 1:24 ` Joe Perches
2016-07-27 7:32 ` Yuval Mintz
2016-07-27 7:24 ` Yuval Mintz
2016-07-31 18:04 ` Joe Perches
2016-07-31 0:15 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).