Messages in this thread | | | Subject | Re: [Patch v1 04/10] perf/x86: add memory profiling via PEBS Load Latency | From | Peter Zijlstra <> | Date | Mon, 29 Oct 2012 16:38:32 +0100 |
| |
On Mon, 2012-10-29 at 16:15 +0100, Stephane Eranian wrote: > + fll = event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT; > + > perf_sample_data_init(&data, 0, event->hw.last_period); > > + data.period = event->hw.last_period; > + sample_type = event->attr.sample_type; > + > + /* > + * if PEBS-LL or PreciseStore > + */ > + if (fll) { > + if (sample_type & PERF_SAMPLE_ADDR) > + data.addr = pebs->dla; > + > + /* > + * Use latency for cost (only avail with PEBS-LL) > + */ > + if (fll && (sample_type & PERF_SAMPLE_COST)) > + data.cost = pebs->lat; > + > + /* > + * data.dsrc encodes the data source > + */ > + if (sample_type & PERF_SAMPLE_DSRC) { > + if (fll) > + data.dsrc.val = load_latency_data(pebs->dse); > + } > + }
Why does fl1 exist? the above looks really convoluted, all fl1 checks inside are pointless. Also 'fl1' is a bizarre name, 'pebs_ll' would be a better name I guess.
What's wrong with:
if (event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT) { if (sample_type & PERF_SAMPLE_ADDR) data.addr = pebs->dla;
if (sample_type & PERF_SAMPLE_COST) data.cost = perf->lat;
if (sample_type & PERF_SAMPLE_DSRC) data.dsrc.val = load_latency_data(pebs->dse); }
| |