Re: [PATCH v2.1 RFC 17/12] xen/trace: Drop cycles parameter

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 22/09/2021 08:03, Jan Beulich wrote:
> On 21.09.2021 22:08, Andrew Cooper wrote:
>> Not a patch, but an RFC idea for one...
>>
>> It occurred to me that the cycles parameter from __trace_var() and
>> friends is pointless, as the cycles bit is encoded in the top bit of the
>> event field anyway, and passed in the adjacent parameter.
>>
>> Dropping the cycles parameter results in +85/-1029 (-944) net change.
>>
>> The common change in callers is e.g. from:
>>
>> lea    0x3c(%rsp),%rcx
>> mov    $0x4,%edx
>> mov    $0x1,%esi
>> mov    $0x10f002,%edi
>> mov    %ebp,0x3c(%rsp)
>> callq  ffff82d04022ea20 <__trace_var>
>>
>> to this:
>>
>> lea    0x3c(%rsp),%rdx
>> mov    $0x4,%esi
>> mov    $0x8010f002,%edi
>> mov    %ebp,0x3c(%rsp)
>> callq  ffff82d04022ea20 <__trace_var>
>>
>>
>> Just sprinkling "| TRC_HD_CYCLE_FLAG" over the place makes things a
>> little bit unwieldy.
>>
>> Instead, I was thinking of implementing trace() (and a thin trace_time()
>> wrapper) mirroring the "new API" in patch 14.  Half of the trace_var()
>> users should be __trace_var() already because of living inside a
>> tb_init_done conditional, and the rest are actually opencoded TRACE()
>> taking no extra data.
>>
>> Thoughts?
> Sounds quite reasonable to me.

In which case, for v3 I'll reposition "new API" towards the beginning of
the series, extend it with these two new APIs, then adjust all the
cleanup patches to avoid double-patching most of the call-sites.

~Andrew