linux-kernel - Re: [PATCH 3/8] ARCv2: perf: implement "event_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150615153831.GF3644@twins.programming.kicks-ass.net>
Date:	Mon, 15 Jun 2015 17:38:31 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
Cc:	linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
	arnd@...db.de, Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
	arc-linux-dev@...opsys.com,
	Arnaldo Carvalho de Melo <acme@...nel.org>
Subject: Re: [PATCH 3/8] ARCv2: perf: implement "event_set_period" for future
 use with interrupts

On Tue, Jun 09, 2015 at 05:49:27PM +0530, Vineet Gupta wrote:
> @@ -99,8 +99,7 @@ static void arc_perf_event_update(struct perf_event *event,
>  	} while (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
>  				 new_raw_count) != prev_raw_count);
>  
> -	delta = (new_raw_count - prev_raw_count) &
> -		((1ULL << arc_pmu->counter_size) - 1ULL);
> +	delta = (new_raw_count - prev_raw_count) & arc_pmu->max_period;

I don't know how your PMU works, but you seem to be assuming new_raw >
prev_raw, which implies its counting up.

Now, typically these things trigger when they reach 0, so when we're
counting up that would mean your values are negative.

In that case, where's the sign extension?

> @@ -182,6 +181,13 @@ static int arc_pmu_event_init(struct perf_event *event)
>  	struct hw_perf_event *hwc = &event->hw;
>  	int ret;
>  
> +	if (!is_sampling_event(event)) {
> +		hwc->sample_period  = arc_pmu->max_period;
> +		hwc->last_period = hwc->sample_period;
> +		local64_set(&hwc->period_left, hwc->sample_period);
> +	} else
> +		return -ENOENT;

-ENOENT is wrong for is_sampling_event().

Either the event is for this PMU, in which case you should return a
fatal error, or its not (-ENOENT is correct in that case).

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