max_cpus is not available since 20374 changeset (Miscellaneous data placement adjustments). It was moved to __initdata section. This section is freed after Xen initialization. Assume that max_cpus is always equal to XEN_HYPER_SIZE(cpumask_t) * 8. Signed-off-by: Daniel Kiper <daniel.kiper-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> diff -Npru crash-6.0.8.orig/xen_hyper.c crash-6.0.8/xen_hyper.c --- crash-6.0.8.orig/xen_hyper.c 2012-06-29 16:59:18.000000000 +0200 +++ crash-6.0.8/xen_hyper.c 2012-07-05 14:52:59.000000000 +0200 @@ -1879,11 +1879,9 @@ xen_hyper_get_cpu_info(void) uint *cpu_idx; int i, j, cpus; - get_symbol_data("max_cpus", sizeof(xht->max_cpus), &xht->max_cpus); XEN_HYPER_STRUCT_SIZE_INIT(cpumask_t, "cpumask_t"); - if (XEN_HYPER_SIZE(cpumask_t) * 8 > xht->max_cpus) { - xht->max_cpus = XEN_HYPER_SIZE(cpumask_t) * 8; - } + xht->max_cpus = XEN_HYPER_SIZE(cpumask_t) * 8; + if (xht->cpumask) { free(xht->cpumask); }
>>> On 10.08.12 at 15:25, Daniel Kiper <daniel.kiper@oracle.com> wrote: > max_cpus is not available since 20374 changeset (Miscellaneous data > placement adjustments). It was moved to __initdata section. This section > is freed after Xen initialization. Assume that max_cpus is always > equal to XEN_HYPER_SIZE(cpumask_t) * 8.Just to repeat my response to the original version of this patch, which I don''t recall having got any answer from you: "Using nr_cpu_ids, when available, would seem a better fit. And I don''t see why, on dumps from old hypervisors, you wouldn''t want to continue using max_cpus. Oh, wait, I see - you would have to be able to tell whether it actually sits in .init.data, which might not be strait forward." Jan> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> > > diff -Npru crash-6.0.8.orig/xen_hyper.c crash-6.0.8/xen_hyper.c > --- crash-6.0.8.orig/xen_hyper.c 2012-06-29 16:59:18.000000000 +0200 > +++ crash-6.0.8/xen_hyper.c 2012-07-05 14:52:59.000000000 +0200 > @@ -1879,11 +1879,9 @@ xen_hyper_get_cpu_info(void) > uint *cpu_idx; > int i, j, cpus; > > - get_symbol_data("max_cpus", sizeof(xht->max_cpus), &xht->max_cpus); > XEN_HYPER_STRUCT_SIZE_INIT(cpumask_t, "cpumask_t"); > - if (XEN_HYPER_SIZE(cpumask_t) * 8 > xht->max_cpus) { > - xht->max_cpus = XEN_HYPER_SIZE(cpumask_t) * 8; > - } > + xht->max_cpus = XEN_HYPER_SIZE(cpumask_t) * 8; > + > if (xht->cpumask) { > free(xht->cpumask); > }
Daniel Kiper
2012-Aug-13 08:12 UTC
Re: [PATCH v2 1/6] xen: Always calculate max_cpus value
On Fri, Aug 10, 2012 at 03:39:29PM +0100, Jan Beulich wrote:> >>> On 10.08.12 at 15:25, Daniel Kiper <daniel.kiper@oracle.com> wrote: > > max_cpus is not available since 20374 changeset (Miscellaneous data > > placement adjustments). It was moved to __initdata section. This section > > is freed after Xen initialization. Assume that max_cpus is always > > equal to XEN_HYPER_SIZE(cpumask_t) * 8. > > Just to repeat my response to the original version of this patch, > which I don''t recall having got any answer from you: > > "Using nr_cpu_ids, when available, would seem a better fit. And > I don''t see why, on dumps from old hypervisors, you wouldn''t > want to continue using max_cpus. Oh, wait, I see - you would > have to be able to tell whether it actually sits in .init.data, which > might not be strait forward."As I promised earlier I thought about that. The simplest way to do that is to check in which section max_cpus resides. There is some instrumentation in crash tool to do that. However, sadly it does not differentiate between .data and .init.data section. I could write something from scratch which could do that but I think it could have larger costs then potential gains. Let''s leave it as is now. Current approximation is not so bad. However, if any opportunity appears (some functions could differentiate between .data and .init.data section) then I could fix this. Sorry, I should attach above description to original message. Daniel
>>> On 13.08.12 at 10:12, Daniel Kiper <daniel.kiper@oracle.com> wrote: > On Fri, Aug 10, 2012 at 03:39:29PM +0100, Jan Beulich wrote: >> >>> On 10.08.12 at 15:25, Daniel Kiper <daniel.kiper@oracle.com> wrote: >> > max_cpus is not available since 20374 changeset (Miscellaneous data >> > placement adjustments). It was moved to __initdata section. This section >> > is freed after Xen initialization. Assume that max_cpus is always >> > equal to XEN_HYPER_SIZE(cpumask_t) * 8. >> >> Just to repeat my response to the original version of this patch, >> which I don''t recall having got any answer from you: >> >> "Using nr_cpu_ids, when available, would seem a better fit. And >> I don''t see why, on dumps from old hypervisors, you wouldn''t >> want to continue using max_cpus. Oh, wait, I see - you would >> have to be able to tell whether it actually sits in .init.data, which >> might not be strait forward." > > As I promised earlier I thought about that. The simplest way > to do that is to check in which section max_cpus resides. There > is some instrumentation in crash tool to do that. However, sadly > it does not differentiate between .data and .init.data section. > I could write something from scratch which could do that but > I think it could have larger costs then potential gains. > Let''s leave it as is now. Current approximation is not so bad. > However, if any opportunity appears (some functions could > differentiate between .data and .init.data section) then > I could fix this.But minimally you should be using nr_cpu_ids when available. You just have to be prepared for bits beyond that value within any cpumask_t instance to have random contents. Jan
Daniel Kiper
2012-Aug-14 06:17 UTC
Re: [PATCH v2 1/6] xen: Always calculate max_cpus value
On Mon, Aug 13, 2012 at 10:10:19AM +0100, Jan Beulich wrote:> >>> On 13.08.12 at 10:12, Daniel Kiper <daniel.kiper@oracle.com> wrote: > > On Fri, Aug 10, 2012 at 03:39:29PM +0100, Jan Beulich wrote: > >> >>> On 10.08.12 at 15:25, Daniel Kiper <daniel.kiper@oracle.com> wrote: > >> > max_cpus is not available since 20374 changeset (Miscellaneous data > >> > placement adjustments). It was moved to __initdata section. This section > >> > is freed after Xen initialization. Assume that max_cpus is always > >> > equal to XEN_HYPER_SIZE(cpumask_t) * 8. > >> > >> Just to repeat my response to the original version of this patch, > >> which I don''t recall having got any answer from you: > >> > >> "Using nr_cpu_ids, when available, would seem a better fit. And > >> I don''t see why, on dumps from old hypervisors, you wouldn''t > >> want to continue using max_cpus. Oh, wait, I see - you would > >> have to be able to tell whether it actually sits in .init.data, which > >> might not be strait forward." > > > > As I promised earlier I thought about that. The simplest way > > to do that is to check in which section max_cpus resides. There > > is some instrumentation in crash tool to do that. However, sadly > > it does not differentiate between .data and .init.data section. > > I could write something from scratch which could do that but > > I think it could have larger costs then potential gains. > > Let''s leave it as is now. Current approximation is not so bad. > > However, if any opportunity appears (some functions could > > differentiate between .data and .init.data section) then > > I could fix this. > > But minimally you should be using nr_cpu_ids when available. > You just have to be prepared for bits beyond that value within > any cpumask_t instance to have random contents.I will try to improve that after my vacation (on September). Daniel