Re: [PATCH v2 2/2] s390/kvm: Add tracepoint for DIAG 9c directed yield operations

[Christian Borntraeger <borntraeger@linux.ibm.com>]

Am 23.07.26 um 21:56 schrieb Ciunas Bennett:
> 
> 
> On 23/07/2026 16:28, Christian Borntraeger wrote:
>>
> 
> [...]
> 
>>> +TRACE_EVENT(kvm_s390_diag_9c,
>>> +        TP_PROTO(VCPU_PROTO_COMMON, int target_vcpu, int target_cpu,
>>> +             const char *result),
>>> +        TP_ARGS(VCPU_ARGS_COMMON, target_vcpu, target_cpu, result),
>>> +
>>> +        TP_STRUCT__entry(
>>> +            VCPU_FIELD_COMMON
>>> +            __field(int, target_vcpu)
>>> +            __field(int, target_cpu)
>>> +            __field(int, current_cpu)
>>> +            __string(result, result)
>>> +            ),
>>> +
>>> +        TP_fast_assign(
>>> +            VCPU_ASSIGN_COMMON
>>> +            __entry->target_vcpu = target_vcpu;
>>> +            __entry->target_cpu = target_cpu;
>>> +            __entry->current_cpu = smp_processor_id();
>>> +            __assign_str(result);
>>> +            ),
>>> +
>>> +        VCPU_TP_PRINTK(
>>> +               "diag=9c vcpu=%d pcpu=%d target_vcpu=%d target_pcpu=%d result=%s",
>>> +               __entry->id,
>> I think vcpu=%d re-prints __entry->id, which VCPU_TP_PRINTK() already emits as the line prefix; likewise current_cpu/pcpu=%d duplicates ftrace's built-in per-event CPU column.
>> Can you double check?
> 
> Yes it is and I was aware that VCPU_TP_PRINTK already includes the vCPU ID
> in the prefix, but I kept it in the message for readability.
> Having all the relevant fields together in a single format string makes
> the output a bit easier to grep and follow visually I find.
> That said, if you'd prefer to keep it consistent with the other
> tracepoints, I'm happy to drop the duplicate vCPU field.

Yes, please drop.> 
>>
>>> +               __entry->current_cpu,
>>> +               __entry->target_vcpu,
>>> +               __entry->target_cpu,
>>> +               __get_str(result))
>>
>> result is a dynamic __string of three fixed literals — an int plus __print_symbolic() (matching diagnose_codes style in the same file) would also allow splitting "ignored", which currently conflates four distinct causes despite the commit's diagnosability goal.
>> This is not trivial though, due to the VCPU_EVENT which also uses result. So maybe this is not that easy and feel free to ignore that.
>>
> I'm not against changing it to an int plus __print_symbolic() if you
> think that is worth doing?
> The hesitation is of course that VCPU_EVENT() cannot use __print_symbolic()
> directly, so this would also need a small helper to map the result code
> back to a string there. That means a bit more code for fairly limited
> value in this case.
> No issues to change it if you think it is worthwhile, but otherwise
> I'd prefer to keep the current version and avoid the extra helper.

Keep it as is.
Thank you for doing this.