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

[Laurent Vivier]

Le 04/02/2020 à 03:55, Josh Kunz a écrit :
> 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.
> 

If generated code is the same, I have no objection.

Thanks,
Laurent