Messages in this thread | | | Date | Thu, 6 Sep 2018 16:38:36 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH V2 5/6] x86/intel_rdt: Use perf infrastructure for measurements |
| |
On Thu, Aug 16, 2018 at 01:16:08PM -0700, Reinette Chatre wrote: > + l2_miss_event = perf_event_create_kernel_counter(&perf_miss_attr, > + plr->cpu, > + NULL, NULL, NULL); > + if (IS_ERR(l2_miss_event)) > + goto out; > + > + l2_hit_event = perf_event_create_kernel_counter(&perf_hit_attr, > + plr->cpu, > + NULL, NULL, NULL); > + if (IS_ERR(l2_hit_event)) > + goto out_l2_miss; > + > + local_irq_disable(); > + /* > + * Check any possible error state of events used by performing > + * one local read. > + */ > + if (perf_event_read_local(l2_miss_event, &tmp, NULL, NULL)) { > + local_irq_enable(); > + goto out_l2_hit; > + } > + if (perf_event_read_local(l2_hit_event, &tmp, NULL, NULL)) { > + local_irq_enable(); > + goto out_l2_hit; > + } > + > + /* > + * Disable hardware prefetchers. > * > + * Call wrmsr direcly to avoid the local register variables from > + * being overwritten due to reordering of their assignment with > + * the wrmsr calls. > + */ > + __wrmsr(MSR_MISC_FEATURE_CONTROL, prefetch_disable_bits, 0x0);
So what about virt?
> + > + /* Initialize rest of local variables */ > + /* > + * Performance event has been validated right before this with > + * interrupts disabled - it is thus safe to read the counter index. > + */ > + l2_miss_pmcnum = x86_perf_rdpmc_ctr_get(l2_miss_event); > + l2_hit_pmcnum = x86_perf_rdpmc_ctr_get(l2_hit_event); > + line_size = plr->line_size; > + mem_r = plr->kmem; > + size = plr->size;
You probably want READ_ONCE() on that, the volatile cast in there disallows the compiler from re-loading the values later.
> + > + /* > + * Read counter variables twice - first to load the instructions > + * used in L1 cache, second to capture accurate value that does not > + * include cache misses incurred because of instruction loads. > + */ > + rdpmcl(l2_hit_pmcnum, l2_hits_before);
And this again does do virt.
> + rdpmcl(l2_miss_pmcnum, l2_miss_before); > + /* > + * From SDM: Performing back-to-back fast reads are not guaranteed > + * to be monotonic. To guarantee monotonicity on back-toback reads, > + * a serializing instruction must be placed between the two > + * RDPMC instructions > + */ > + rmb();
You're copying the horrid horrid (did I say truly horrid?) use of 'serializing' from the SDM. Please don't do that.
LFENCE is not a serializing instruction. But given the (new) definition LFENCE does ensure all prior instructions are retired before it proceeds.
> + rdpmcl(l2_hit_pmcnum, l2_hits_before); > + rdpmcl(l2_miss_pmcnum, l2_miss_before); > + /* > + * rdpmc is not a serializing instruction. Add barrier to prevent > + * instructions that follow to begin executing before reading the > + * counter value. > + */ > + rmb(); > + for (i = 0; i < size; i += line_size) { > + /* > + * Add a barrier to prevent speculative execution of this > + * loop reading beyond the end of the buffer. > + */ > + rmb(); > + asm volatile("mov (%0,%1,1), %%eax\n\t" > + : > + : "r" (mem_r), "r" (i) > + : "%eax", "memory");
Why does that need to be asm?
> + }
I think you want another LFENCE here, to ensure the RDPMCs don't overlap with the last LOAD in the loop above.
> + rdpmcl(l2_hit_pmcnum, l2_hits_after); > + rdpmcl(l2_miss_pmcnum, l2_miss_after); > + /* > + * rdpmc is not a serializing instruction. Add barrier to ensure > + * events measured have completed and prevent instructions that > + * follow to begin executing before reading the counter value. > + */ > + rmb(); > + /* Re-enable hardware prefetchers */ > + wrmsr(MSR_MISC_FEATURE_CONTROL, 0x0, 0x0);
So what I do in userspace is:
mmap_read_pinned(ctx); /* prime */
for (many-times) {
cnt = mmap_read_pinned(evt); barrier(); cnt = mmap_read_pinned(evt) - cnt; update_stats(&empty, cnt);
cnt = mmap_read_pinned(evt); barrier(); /* the thing */ barrier(); cnt = mmap_read_pinned(evt) - cnt; update_stats(&stat, cnt);
}
sub_stats(&stat, &empty);
Maybe I should've used asm("lfence" ::: "memory") instead of barrier(), but the results were good enough.
| |