thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro
PATCH 1/1 thinkpad_acpi: Remove side effects from vdbg_printk -> no_printk macro
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
--- drivers/platform/x86/thinkpad_acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/platform/x86/thinkpad_acpi.c +1 -1
@@ -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)
Quoted 6 lines · Joe Perches
vdbg_printkwhen not usingCONFIG_THINKPAD_ACPI_DEBUGusesno_printkwhich produces no logging output but always evaluates arguments.Change the macro to surround the
no_printkcall withdo { if (0) no_printk(...); } while (0)
Quoted 8 lines
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...
drivers/platform/x86/thinkpad_acpi.c static const char *str_supported(int is_supported); · line 406
Quoted 9 lines
drivers/platform/x86/thinkpad_acpi.c +1
-1@@ -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?
Signature · 4 lines
"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
Quoted 9 lines · Henrique de Moraes Holschuh
Quoted 8 lines · Joe Perches
vdbg_printkwhen not usingCONFIG_THINKPAD_ACPI_DEBUGusesno_printkwhich produces no logging output but always evaluates arguments.Change the macro to surround the
no_printkcall withdo { if (0) no_printk(...); } while (0)to avoid the unnecessary argument evaluations.
Quoted 8 lines
Quoted 6 lines
$ 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.oldThe 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.
Quoted 4 lines
Quoted 4 lines
drivers/platform/x86/thinkpad_acpi.c +0
-0@@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);#else
Quoted 8 lines
drivers/platform/x86/thinkpad_acpi.c static const char *str_supported(int is_supported); · line 406
Quoted 5 lines
drivers/platform/x86/thinkpad_acpi.c +1
-1static 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.
Quoted 37 lines · Joe Perches
Quoted 9 lines · Henrique de Moraes Holschuh
Quoted 8 lines · Joe Perches
vdbg_printkwhen not usingCONFIG_THINKPAD_ACPI_DEBUGusesno_printkwhich produces no logging output but always evaluates arguments.Change the macro to surround the
no_printkcall withdo { if (0) no_printk(...); } while (0)to avoid the unnecessary argument evaluations.
Quoted 8 lines
Quoted 6 lines
$ 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.oldThe code size savings of this change are really small...
yup. This is just an on-the-way patch to get all the
no_printkuses in a state that can be converted more simply to a side-effect free mechanism.Quoted 4 lines
Quoted 4 lines
drivers/platform/x86/thinkpad_acpi.c +0
-0@@ -402,7 +402,7 @@ static const char *str_supported(int is_supported);#else Quoted 8 lines
drivers/platform/x86/thinkpad_acpi.c static const char *str_supported(int is_supported); · line 406
Quoted 5 lines
drivers/platform/x86/thinkpad_acpi.c +1
-1static 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?
Quoted 3 lines
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.
Signature · 4 lines
"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
Quoted 6 lines · Joe Perches
vdbg_printkwhen not usingCONFIG_THINKPAD_ACPI_DEBUGusesno_printkwhich produces no logging output but always evaluates arguments.Change the macro to surround the
no_printkcall withdo { if (0) no_printk(...); } while (0)
Quoted 8 lines
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.oldSigned-off-by Joe Perches <[email protected]>
Quoted 18 lines
--- drivers/platform/x86/thinkpad_acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/platform/x86/thinkpad_acpi.c +1
-1@@ -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)
Signature · 6 lines
"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 ------------------------------------------------------------------------------
Darren Hart
AppliedQuoted 9 lines · Henrique de Moraes Holschuh
Quoted 8 lines · Joe Perches
vdbg_printkwhen not usingCONFIG_THINKPAD_ACPI_DEBUGusesno_printkwhich produces no logging output but always evaluates arguments.Change the macro to surround the
no_printkcall withdo { if (0) no_printk(...); } while (0)to avoid the unnecessary argument evaluations.
Quoted 8 lines
Quoted 6 lines
$ 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.oldSigned-off-by Joe Perches <[email protected]>
Queued, thanks.
Signature · 2 lines
Darren Hart Intel Open Source Technology Center