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

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

Am 25.06.26 um 15:16 schrieb Ciunas Bennett:
> Add a new tracepoint kvm_s390_diag_9c to provide detailed observability
> for directed yield operations. The tracepoint captures:
> - Source and target VCPU IDs
> - Current and target physical CPU numbers
> - Operation result (done, ignored, yield forwarded)
> 
> This enables better analysis of VCPU scheduling behaviour and helps
> diagnose performance issues related to directed yields in virtualised
> s390 environments.

Nothing critical, just some ideas.

> 
> Signed-off-by: Ciunas Bennett <ciunas@linux.ibm.com>
> ---
>   arch/s390/kvm/diag.c  |  1 +
>   arch/s390/kvm/trace.h | 30 ++++++++++++++++++++++++++++++
>   2 files changed, 31 insertions(+)
> 
> diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> index 85c84421b510..031ab6e5d6c4 100644
> --- a/arch/s390/kvm/diag.c
> +++ b/arch/s390/kvm/diag.c
> @@ -228,6 +228,7 @@ static int __diag_time_slice_end_directed(struct kvm_vcpu *vcpu)
>   out:
>   	VCPU_EVENT(vcpu, 5, "diag time slice end directed to %d: %s", tid,
>   		   result);
> +	trace_kvm_s390_diag_9c(vcpu, tid, tcpu_cpu, result);
>   	return 0;
>   }
>   
> diff --git a/arch/s390/kvm/trace.h b/arch/s390/kvm/trace.h
> index aa419eb6a0c8..2d6da21f590c 100644
> --- a/arch/s390/kvm/trace.h
> +++ b/arch/s390/kvm/trace.h
> @@ -283,6 +283,36 @@ TRACE_EVENT(kvm_s390_handle_diag,
>   			   __print_symbolic(__entry->code, diagnose_codes))
>   	);
>   
> +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?

> +		       __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.

> +	);
> +
>   TRACE_EVENT(kvm_s390_handle_lctl,
>   	    TP_PROTO(VCPU_PROTO_COMMON, int g, int reg1, int reg3, u64 addr),
>   	    TP_ARGS(VCPU_ARGS_COMMON, g, reg1, reg3, addr),