Messages in this thread | | | From | "Dey, Megha" <> | Subject | RE: [PATCH V0 2/3] perf/x86/intel/bm.c: Add Intel Branch Monitoring support | Date | Sat, 11 Nov 2017 00:53:33 +0000 |
| |
>-----Original Message----- >From: Jiri Olsa [mailto:jolsa@redhat.com] >Sent: Saturday, November 4, 2017 6:26 AM >To: Megha Dey <megha.dey@linux.intel.com> >Cc: x86@kernel.org; linux-kernel@vger.kernel.org; linux- >doc@vger.kernel.org; tglx@linutronix.de; mingo@redhat.com; >hpa@zytor.com; andriy.shevchenko@linux.intel.com; >kstewart@linuxfoundation.org; Yu, Yu-cheng <yu-cheng.yu@intel.com>; >Brown, Len <len.brown@intel.com>; gregkh@linuxfoundation.org; >peterz@infradead.org; acme@kernel.org; >alexander.shishkin@linux.intel.com; namhyung@kernel.org; >vikas.shivappa@linux.intel.com; pombredanne@nexb.com; >me@kylehuey.com; bp@suse.de; Andrejczuk, Grzegorz ><grzegorz.andrejczuk@intel.com>; Luck, Tony <tony.luck@intel.com>; >corbet@lwn.net; Shankar, Ravi V <ravi.v.shankar@intel.com>; Dey, Megha ><megha.dey@intel.com> >Subject: Re: [PATCH V0 2/3] perf/x86/intel/bm.c: Add Intel Branch >Monitoring support > >On Fri, Nov 03, 2017 at 11:00:05AM -0700, Megha Dey wrote: > >SNIP > >> + >> +static void intel_bm_event_update(struct perf_event *event) { >> + union bm_detect_status cur_stat; >> + >> + rdmsrl(BR_DETECT_STATUS_MSR, cur_stat.raw); >> + local64_set(&event->hw.prev_count, (uint64_t)cur_stat.raw); } >> + >> +static void intel_bm_event_stop(struct perf_event *event, int mode) { >> + wrmsrl(BR_DETECT_COUNTER_CONFIG_BASE + event->id, >> + (event->hw.bm_counter_conf & ~1)); >> + >> + intel_bm_event_update(event); >> +} >> + >> +static void intel_bm_event_del(struct perf_event *event, int flags) { >> + intel_bm_event_stop(event, flags); >> +} >> + >> +static void intel_bm_event_read(struct perf_event *event) { } > >should you call intel_bm_event_update in here? so the read syscall gets >updated data in case the case the event is active
We do not update the event->count in the intel_bm_event_update function. We are basically saving the status register contents when a task is being scheduled out. So it has nothing to do with the read syscall. Having said that, I will probably put what stop() does in del() and get rid of the stop() function. > >jirka
| |