vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
no_printk which produces no logging output but always
evaluates arguments.
Change the macro to surround the no_printk call with
do { if (0) no_printk(...); } while (0)
to avoid the unnecessary argument evaluations.
$ size drivers/platform/x86/thinkpad_acpi.o*
text data bss dec hex filename
60918 6184 824 67926 10956 drivers/platform/x86/thinkpad_acpi.o.new
60927 6184 824 67935 1095f drivers/platform/x86/thinkpad_acpi.o.old
Signed-off-by: Joe Perches <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 33e488c..131dd74 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
#else
static inline const char *str_supported(int is_supported) { return ""; }
#define vdbg_printk(a_dbg_level, format, arg...) \
- no_printk(format, ##arg)
+ do { if (0) no_printk(format, ##arg); } while (0)
#endif
static void tpacpi_log_usertask(const char * const what)
On Wed, Aug 26, 2015, at 15:13, Joe Perches wrote:
> vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> no_printk which produces no logging output but always
> evaluates arguments.
>
> Change the macro to surround the no_printk call with
> do { if (0) no_printk(...); } while (0)
> to avoid the unnecessary argument evaluations.
>
> $ size drivers/platform/x86/thinkpad_acpi.o*
> text data bss dec hex filename
> 60918 6184 824 67926 10956
> drivers/platform/x86/thinkpad_acpi.o.new
> 60927 6184 824 67935 1095f
> drivers/platform/x86/thinkpad_acpi.o.old
The code size savings of this change are really small...
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
> #else
> static inline const char *str_supported(int is_supported) { return ""; }
> #define vdbg_printk(a_dbg_level, format, arg...) \
> - no_printk(format, ##arg)
> + do { if (0) no_printk(format, ##arg); } while (0)
> #endif
And won't this change disable compile-time checking of 'format ## arg'
for issues?
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
On Wed, 2015-08-26 at 15:27 -0300, Henrique de Moraes Holschuh wrote:
> On Wed, Aug 26, 2015, at 15:13, Joe Perches wrote:
> > vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> > no_printk which produces no logging output but always
> > evaluates arguments.
> >
> > Change the macro to surround the no_printk call with
> > do { if (0) no_printk(...); } while (0)
> > to avoid the unnecessary argument evaluations.
> >
> > $ size drivers/platform/x86/thinkpad_acpi.o*
> > text data bss dec hex filename
> > 60918 6184 824 67926 10956
> > drivers/platform/x86/thinkpad_acpi.o.new
> > 60927 6184 824 67935 1095f
> > drivers/platform/x86/thinkpad_acpi.o.old
>
> The code size savings of this change are really small...
yup. This is just an on-the-way patch to get
all the no_printk uses in a state that can
be converted more simply to a side-effect free
mechanism.
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
> > #else
> > static inline const char *str_supported(int is_supported) { return ""; }
> > #define vdbg_printk(a_dbg_level, format, arg...) \
> > - no_printk(format, ##arg)
> > + do { if (0) no_printk(format, ##arg); } while (0)
> > #endif
>
> And won't this change disable compile-time checking of 'format ## arg'
> for issues?
No. The call is still there so the compiler checks
the format/argument matches, but eliminates the
call and any side-effect calls from the object file.
On Wed, 26 Aug 2015, Joe Perches wrote:
> On Wed, 2015-08-26 at 15:27 -0300, Henrique de Moraes Holschuh wrote:
> > On Wed, Aug 26, 2015, at 15:13, Joe Perches wrote:
> > > vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> > > no_printk which produces no logging output but always
> > > evaluates arguments.
> > >
> > > Change the macro to surround the no_printk call with
> > > do { if (0) no_printk(...); } while (0)
> > > to avoid the unnecessary argument evaluations.
> > >
> > > $ size drivers/platform/x86/thinkpad_acpi.o*
> > > text data bss dec hex filename
> > > 60918 6184 824 67926 10956
> > > drivers/platform/x86/thinkpad_acpi.o.new
> > > 60927 6184 824 67935 1095f
> > > drivers/platform/x86/thinkpad_acpi.o.old
> >
> > The code size savings of this change are really small...
>
> yup. This is just an on-the-way patch to get
> all the no_printk uses in a state that can
> be converted more simply to a side-effect free
> mechanism.
>
> > > --- a/drivers/platform/x86/thinkpad_acpi.c
> > > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > > @@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
> > > #else
> > > static inline const char *str_supported(int is_supported) { return ""; }
> > > #define vdbg_printk(a_dbg_level, format, arg...) \
> > > - no_printk(format, ##arg)
> > > + do { if (0) no_printk(format, ##arg); } while (0)
> > > #endif
> >
> > And won't this change disable compile-time checking of 'format ## arg'
> > for issues?
>
> No. The call is still there so the compiler checks
> the format/argument matches, but eliminates the
> call and any side-effect calls from the object file.
Ok, very well. In that case, I will ack it.
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
On Wed, 26 Aug 2015, Joe Perches wrote:
> vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> no_printk which produces no logging output but always
> evaluates arguments.
>
> Change the macro to surround the no_printk call with
> do { if (0) no_printk(...); } while (0)
> to avoid the unnecessary argument evaluations.
>
> $ size drivers/platform/x86/thinkpad_acpi.o*
> text data bss dec hex filename
> 60918 6184 824 67926 10956 drivers/platform/x86/thinkpad_acpi.o.new
> 60927 6184 824 67935 1095f drivers/platform/x86/thinkpad_acpi.o.old
>
> Signed-off-by: Joe Perches <[email protected]>
Acked-by: Henrique de Moraes Holschuh <[email protected]>
> ---
> drivers/platform/x86/thinkpad_acpi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 33e488c..131dd74 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);
> #else
> static inline const char *str_supported(int is_supported) { return ""; }
> #define vdbg_printk(a_dbg_level, format, arg...) \
> - no_printk(format, ##arg)
> + do { if (0) no_printk(format, ##arg); } while (0)
> #endif
>
> static void tpacpi_log_usertask(const char * const what)
>
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
On Thu, Aug 27, 2015 at 02:33:06PM -0300, Henrique de Moraes Holschuh wrote:
> On Wed, 26 Aug 2015, Joe Perches wrote:
> > vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
> > no_printk which produces no logging output but always
> > evaluates arguments.
> >
> > Change the macro to surround the no_printk call with
> > do { if (0) no_printk(...); } while (0)
> > to avoid the unnecessary argument evaluations.
> >
> > $ size drivers/platform/x86/thinkpad_acpi.o*
> > text data bss dec hex filename
> > 60918 6184 824 67926 10956 drivers/platform/x86/thinkpad_acpi.o.new
> > 60927 6184 824 67935 1095f drivers/platform/x86/thinkpad_acpi.o.old
> >
> > Signed-off-by: Joe Perches <[email protected]>
>
> Acked-by: Henrique de Moraes Holschuh <[email protected]>
Queued, thanks.
--
Darren Hart
Intel Open Source Technology Center