[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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


> 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

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.