RE: [PATCH V6] perf: Add PERF_SAMPLE_PHYS_ADDR

[Liang, Kan]

> On Wed, Aug 23, 2017 at 04:33:08PM +0200, Peter Zijlstra wrote:
> > > @@ -6145,6 +6183,9 @@ void perf_prepare_sample(struct
> > > perf_event_header *header,
> > >
> > >  		header->size += size;
> > >  	}
> > > +
> > > +	if (sample_type & PERF_SAMPLE_PHYS_ADDR)
> > > +		data->phys_addr = perf_virt_to_phys(data->addr);
> >
> > Only problem with this now is that it requires SAMPLE_ADDR to also be
> > set in order to obtain data->addr.
> >
> > Either fix all to set data->attr for (SAMPLE_ADDR || SAMPLE_PHYS_ADDR)
> > or mandate SAMPLE_ADDR when SAMPLE_PHYS_ADDR.
> 
> I think the former suggestion is better, as it allows for smaller samples.

For the latter, it's easy to be implemented, and already in the perf tool patch.
But yes, it will bring extra overhead, if the user doesn't care about the virtual
address. 

For the former, do you mean data->type or event->attr?
I don't think we can set either of them. 
It will impact the perf_output_sample and perf_event__header_size.

For x86, I think we can do something as below. But I'm not sure other architectures.
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index a322fed..9bf29dc 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -1175,7 +1175,7 @@ static void setup_pebs_sample_data(struct perf_event *event,
        else
                regs->flags &= ~PERF_EFLAGS_EXACT;

-       if ((sample_type & PERF_SAMPLE_ADDR) &&
+       if ((sample_type & (PERF_SAMPLE_ADDR | PERF_SAMPLE_PHYS_ADDR) &&
            x86_pmu.intel_cap.pebs_format >= 1)
                data->addr = pebs->dla;

Thanks,
Kan