Xen devel - Sep 2007 - Re: [PATCH] kexec/kdump: statically allocate xen_phys_cpus

You posted to xen-ia64-devel & xen-ia64 (deleted the wrong word?)
rather than xen-devel so I added that to the CC.

On Thu, 2007-09-20 at 13:38 +0900, Simon Horman wrote:
> On IA64 alloc_bootmem_low() can''t be called this early.
> 
> Before alloc_bootmem_low() can be called init_bootmem(), which is called in
> find_memory(). However xen_machine_kexec_setup_resources() is indirectly
> called earlier on in find_memory().
Can the root cause not be fixed i.e. allow the call to
alloc_bootmem_low() earlier or move the call to
xen_machine_kexec_setup_resource() later, rather than papering over it
by making a potentially large static array?

Ian.
> 
> Cc: Ian Campbell <ian.campbell@xensource.com>
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> ---
> Date: Thu, 20 Sep 2007 13:37:10 +0900
> From: Simon Horman <horms@verge.net.au>
> 
> Posted to xen-devel as it changes common code.
> As suggested by Alex Williamson.
> 
> Index: x/drivers/xen/core/machine_kexec.c
> ==================================================================> ---
x/drivers/xen/core/machine_kexec.c	2007-05-11 18:19:20.000000000 +0900
> +++ x/drivers/xen/core/machine_kexec.c	2007-05-14 19:18:03.000000000 +0900
> @@ -13,7 +13,7 @@ extern void machine_kexec_setup_load_arg
>  
>  static int __initdata xen_max_nr_phys_cpus;
>  static struct resource xen_hypervisor_res;
> -static struct resource *xen_phys_cpus;
> +static struct resource xen_phys_cpus[NR_CPUS];
>  
>  void __init xen_machine_kexec_setup_resources(void)
>  {
> @@ -42,11 +42,6 @@ void xen_machine_kexec_setup_resources(v
>  
>  	xen_max_nr_phys_cpus = k;
>  
> -	/* allocate xen_phys_cpus */
> -
> -	xen_phys_cpus = alloc_bootmem_low(k * sizeof(struct resource));
> -	BUG_ON(xen_phys_cpus == NULL);
> -
>  	/* fill in xen_phys_cpus with per-cpu crash note information */
>  
>  	for (k = 0; k < xen_max_nr_phys_cpus; k++) {

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

On Thu, Sep 20, 2007 at 10:15:01AM +0100, Ian Campbell
wrote:
> You posted to xen-ia64-devel & xen-ia64 (deleted the wrong word?)
> rather than xen-devel so I added that to the CC.
Thanks. I''ve trimmed xen-ia64
> On Thu, 2007-09-20 at 13:38 +0900, Simon Horman wrote:
> > On IA64 alloc_bootmem_low() can''t be called this early.
> > 
> > Before alloc_bootmem_low() can be called init_bootmem(), which is
called in
> > find_memory(). However xen_machine_kexec_setup_resources() is
indirectly
> > called earlier on in find_memory().
> 
> Can the root cause not be fixed i.e. allow the call to
> alloc_bootmem_low() earlier or move the call to
> xen_machine_kexec_setup_resource() later, rather than papering over it
> by making a potentially large static array?
The last time that I looked, it seemed to be quite painful.
But I''ll look over it again, I agree that the static array
should be avoided if possible.
> > Cc: Ian Campbell <ian.campbell@xensource.com>
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> > 
> > ---
> > Date: Thu, 20 Sep 2007 13:37:10 +0900
> > From: Simon Horman <horms@verge.net.au>
> > 
> > Posted to xen-devel as it changes common code.
> > As suggested by Alex Williamson.
> > 
> > Index: x/drivers/xen/core/machine_kexec.c
> > ==================================================================>
> --- x/drivers/xen/core/machine_kexec.c	2007-05-11 18:19:20.000000000 +0900
> > +++ x/drivers/xen/core/machine_kexec.c	2007-05-14 19:18:03.000000000
+0900
> > @@ -13,7 +13,7 @@ extern void machine_kexec_setup_load_arg
> >  
> >  static int __initdata xen_max_nr_phys_cpus;
> >  static struct resource xen_hypervisor_res;
> > -static struct resource *xen_phys_cpus;
> > +static struct resource xen_phys_cpus[NR_CPUS];
> >  
> >  void __init xen_machine_kexec_setup_resources(void)
> >  {
> > @@ -42,11 +42,6 @@ void xen_machine_kexec_setup_resources(v
> >  
> >  	xen_max_nr_phys_cpus = k;
> >  
> > -	/* allocate xen_phys_cpus */
> > -
> > -	xen_phys_cpus = alloc_bootmem_low(k * sizeof(struct resource));
> > -	BUG_ON(xen_phys_cpus == NULL);
> > -
> >  	/* fill in xen_phys_cpus with per-cpu crash note information */
> >  
> >  	for (k = 0; k < xen_max_nr_phys_cpus; k++) {
-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/


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

On Thu, Sep 20, 2007 at 06:38:08PM +0900, Horms wrote:
> On Thu, Sep 20, 2007 at 10:15:01AM +0100, Ian Campbell wrote:
> > You posted to xen-ia64-devel & xen-ia64 (deleted the wrong word?)
> > rather than xen-devel so I added that to the CC.
> 
> Thanks. I''ve trimmed xen-ia64
> 
> > On Thu, 2007-09-20 at 13:38 +0900, Simon Horman wrote:
> > > On IA64 alloc_bootmem_low() can''t be called this early.
> > > 
> > > Before alloc_bootmem_low() can be called init_bootmem(), which is
called in
> > > find_memory(). However xen_machine_kexec_setup_resources() is
indirectly
> > > called earlier on in find_memory().
> > 
> > Can the root cause not be fixed i.e. allow the call to
> > alloc_bootmem_low() earlier or move the call to
> > xen_machine_kexec_setup_resource() later, rather than papering over it
> > by making a potentially large static array?
> 
> The last time that I looked, it seemed to be quite painful.
> But I''ll look over it again, I agree that the static array
> should be avoided if possible.
Hi Ian,

I have found that by changing things around a bit so that
xen_machine_kexec_setup_resource() is called directly in
find_memory() after init_bootmem() the problem that I was
seeing goes away without requiring static allocation.

I''m dropping this patch accordingly.
> > > Cc: Ian Campbell <ian.campbell@xensource.com>
> > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > 
> > > ---
> > > Date: Thu, 20 Sep 2007 13:37:10 +0900
> > > From: Simon Horman <horms@verge.net.au>
> > > 
> > > Posted to xen-devel as it changes common code.
> > > As suggested by Alex Williamson.
> > > 
> > > Index: x/drivers/xen/core/machine_kexec.c
> > >
==================================================================> > >
--- x/drivers/xen/core/machine_kexec.c	2007-05-11 18:19:20.000000000 +0900
> > > +++ x/drivers/xen/core/machine_kexec.c	2007-05-14
19:18:03.000000000 +0900
> > > @@ -13,7 +13,7 @@ extern void machine_kexec_setup_load_arg
> > >  
> > >  static int __initdata xen_max_nr_phys_cpus;
> > >  static struct resource xen_hypervisor_res;
> > > -static struct resource *xen_phys_cpus;
> > > +static struct resource xen_phys_cpus[NR_CPUS];
> > >  
> > >  void __init xen_machine_kexec_setup_resources(void)
> > >  {
> > > @@ -42,11 +42,6 @@ void xen_machine_kexec_setup_resources(v
> > >  
> > >  	xen_max_nr_phys_cpus = k;
> > >  
> > > -	/* allocate xen_phys_cpus */
> > > -
> > > -	xen_phys_cpus = alloc_bootmem_low(k * sizeof(struct resource));
> > > -	BUG_ON(xen_phys_cpus == NULL);
> > > -
> > >  	/* fill in xen_phys_cpus with per-cpu crash note information */
> > >  
> > >  	for (k = 0; k < xen_max_nr_phys_cpus; k++) {
> 
> -- 
> Horms
>   H: http://www.vergenet.net/~horms/
>   W: http://www.valinux.co.jp/en/
> 
-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/


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