Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Fri, 21 Feb 2020 14:18:54 +0100
From:   Jiri Olsa <jolsa@redhat.com>
To:     "Liang, Kan" <kan.liang@linux.intel.com>
Cc:     acme@kernel.org, mingo@redhat.com, peterz@infradead.org,
        linux-kernel@vger.kernel.org, mark.rutland@arm.com,
        namhyung@kernel.org, ravi.bangoria@linux.ibm.com,
        yao.jin@linux.intel.com, ak@linux.intel.com
Subject: Re: [PATCH 0/5] Support metric group constraint
Message-ID: <20200221131854.GD652992@krava>
References: <1582139320-75181-1-git-send-email-kan.liang@linux.intel.com>
 <20200220113924.GB565976@krava>
 <534b4b99-466a-0a5b-e9f5-b4711abd8a4a@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <534b4b99-466a-0a5b-e9f5-b4711abd8a4a@linux.intel.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Feb 20, 2020 at 11:03:35AM -0500, Liang, Kan wrote:
> 
> 
> On 2/20/2020 6:39 AM, Jiri Olsa wrote:
> > On Wed, Feb 19, 2020 at 11:08:35AM -0800, kan.liang@linux.intel.com wrote:
> > > From: Kan Liang <kan.liang@linux.intel.com>
> > > 
> > > Some metric groups, e.g. Page_Walks_Utilization, will never count when
> > > NMI watchdog is enabled.
> > > 
> > >   $echo 1 > /proc/sys/kernel/nmi_watchdog
> > >   $perf stat -M Page_Walks_Utilization
> > > 
> > >   Performance counter stats for 'system wide':
> > > 
> > >   <not counted>      itlb_misses.walk_pending       (0.00%)
> > >   <not counted>      dtlb_load_misses.walk_pending  (0.00%)
> > >   <not counted>      dtlb_store_misses.walk_pending (0.00%)
> > >   <not counted>      ept.walk_pending               (0.00%)
> > >   <not counted>      cycles                         (0.00%)
> > > 
> > >         2.343460588 seconds time elapsed
> > > 
> > >   Some events weren't counted. Try disabling the NMI watchdog:
> > >          echo 0 > /proc/sys/kernel/nmi_watchdog
> > >          perf stat ...
> > >          echo 1 > /proc/sys/kernel/nmi_watchdog
> > >   The events in group usually have to be from the same PMU. Try
> > >   reorganizing the group.
> > > 
> > > A metric group is a weak group, which relies on group validation
> > > code in the kernel to determine whether to be opened as a group or
> > > a non-group. However, group validation code may return false-positives,
> > > especially when NMI watchdog is enabled. (The metric group is allowed
> > > as a group but will never be scheduled.)
> > > 
> > > The attempt to fix the group validation code has been rejected.
> > > https://lore.kernel.org/lkml/20200117091341.GX2827@hirez.programming.kicks-ass.net/
> > > Because we cannot accurately predict whether the group can be scheduled
> > > as a group, only by checking current status.
> > > 
> > > This patch set provides another solution to mitigate the issue.
> > > Add "MetricConstraint" in event list, which provides a hint for perf tool,
> > > e.g. "MetricConstraint": "NO_NMI_WATCHDOG". Perf tool can change the
> > > metric group to non-group (standalone metrics) if NMI watchdog is enabled.
> > 
> > the problem is in the missing counter, that's taken by NMI watchdog, right?
> > 
> > and it's problem for any metric that won't fit to the available
> > counters.. shouldn't we rather do this workaround for any metric
> > that wouldn't fit in available counters?
> 
> I think current perf already did this.
> All metric groups are weak group. Kernel (validate_group()) tells perf tool
> whether a metric group fit to available counters.
> If yes, the metric group will be scheduled as a group.
> If no, perf tool will not using a group and re-try. The code is as below.
> 
>  try_again:
>  		if (create_perf_stat_counter(counter, &stat_config, &target) < 0) {
> 
>  			/* Weak group failed. Reset the group. */
>  			if ((errno == EINVAL || errno == EBADF) &&
>  			    counter->leader != counter &&
>  			    counter->weak_group) {
>  				counter = perf_evlist__reset_weak_group(evsel_list, counter);
>  				goto try_again;
>  			}
> 
> 
> This patch set is to workaroud the false-positives from the kernel.

ah so validate_group will say the group can go on, but then
the pmu add will fail because the countters are occupied

thanks for explanation, looks good

jirka