| Message ID | 1440612818.2780.24.camel@perches.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
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?
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 platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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) >
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.
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)
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(-) -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html