mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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
Date: Mon, 7 Dec 2015 10:28:59 +0100	[thread overview]
Message-ID: <20151207092859.GB21484@krava.brq.redhat.com> (raw)
In-Reply-To: <1449437706-9210-1-git-send-email-eranian@google.com>

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

      reply	other threads:[~2015-12-07  9:29 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-06 21:35 Stephane Eranian
2015-12-07  9:28 ` Jiri Olsa [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151207092859.GB21484@krava.brq.redhat.com \
    --to=jolsa@redhat.com \
    --cc=acme@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=eranian@google.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®