Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1214092 > unrolled thread

[PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro

Started byJoe Perches <joe@perches.com>
First post2015-08-26 20:20 +0200
Last post2015-08-28 20:00 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk ->  no_printk macro Joe Perches <joe@perches.com> - 2015-08-26 20:20 +0200
    Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk ->  no_printk macro Henrique de Moraes Holschuh <hmh@hmh.eng.br> - 2015-08-26 20:30 +0200
      Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk ->  no_printk macro Joe Perches <joe@perches.com> - 2015-08-26 20:40 +0200
        Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk ->  no_printk macro Henrique de Moraes Holschuh <hmh@hmh.eng.br> - 2015-08-27 19:40 +0200
    Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk ->  no_printk macro Henrique de Moraes Holschuh <hmh@hmh.eng.br> - 2015-08-27 19:40 +0200
      Re: [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk ->  no_printk macro Darren Hart <dvhart@infradead.org> - 2015-08-28 20:00 +0200

#1214092 — [PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro

FromJoe Perches <joe@perches.com>
Date2015-08-26 20:20 +0200
Subject[PATCH] thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro
Message-ID<q1NsJ-55Y-11@gated-at.bofh.it>
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 <joe@perches.com>
---
 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)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1214109

FromHenrique de Moraes Holschuh <hmh@hmh.eng.br>
Date2015-08-26 20:30 +0200
Message-ID<q1NCq-5hp-25@gated-at.bofh.it>
In reply to#1214092
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1214123

FromJoe Perches <joe@perches.com>
Date2015-08-26 20:40 +0200
Message-ID<q1NM7-5sE-43@gated-at.bofh.it>
In reply to#1214109
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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1214742

FromHenrique de Moraes Holschuh <hmh@hmh.eng.br>
Date2015-08-27 19:40 +0200
Message-ID<q29jA-2Rz-27@gated-at.bofh.it>
In reply to#1214123
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1214743

FromHenrique de Moraes Holschuh <hmh@hmh.eng.br>
Date2015-08-27 19:40 +0200
Message-ID<q29jB-2Rz-31@gated-at.bofh.it>
In reply to#1214092
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 <joe@perches.com>

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1215487

FromDarren Hart <dvhart@infradead.org>
Date2015-08-28 20:00 +0200
Message-ID<q2w6u-21Z-23@gated-at.bofh.it>
In reply to#1214743
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 <joe@perches.com>
> 
> Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

Queued, thanks.

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web