Re: [PATCH] trace-cmd: rework of the pid detection of vcpus

[Steven Rostedt <rostedt@goodmis.org>]

On Wed,  4 May 2022 01:02:42 +0000
Vineeth Pillai <vineethrp@google.com> wrote:


>  /* --- System --- */
>  unsigned long long tracecmd_generate_traceid(void);
>  int tracecmd_count_cpus(void);
> +int tracecmd_count_cpus_apicid(int *max_apicid);
>  
>  /* --- Hack! --- */
>  int tracecmd_blk_hack(struct tracecmd_input *handle);
> diff --git a/lib/trace-cmd/trace-msg.c b/lib/trace-cmd/trace-msg.c
> index 39465ad..1bb56a0 100644
> --- a/lib/trace-cmd/trace-msg.c
> +++ b/lib/trace-cmd/trace-msg.c
> @@ -73,6 +73,7 @@ struct tracecmd_msg_trace_req {
>  struct tracecmd_msg_trace_resp {
>  	be32 flags;
>  	be32 cpus;
> +	be32 max_apicid;
>  	be32 page_size;
>  	u64 trace_id;
>  	char tsync_proto_name[TRACECMD_TSYNC_PNAME_LENGTH];

Unfortunately, the above will break the protocol for released instances of
trace-cmd that is already out there. One requirement I have is that if two
instances of trace-cmd use to work (on host and guest) that if you upgrade
one of them, what use to work still does.

We need to figure out another way to handle this :-/


>  static void stop_mapping_vcpus(struct buffer_instance *instance,
> @@ -3961,21 +3959,19 @@ static void stop_mapping_vcpus(struct buffer_instance *instance,
>  	struct trace_mapping tmap = { };
>  	struct tep_handle *tep;
>  	const char *systems[] = { "kvm", NULL };
> -	int i;
>  
>  	if (!guest->instance)
>  		return;
>  
>  	tmap.pids = guest->task_pids;
> +	tmap.nr_cpus = 0;
>  	tmap.max_cpus = instance->cpu_count;
> +	tmap.max_apicid = instance->max_apicid;
>  
> -	tmap.map = malloc(sizeof(*tmap.map) * tmap.max_cpus);
> +	tmap.map = calloc((tmap.max_apicid + 1), sizeof(*tmap.map));
>  	if (!tmap.map)
>  		return;
>  
> -	for (i = 0; i < tmap.max_cpus; i++)
> -		tmap.map[i] = -1;

Note, we wrote this with -1, because we envisioned working with hypervisors
that may not be Linux, and that PID = 0 may be legitimate.

> -
>  	tracefs_instance_file_write(guest->instance, "events/kvm/kvm_entry/enable", "0");
>  
>  	tep = tracefs_local_events_system(NULL, systems);

We'll have to investigate a solution for this further.

-- Steve