From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755286AbdKKAxi convert rfc822-to-8bit (ORCPT ); Fri, 10 Nov 2017 19:53:38 -0500 Received: from mga02.intel.com ([134.134.136.20]:34284 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755243AbdKKAxf (ORCPT ); Fri, 10 Nov 2017 19:53:35 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,377,1505804400"; d="scan'208";a="6377717" From: "Dey, Megha" To: Jiri Olsa , Megha Dey 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" , "Brown, Len" , "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" , "Luck, Tony" , "corbet@lwn.net" , "Shankar, Ravi V" Subject: RE: [PATCH V0 2/3] perf/x86/intel/bm.c: Add Intel Branch Monitoring support Thread-Topic: [PATCH V0 2/3] perf/x86/intel/bm.c: Add Intel Branch Monitoring support Thread-Index: AQHTVMuJ78tDWbmRNES2UyAi9yp0nKMErLeAgAm3zPA= Date: Sat, 11 Nov 2017 00:53:33 +0000 Message-ID: References: <1509732006-5917-1-git-send-email-megha.dey@linux.intel.com> <1509732006-5917-3-git-send-email-megha.dey@linux.intel.com> <20171104132611.GE8454@krava> In-Reply-To: <20171104132611.GE8454@krava> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYTQxNzMzNDMtMjRjZi00YzVjLTg5ODQtZGI4NDFlNTVlYzA3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkJsS041c0d4ZXMzN0FFN0xSa3JDbHdLK3g3cWdBXC9kdWluY2xkblgwRlBJPSJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.22.254.140] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >-----Original Message----- >From: Jiri Olsa [mailto:jolsa@redhat.com] >Sent: Saturday, November 4, 2017 6:26 AM >To: Megha Dey >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 ; >Brown, Len ; 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 >; Luck, Tony ; >corbet@lwn.net; Shankar, Ravi V ; Dey, Megha > >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