WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: "Tian, Kevin" <kevin.tian@xxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Juergen Gross <juergen.gross@xxxxxxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH] use per-cpu variables in cpufreq
From: "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>
Date: Mon, 30 May 2011 23:33:58 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "mark.langsdorf@xxxxxxx" <mark.langsdorf@xxxxxxx>
Delivery-date: Mon, 30 May 2011 11:18:22 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <625BA99ED14B2D499DC4E29D8138F1505CA61C0472@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
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>
References: <4DDFA72D.2060806@xxxxxxxxxxxxxx> <CA066861.2E032%keir@xxxxxxx> <625BA99ED14B2D499DC4E29D8138F1505CA61C0472@xxxxxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcwdDD0rZoaoxIRchEuSuWCL9ZFzggBk8vZwAA+O6SA=
Thread-topic: [Xen-devel] [PATCH] use per-cpu variables in cpufreq
Tian, Kevin wrote:
>> From: Keir Fraser
>> Sent: Saturday, May 28, 2011 3:53 PM
>> 
>> 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 
> 
> exactly. Suppose many of those arrays are only referenced at driver
> initialization phase, or not referenced frequently. Not to use percpu
> variable here is just 
> fine unless you find some actual hot lines which then can be fixed
> accordingly. 
> 
> Thanks
> Kevin

Agree Keir and Kevin.
Static array during boot maybe simple and reliable.

> 
>> 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... 

It's fine to me to merge acpi_cpufreq_data and powernow_cpufreq_data.
In fact acpi_cpufreq_data can be commonly used by both Intel and AMD cpufreq 
logic. 
I didn't image the benefit of adding a similar struct powernow_cpufreq_data.

Thanks,
Jinsong

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


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