Re: [PATCH v2 2/4] linux-user: Use `qemu_log' for strace

[Josh Kunz]

On Tue, Jan 28, 2020 at 7:07 AM Laurent Vivier <address@hidden> wrote:
>
> Le 17/01/2020 à 20:28, Josh Kunz a écrit :
> > This change switches linux-user strace logging to use the newer `qemu_log`
> > logging subsystem rather than the older `gemu_log` (notice the "g")
> > logger. `qemu_log` has several advantages, namely that it allows logging
> > to a file, and provides a more unified interface for configuration
> > of logging (via the QEMU_LOG environment variable or options).
> >
> > This change introduces a new log mask: `LOG_STRACE` which is used for
> > logging of user-mode strace messages.
> >
> > Signed-off-by: Josh Kunz <address@hidden>
> > ---
> >  include/qemu/log.h   |   2 +
> >  linux-user/main.c    |  30 ++-
> >  linux-user/qemu.h    |   1 -
> >  linux-user/signal.c  |   2 +-
> >  linux-user/strace.c  | 479 ++++++++++++++++++++++---------------------
> >  linux-user/syscall.c |  13 +-
> >  util/log.c           |   2 +
> >  7 files changed, 278 insertions(+), 251 deletions(-)
> >
> ...
> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> > index 629f3a21b5..54e60f3807 100644
> > --- a/linux-user/syscall.c
> > +++ b/linux-user/syscall.c
> > @@ -12098,14 +12098,15 @@ abi_long do_syscall(void *cpu_env, int num, 
> > abi_long arg1,
> >      record_syscall_start(cpu, num, arg1,
> >                           arg2, arg3, arg4, arg5, arg6, arg7, arg8);
> >
> > -    if (unlikely(do_strace)) {
> > +    if (unlikely(qemu_loglevel_mask(LOG_STRACE))) {
> >          print_syscall(num, arg1, arg2, arg3, arg4, arg5, arg6);
> > -        ret = do_syscall1(cpu_env, num, arg1, arg2, arg3, arg4,
> > -                          arg5, arg6, arg7, arg8);
> > +    }
> > +
> > +    ret = do_syscall1(cpu_env, num, arg1, arg2, arg3, arg4,
> > +                      arg5, arg6, arg7, arg8);
> > +
> > +    if (unlikely(qemu_loglevel_mask(LOG_STRACE))) {
> >          print_syscall_ret(num, ret);
> > -    } else {
> > -        ret = do_syscall1(cpu_env, num, arg1, arg2, arg3, arg4,
> > -                          arg5, arg6, arg7, arg8);
> >      }
> >
> >      record_syscall_return(cpu, num, ret);
>
> In term of performance perhaps it sould be better to only test once for
> the mask as it is done before?

Modern compilers will generate functionally identical sequences for
test once or testing twice (which is to say, they recognize they are
the same compare: https://godbolt.org/z/VyrMHf IMO testing twice is
nicer to read, so I'm leaving it that way for now unless you object.