2022-05-02 21:20:33

by Carlos Llamas

[permalink] [raw]
Subject: [PATCH v2 4/5] binder: convert logging macros into functions

Converting binder_debug() and binder_user_error() macros into functions
reduces the overall object size by 16936 bytes when cross-compiled with
aarch64-linux-gnu-gcc 11.2.0:

$ size drivers/android/binder.o.{old,new}
text data bss dec hex filename
77935 6168 20264 104367 197af drivers/android/binder.o.old
65551 1616 20264 87431 15587 drivers/android/binder.o.new

This is particularly beneficial to functions binder_transaction() and
binder_thread_write() which repeatedly use these macros and are both
part of the critical path for all binder transactions.

$ nm --size vmlinux.{old,new} |grep ' binder_transaction$'
0000000000002f60 t binder_transaction
0000000000002358 t binder_transaction

$ nm --size vmlinux.{old,new} |grep binder_thread_write
0000000000001c54 t binder_thread_write
00000000000014a8 t binder_thread_write

Signed-off-by: Carlos Llamas <[email protected]>
---
drivers/android/binder.c | 41 ++++++++++++++++++++++++++++------------
1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index b9df0c8a68d3..bfb21e258427 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -133,19 +133,36 @@ static int binder_set_stop_on_user_error(const char *val,
module_param_call(stop_on_user_error, binder_set_stop_on_user_error,
param_get_int, &binder_stop_on_user_error, 0644);

-#define binder_debug(mask, x...) \
- do { \
- if (binder_debug_mask & mask) \
- pr_info_ratelimited(x); \
- } while (0)
+static __printf(2, 3) void binder_debug(int mask, const char *format, ...)
+{
+ struct va_format vaf;
+ va_list args;

-#define binder_user_error(x...) \
- do { \
- if (binder_debug_mask & BINDER_DEBUG_USER_ERROR) \
- pr_info_ratelimited(x); \
- if (binder_stop_on_user_error) \
- binder_stop_on_user_error = 2; \
- } while (0)
+ if (binder_debug_mask & mask) {
+ va_start(args, format);
+ vaf.va = &args;
+ vaf.fmt = format;
+ pr_info_ratelimited("%pV", &vaf);
+ va_end(args);
+ }
+}
+
+static __printf(1, 2) void binder_user_error(const char *format, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ if (binder_debug_mask & BINDER_DEBUG_USER_ERROR) {
+ va_start(args, format);
+ vaf.va = &args;
+ vaf.fmt = format;
+ pr_info_ratelimited("%pV", &vaf);
+ va_end(args);
+ }
+
+ if (binder_stop_on_user_error)
+ binder_stop_on_user_error = 2;
+}

#define binder_set_extended_error(ee, _id, _command, _param) \
do { \
--
2.36.0.464.gb9c8b46e94-goog


2022-05-03 01:08:21

by Todd Kjos

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] binder: convert logging macros into functions

On Fri, Apr 29, 2022 at 4:57 PM Carlos Llamas <[email protected]> wrote:
>
> Converting binder_debug() and binder_user_error() macros into functions
> reduces the overall object size by 16936 bytes when cross-compiled with
> aarch64-linux-gnu-gcc 11.2.0:
>
> $ size drivers/android/binder.o.{old,new}
> text data bss dec hex filename
> 77935 6168 20264 104367 197af drivers/android/binder.o.old
> 65551 1616 20264 87431 15587 drivers/android/binder.o.new
>
> This is particularly beneficial to functions binder_transaction() and
> binder_thread_write() which repeatedly use these macros and are both
> part of the critical path for all binder transactions.
>
> $ nm --size vmlinux.{old,new} |grep ' binder_transaction$'
> 0000000000002f60 t binder_transaction
> 0000000000002358 t binder_transaction
>
> $ nm --size vmlinux.{old,new} |grep binder_thread_write
> 0000000000001c54 t binder_thread_write
> 00000000000014a8 t binder_thread_write
>
> Signed-off-by: Carlos Llamas <[email protected]>

Acked-by: Todd Kjos <[email protected]>

> ---
> drivers/android/binder.c | 41 ++++++++++++++++++++++++++++------------
> 1 file changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index b9df0c8a68d3..bfb21e258427 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -133,19 +133,36 @@ static int binder_set_stop_on_user_error(const char *val,
> module_param_call(stop_on_user_error, binder_set_stop_on_user_error,
> param_get_int, &binder_stop_on_user_error, 0644);
>
> -#define binder_debug(mask, x...) \
> - do { \
> - if (binder_debug_mask & mask) \
> - pr_info_ratelimited(x); \
> - } while (0)
> +static __printf(2, 3) void binder_debug(int mask, const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
>
> -#define binder_user_error(x...) \
> - do { \
> - if (binder_debug_mask & BINDER_DEBUG_USER_ERROR) \
> - pr_info_ratelimited(x); \
> - if (binder_stop_on_user_error) \
> - binder_stop_on_user_error = 2; \
> - } while (0)
> + if (binder_debug_mask & mask) {
> + va_start(args, format);
> + vaf.va = &args;
> + vaf.fmt = format;
> + pr_info_ratelimited("%pV", &vaf);
> + va_end(args);
> + }
> +}
> +
> +static __printf(1, 2) void binder_user_error(const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + if (binder_debug_mask & BINDER_DEBUG_USER_ERROR) {
> + va_start(args, format);
> + vaf.va = &args;
> + vaf.fmt = format;
> + pr_info_ratelimited("%pV", &vaf);
> + va_end(args);
> + }
> +
> + if (binder_stop_on_user_error)
> + binder_stop_on_user_error = 2;
> +}
>
> #define binder_set_extended_error(ee, _id, _command, _param) \
> do { \
> --
> 2.36.0.464.gb9c8b46e94-goog
>

2022-05-09 10:57:35

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH v2 4/5] binder: convert logging macros into functions

On Fri, Apr 29, 2022 at 11:56:43PM +0000, Carlos Llamas wrote:
> Converting binder_debug() and binder_user_error() macros into functions
> reduces the overall object size by 16936 bytes when cross-compiled with
> aarch64-linux-gnu-gcc 11.2.0:
>
> $ size drivers/android/binder.o.{old,new}
> text data bss dec hex filename
> 77935 6168 20264 104367 197af drivers/android/binder.o.old
> 65551 1616 20264 87431 15587 drivers/android/binder.o.new
>
> This is particularly beneficial to functions binder_transaction() and
> binder_thread_write() which repeatedly use these macros and are both
> part of the critical path for all binder transactions.
>
> $ nm --size vmlinux.{old,new} |grep ' binder_transaction$'
> 0000000000002f60 t binder_transaction
> 0000000000002358 t binder_transaction
>
> $ nm --size vmlinux.{old,new} |grep binder_thread_write
> 0000000000001c54 t binder_thread_write
> 00000000000014a8 t binder_thread_write
>
> Signed-off-by: Carlos Llamas <[email protected]>
> ---

Looks good to me,
Acked-by: Christian Brauner (Microsoft) <[email protected]>