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

[Keir Fraser <keir@xxxxxxx>]

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