Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220429235644.697372-1-cmllamas@google.com> <20220429235644.697372-5-cmllamas@google.com>
In-Reply-To: <20220429235644.697372-5-cmllamas@google.com>
From:   Todd Kjos <tkjos@google.com>
Date:   Mon, 2 May 2022 08:28:53 -0700
Message-ID: <CAHRSSEyRm2wkFhxE+wz6bVgne7+pBE0_cjL=bhcesFqA1=Sy6Q@mail.gmail.com>
Subject: Re: [PATCH v2 4/5] binder: convert logging macros into functions
To:     Carlos Llamas <cmllamas@google.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= <arve@android.com>,
        Todd Kjos <tkjos@android.com>,
        Martijn Coenen <maco@android.com>,
        Christian Brauner <brauner@kernel.org>,
        Suren Baghdasaryan <surenb@google.com>,
        Joel Fernandes <joel@joelfernandes.org>,
        Hridya Valsaraju <hridya@google.com>, kernel-team@android.com,
        linux-kernel@vger.kernel.org, Shuah Khan <shuah@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>, Li Li <dualli@google.com>,
        Masahiro Yamada <masahiroy@kernel.org>,
        linux-kselftest@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Apr 29, 2022 at 4:57 PM Carlos Llamas <cmllamas@google.com> 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 <cmllamas@google.com>

Acked-by: Todd Kjos <tkjos@google.com>

> ---
>  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
>