[PATCH 11/14] tools/perf: Process the DTL entries in queue and deliver samples

[Athira Rajeev]

> On 27 Aug 2025, at 10:59 PM, Adrian Hunter <adrian.hunter at intel.com> wrote:
> 
> On 15/08/2025 11:34, Athira Rajeev wrote:
>> Create samples from DTL entries for displaying in perf report
>> and perf script. When the different PERF_RECORD_XX records are
>> processed from perf session, powerpc_vpadtl_process_event() will
>> be invoked. For each of the PERF_RECORD_XX record, compare the timestamp
>> of perf record with timestamp of top element in the auxtrace heap.
>> Process the auxtrace queue if the timestamp of element from heap is
>> lower than timestamp from entry in perf record.
>> 
>> Sometimes it could happen that one buffer is only partially
>> processed. if the timestamp of occurrence of another event is more
>> than currently processed element in the queue, it will move on
>> to next perf record. So keep track of position of buffer to
>> continue processing next time. Update the timestamp of the
>> auxtrace heap with the timestamp of last processed entry from
>> the auxtrace buffer.
>> 
>> Generate perf sample for each entry in the dispatch trace log.
>> Fill in the sample details:
>> - sample ip is picked from srr0 field of dtl_entry
>> - sample cpu is picked from processor_id of dtl_entry
>> - sample id is from sample_id of powerpc_vpadtl
>> - cpumode is set to PERF_RECORD_MISC_KERNEL
>> - Additionally save the details in raw_data of sample. This
>> is to print the relevant fields in perf_sample__fprintf_synth()
>> when called from builtin-script
>> 
>> The sample is processed by calling perf_session__deliver_synth_event()
>> so that it gets included in perf report.
>> 
>> Sample Output:
>> 
>>  ./perf record -a -e sched:*,vpa_dtl/dtl_all/ -c 1000000000 sleep 1
>>  [ perf record: Woken up 1 times to write data ]
>>  [ perf record: Captured and wrote 0.300 MB perf.data ]
>> 
>>  ./perf report
>> 
>>  # Samples: 321  of event 'vpa-dtl'
>>  # Event count (approx.): 321
>>  #
>>  # Children      Self  Command  Shared Object      Symbol
>>  # ........  ........  .......  .................  ..............................
>>  #
>>     100.00%   100.00%  swapper  [kernel.kallsyms]  [k] plpar_hcall_norets_notrace
>> 
>> Signed-off-by: Athira Rajeev <atrajeev at linux.ibm.com>
>> ---
>> tools/perf/util/powerpc-vpadtl.c | 181 +++++++++++++++++++++++++++++++
>> 1 file changed, 181 insertions(+)
>> 
>> diff --git a/tools/perf/util/powerpc-vpadtl.c b/tools/perf/util/powerpc-vpadtl.c
>> index 299927901c9d..370c566f9ac2 100644
>> --- a/tools/perf/util/powerpc-vpadtl.c
>> +++ b/tools/perf/util/powerpc-vpadtl.c
>> @@ -160,6 +160,43 @@ static void powerpc_vpadtl_dump_event(struct powerpc_vpadtl *vpa, unsigned char
>> powerpc_vpadtl_dump(vpa, buf, len);
>> }
>> 
>> +/*
>> + * Generate perf sample for each entry in the dispatch trace log.
>> + *   - sample ip is picked from srr0 field of dtl_entry
>> + *   - sample cpu is picked from logical cpu.
>> + *   - sample id is from sample_id of powerpc_vpadtl
>> + *   - cpumode is set to PERF_RECORD_MISC_KERNEL
> 
> Above 4 lines of comments are a bit redundant.
> 
>> + *   - Additionally save the details in raw_data of sample. This
>> + *   is to print the relevant fields in perf_sample__fprintf_synth()
>> + *   when called from builtin-script
>> + */
>> +static int powerpc_vpadtl_sample(struct dtl_entry *record, struct powerpc_vpadtl *vpa, u64 save, int cpu)
>> +{
>> + struct perf_sample sample;
>> + union perf_event event;
>> +
>> + sample.ip = be64_to_cpu(record->srr0);
>> + sample.period = 1;
>> + sample.cpu = cpu;
>> + sample.id = vpa->sample_id;
>> + sample.callchain = NULL;
>> + sample.branch_stack = NULL;
>> + memset(&event, 0, sizeof(event));
>> + sample.cpumode = PERF_RECORD_MISC_KERNEL;
>> + sample.time = save;
>> + sample.raw_data = record;
>> + sample.raw_size = sizeof(record);
>> + event.sample.header.type = PERF_RECORD_SAMPLE;
>> + event.sample.header.misc = sample.cpumode;
>> + event.sample.header.size = sizeof(struct perf_event_header);
>> + if (perf_session__deliver_synth_event(vpa->session, &event,
>> + &sample)) {
> 
> There is some inconsistency with line wrapping
I will handle this properly in V2
> 
>> + pr_debug("Failed to create sample for dtl entry\n");
>> + return -1;
>> + }
>> + return 0;
>> +}
>> +
>> static int powerpc_vpadtl_get_buffer(struct powerpc_vpadtl_queue *vpaq)
>> {
>> struct auxtrace_buffer *buffer = vpaq->buffer;
>> @@ -233,6 +270,148 @@ static int powerpc_vpadtl_decode(struct powerpc_vpadtl_queue *vpaq)
>> return 1;
>> }
>> 
>> +static int powerpc_vpadtl_decode_all(struct powerpc_vpadtl_queue *vpaq)
>> +{
>> + int ret;
>> + unsigned char *buf;
>> +
>> + if (!vpaq->buf_len || (vpaq->pkt_len == vpaq->size)) {
> 
> Unnecessary parentheses around 'vpaq->pkt_len == vpaq->size’
Ok, 
> 
>> + ret = powerpc_vpadtl_get_buffer(vpaq);
>> + if (ret <= 0)
>> + return ret;
>> + }
>> +
>> + if (vpaq->buffer) {
>> + buf = vpaq->buffer->data;
>> + buf += vpaq->pkt_len;
>> + vpaq->dtl = (struct dtl_entry *)buf;
>> + if ((long long)be64_to_cpu(vpaq->dtl->timebase) <= 0) {
>> + if (vpaq->pkt_len != dtl_entry_size && vpaq->buf_len) {
>> + vpaq->pkt_len += dtl_entry_size;
>> + vpaq->buf_len -= dtl_entry_size;
>> + }
>> + return -1;
>> + }
>> + vpaq->pkt_len += dtl_entry_size;
>> + vpaq->buf_len -= dtl_entry_size;
>> + } else
>> + return 0;
> 
> braces {} should be used on all arms of this statement
Sure, 
> 
>> +
>> +
>> + return 1;
>> +}
>> +
>> +static int powerpc_vpadtl_run_decoder(struct powerpc_vpadtl_queue *vpaq, u64 *timestamp)
>> +{
>> + struct powerpc_vpadtl *vpa = vpaq->vpa;
>> + struct dtl_entry *record;
>> + int ret;
>> + double result, div;
>> + double boot_freq = vpaq->tb_freq;
>> + unsigned long long boot_tb = vpaq->boot_tb;
>> + unsigned long long diff;
>> + unsigned long long save;
>> +
>> + while (1) {
>> + ret = powerpc_vpadtl_decode_all(vpaq);
>> + if (!ret) {
>> + pr_debug("All data in the queue has been processed.\n");
>> + return 1;
>> + }
>> +
>> + /*
>> +  * Error is detected when decoding VPA PMU trace. Continue to
>> +  * the next trace data and find out more dtl entries.
>> +  */
>> + if (ret < 0)
>> + continue;
>> +
>> + record = vpaq->dtl;
>> +
>> + diff = be64_to_cpu(record->timebase) - boot_tb;
>> + div = diff / boot_freq;
>> + result = div;
>> + result = result * 1000000000;
>> + save = result;
> 
> It would be nicer for the time calculation to be in a separate function.
> Also 'save' is an odd choice of variable name for a timestamp.

Will have a separate function for time calculation
And will make meaningful name for saving it in V2

Thanks
Athira
> 
>> +
>> + /* Update timestamp for the last record */
>> + if (save > vpaq->timestamp)
>> + vpaq->timestamp = save;
>> +
>> + /*
>> +  * If the timestamp of the queue is later than timestamp of the
>> +  * coming perf event, bail out so can allow the perf event to
>> +  * be processed ahead.
>> +  */
>> + if (vpaq->timestamp >= *timestamp) {
>> + *timestamp = vpaq->timestamp;
>> + vpaq->pkt_len -= dtl_entry_size;
>> + vpaq->buf_len += dtl_entry_size;
>> + return 0;
>> + }
>> +
>> + ret = powerpc_vpadtl_sample(record, vpa, save, vpaq->cpu);
>> + if (ret)
>> + continue;
>> + }
>> + return 0;
>> +}
>> +
>> +/*
>> + * For each of the PERF_RECORD_XX record, compare the timestamp
>> + * of perf record with timestamp of top element in the auxtrace heap.
>> + * Process the auxtrace queue if the timestamp of element from heap is
>> + * lower than timestamp from entry in perf record.
>> + *
>> + * Update the timestamp of the auxtrace heap with the timestamp
>> + * of last processed entry from the auxtrace buffer.
>> + */
>> +static int powerpc_vpadtl_process_queues(struct powerpc_vpadtl *vpa, u64 timestamp)
>> +{
>> + unsigned int queue_nr;
>> + u64 ts;
>> + int ret;
>> +
>> + while (1) {
>> + struct auxtrace_queue *queue;
>> + struct powerpc_vpadtl_queue *vpaq;
>> +
>> + if (!vpa->heap.heap_cnt)
>> + return 0;
>> +
>> + if (vpa->heap.heap_array[0].ordinal >= timestamp)
>> + return 0;
>> +
>> + queue_nr = vpa->heap.heap_array[0].queue_nr;
>> + queue = &vpa->queues.queue_array[queue_nr];
>> + vpaq = queue->priv;
>> +
>> + auxtrace_heap__pop(&vpa->heap);
>> +
>> + if (vpa->heap.heap_cnt) {
>> + ts = vpa->heap.heap_array[0].ordinal + 1;
>> + if (ts > timestamp)
>> + ts = timestamp;
>> + } else
>> + ts = timestamp;
> 
> braces {} should be used on all arms of this statement
> 
>> +
>> + ret = powerpc_vpadtl_run_decoder(vpaq, &ts);
>> + if (ret < 0) {
>> + auxtrace_heap__add(&vpa->heap, queue_nr, ts);
>> + return ret;
>> + }
>> +
>> + if (!ret) {
>> + ret = auxtrace_heap__add(&vpa->heap, queue_nr, ts);
>> + if (ret < 0)
>> + return ret;
>> + } else {
>> + vpaq->on_heap = false;
>> + }
>> + }
>> + return 0;
>> +}
>> +
>> static struct powerpc_vpadtl_queue *powerpc_vpadtl__alloc_queue(struct powerpc_vpadtl *vpa,
>> unsigned int queue_nr)
>> {
>> @@ -368,6 +547,8 @@ static int powerpc_vpadtl_process_event(struct perf_session *session __maybe_unu
>> err = powerpc_vpadtl__update_queues(vpa);
>> if (err)
>> return err;
>> +
>> + err = powerpc_vpadtl_process_queues(vpa, sample->time);
>> }
>> 
>> return err;