Date: Mon, 7 Dec 2015 10:28:59 +0100
From: Jiri Olsa <jolsa@redhat.com>
To: Stephane Eranian <eranian@google.com>
Cc: linux-kernel@vger.kernel.org, acme@redhat.com, peterz@infradead.org,
        mingo@elte.hu, ak@linux.intel.com, kan.liang@intel.com,
        jolsa@kernel.org
Subject: Re: [PATCH] perf/x86: fix filter_events() bug with event mappings
Message-ID: <20151207092859.GB21484@krava.brq.redhat.com>
References: <1449437706-9210-1-git-send-email-eranian@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1449437706-9210-1-git-send-email-eranian@google.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2028
Lines: 67

On Sun, Dec 06, 2015 at 10:35:06PM +0100, Stephane Eranian wrote:

SNIP

> 
> This problem was introduced into commit 8300daa26.
> 
> Fixes: 8300daa26 ("perf/x86: Filter out undefined events from sysfs events attribute")
> 
> Patch is relative to tip.git commit 03b6b9f.
> 
> Signed-off-by: Stephane Eranian <eranian@google.com>
> ---
>  arch/x86/kernel/cpu/perf_event.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
> index e7e63a9..70e16c7 100644
> --- a/arch/x86/kernel/cpu/perf_event.c
> +++ b/arch/x86/kernel/cpu/perf_event.c
> @@ -1534,6 +1534,7 @@ static void __init filter_events(struct attribute **attrs)
>  {
>  	struct device_attribute *d;
>  	struct perf_pmu_events_attr *pmu_attr;
> +	int offset = 0;
>  	int i, j;
>  
>  	for (i = 0; attrs[i]; i++) {
> @@ -1542,14 +1543,23 @@ static void __init filter_events(struct attribute **attrs)
>  		/* str trumps id */
>  		if (pmu_attr->event_str)
>  			continue;
> -		if (x86_pmu.event_map(i))
> +		if (x86_pmu.event_map(i + offset))
>  			continue;
>  
>  		for (j = i; attrs[j]; j++)
>  			attrs[j] = attrs[j + 1];
> +		attrs[j] = NULL;

hum.. the loop above quits when attrs[j] == NULL,
so I dont think u need to assign it again after

the NULL should get shifted in the last iteration of the loop above

>  
>  		/* Check the shifted attr. */
>  		i--;
> +
> +		/*
> +		 * event_map() is index based, the attrs array is organized
> +		 * by increasing event index. If we shift the events, then
> +		 * we need to compensate for the event_map(), otherwise
> +		 * we are looking up the wrong event in the map
> +		 */
> +		offset++;

ouch, thanks for fixing this

I tested the patch and it fixes the issue

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/