Re: [Xen-devel] [PATCH] use per-cpu variables in cpufreq

To:	Juergen Gross <juergen.gross@xxxxxxxxxxxxxx>
Subject:	Re: [Xen-devel] [PATCH] use per-cpu variables in cpufreq
From:	Keir Fraser <keir@xxxxxxx>
Date:	Sat, 28 May 2011 08:52:49 +0100
Cc:	"Liu, Jinsong" <jinsong.liu@xxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx, mark.langsdorf@xxxxxxx
Delivery-date:	Sat, 28 May 2011 01:22:18 -0700
Dkim-signature:	v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:sender:user-agent:date:subject:from:to:cc :message-id:thread-topic:thread-index:in-reply-to:mime-version :content-type:content-transfer-encoding; bh=TT8ce6Q+4q9aLQxC3s4RiXDNQaJR6C96kWZ7Xg7WPTQ=; b=Mx0X8jX5vW/trR8jY7rf2qsbp2pQ/DuEWAWgFz3o0op9K9X30sjzQoB3LeMR3w/mlU qc0jlX7bECbGwQJrERSb9hDb6qlcdnDNxHiu99ZNw2dKEil2bmXrUsta64J0FupA0Y2z WO2PH87Sg10gfF55XFhF7Pf29YUeJ6HX4ggzs=
Domainkey-signature:	a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=sender:user-agent:date:subject:from:to:cc:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; b=YkH7Zo8r55rRFs8as871TUwIOKUF9fpDpuXoxZvIf3OqMxhm4UNTCtRYvp40OD2GNZ 2JxkTxtkSob8Psa5iNVqsaoLbIDeSu8Qz51hr0BmDId7DSxrTFgA1r1FQ8ndrHwO7m9T 3JMumB/6/HyX6et4IQ0JfBJVFhqksvb1ZNlII=
Envelope-to:	www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to:	<4DDFA72D.2060806@xxxxxxxxxxxxxx>
List-help:	<mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id:	Xen developer discussion <xen-devel.lists.xensource.com>
List-post:	<mailto:xen-devel@lists.xensource.com>
List-subscribe:	<http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe:	<http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender:	xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index:	AcwdDD0rZoaoxIRchEuSuWCL9ZFzgg==
Thread-topic:	[Xen-devel] [PATCH] use per-cpu variables in cpufreq
User-agent:	Microsoft-Entourage/12.29.0.110113

On 27/05/2011 14:29, "Juergen Gross" <juergen.gross@xxxxxxxxxxxxxx> wrote:

> On 05/27/11 15:11, Keir Fraser wrote:
>> On 27/05/2011 12:11, "Juergen Gross"<juergen.gross@xxxxxxxxxxxxxx>  wrote:
>> 
>>> The cpufreq driver used some local arrays indexed by cpu number. This patch
>>> replaces those arrays by per-cpu variables. The AMD and INTEL specific parts
>>> used different per-cpu data structures with nearly identical semantics.
>>> Fold the two structures into one by adding a generic architecture data item.
>> Xen's per-cpu data gets freed across cpu offline/online, whereas cpu-indexed
>> arrays of course do not. Will the cpufreq state be correctly handled across
>> offline/online if we switch to per-cpu vars?
> 
> As far as I could see, yes. The data should only be used for cpus with
> a valid acpid->cpuid translation, which is created when a cpu is going
> online and destroyed when it is going offline again.

That simply isn't true. acpiid_to_apicid[] is populated during boot and
entries are never destroyed.

Specifically, my fear is that this data gets pushed into the hypervisor
once-only during dom0 boot (via XENPF_set_processor_pminfo). If it is freed
during processor offline, we lose it forever and have no power management
when/if a CPU is brought back online. Worse I suspect your patch as it is
will crash if some CPUs are offline during boot as you'll deference their
per_cpu area which doesn't actually exist unless a CPU is online. You can
test this for yourself by adding a maxcpus=1 boot parameter for Xen.

The folding of the Intel/AMD structures might still be interesting, and
probably belongs as a separate patch anyway.

Cc'ing Intel and AMD guys to confirm this.

 -- Keir

> It would be nice, however, if the INTEL and/or AMD code owners could
> give an ack on this...
> 
> 
> Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

WARNING - OLD ARCHIVES

xen-devel

Re: [Xen-devel] [PATCH] use per-cpu variables in cpufreq