linux-kernel - Re: [tip:perf/core] perf/x86: Add generic Intel uncore PMU support

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Thu, 21 Jun 2012 15:43:34 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	mingo@...nel.org, hpa@...or.com, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, a.p.zijlstra@...llo.nl,
	torvalds@...ux-foundation.org, zheng.z.yan@...el.com,
	tglx@...utronix.de
Cc:	"tip-bot for Yan, Zheng" <zheng.z.yan@...el.com>,
	linux-tip-commits@...r.kernel.org, linux-kernel@...r.kernel.org,
	hpa@...or.com, mingo@...nel.org, torvalds@...ux-foundation.org,
	a.p.zijlstra@...llo.nl, tglx@...utronix.de
Subject: Re: [tip:perf/core] perf/x86: Add generic Intel uncore PMU support

On Wed, 20 Jun 2012 03:54:20 -0700
"tip-bot for Yan, Zheng" <zheng.z.yan@...el.com> wrote:

> This patch adds the generic Intel uncore PMU support, including helper
> functions that add/delete uncore events, a hrtimer that periodically
> polls the counters to avoid overflow and code that places all events
> for a particular socket onto a single cpu.
> 
> The code design is based on the structure of Sandy Bridge-EP's uncore
> subsystem, which consists of a variety of components, each component
> contains one or more "boxes".
> 
> (Tooling support follows in the next patches.)
>
> ...
>
> +static enum hrtimer_restart uncore_pmu_hrtimer(struct hrtimer *hrtimer)
> +{
> +	struct intel_uncore_box *box;
> +	unsigned long flags;
> +	int bit;
> +
> +	box = container_of(hrtimer, struct intel_uncore_box, hrtimer);
> +	if (!box->n_active || box->cpu != smp_processor_id())
> +		return HRTIMER_NORESTART;
> +	/*
> +	 * disable local interrupt to prevent uncore_pmu_event_start/stop
> +	 * to interrupt the update process
> +	 */
> +	local_irq_save(flags);
> +
> +	for_each_set_bit(bit, box->active_mask, UNCORE_PMC_IDX_MAX)
> +		uncore_perf_event_update(box, box->events[bit]);
> +
> +	local_irq_restore(flags);
> +
> +	hrtimer_forward_now(hrtimer, ns_to_ktime(UNCORE_PMU_HRTIMER_INTERVAL));
> +	return HRTIMER_RESTART;
> +}

i386 allmodconfig:

arch/x86/kernel/cpu/perf_event_intel_uncore.c: In function 'uncore_pmu_hrtimer':
arch/x86/kernel/cpu/perf_event_intel_uncore.c:728: warning: integer overflow in expression
arch/x86/kernel/cpu/perf_event_intel_uncore.c: In function 'uncore_pmu_start_hrtimer':
arch/x86/kernel/cpu/perf_event_intel_uncore.c:735: warning: integer overflow in expression


Should we even be compiling this code in an i386 build?


Regardless of that, we have some head-scratching to do:


#define UNCORE_PMU_HRTIMER_INTERVAL       (60 * NSEC_PER_SEC)

and

#define NSEC_PER_SEC    1000000000L

and 60 billion doesn't fit in 32 bits.  So do we fix the
perf_event_intel_uncore.c callsites?  Or do we fix the
UNCORE_PMU_HRTIMER_INTERVAL definition?  Or do we fix the NSEC_PER_SEC
definition?

I'm thinking perhaps the latter.  What *is* the type of a nanosecond in
Linux?  include/linux/ktime.h is pretty insistent that it is u64.  If
so, NSEC_PER_SEC should logically have type ULL.  But changing both its
size and signedness is a pretty big change.
--
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/