static and __init symbols in machine_kexec.c

static and __init symbols in machine_kexec.c

[Simon Horman]

As per the patch, some symbols that really ought to be __init or static aren't.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 
 linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Curiously, making xen_max_nr_phys_cpus __init causes a section missmatch
error at compile time. I guess this is to do with the way global ints
are handled. I confirmed that its consistent across x86_32, x86_64 and
ia64. So I guess its ok to leave it as is.

/home/horms/work/xen/xen-ia64-unstable.hg/linux-2.6.18-xen/drivers/xen/core/machine_kexec.c:14:
error: xen_max_nr_phys_cpus causes a section type conflict

Index: xen-ia64-unstable.hg/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c
===================================================================
--- xen-ia64-unstable.hg.orig/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c	2007-05-16 15:46:33.000000000 +0900
+++ xen-ia64-unstable.hg/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c	2007-05-16 15:54:43.000000000 +0900
@@ -11,11 +11,11 @@
 extern void machine_kexec_setup_load_arg(xen_kexec_image_t *xki, 
 					 struct kimage *image);
 
-int xen_max_nr_phys_cpus;
-struct resource xen_hypervisor_res;
-struct resource *xen_phys_cpus;
+static int xen_max_nr_phys_cpus;
+static struct __init resource xen_hypervisor_res;
+static struct __init resource *xen_phys_cpus;
 
-void xen_machine_kexec_setup_resources(void)
+void __init xen_machine_kexec_setup_resources(void)
 {
 	xen_kexec_range_t range;
 	struct resource *res;
@@ -104,7 +104,7 @@ void xen_machine_kexec_setup_resources(v
 	return;
 }
 
-void xen_machine_kexec_register_resources(struct resource *res)
+void __init xen_machine_kexec_register_resources(struct resource *res)
 {
 	int k;

[Patch v2] static and __init symbols in machine_kexec.c

[Simon Horman]

As per the patch, some symbols that really ought to be __init or static aren't.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 
 linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Curiously, making xen_max_nr_phys_cpus __init causes a section missmatch
error at compile time. I guess this is to do with the way global ints
are handled. I confirmed that its consistent across x86_32, x86_64 and
ia64. So I guess its ok to leave it as is.

/home/horms/work/xen/xen-ia64-unstable.hg/linux-2.6.18-xen/drivers/xen/core/machine_kexec.c:14:
error: xen_max_nr_phys_cpus causes a section type conflict

Update: put __init somehere a bit less silly for the struct resource 
        symbols

Index: xen-ia64-unstable.hg/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c
===================================================================
--- xen-ia64-unstable.hg.orig/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c	2007-05-16 15:46:33.000000000 +0900
+++ xen-ia64-unstable.hg/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c	2007-05-16 15:54:43.000000000 +0900
@@ -11,11 +11,11 @@
 extern void machine_kexec_setup_load_arg(xen_kexec_image_t *xki, 
 					 struct kimage *image);
 
-int xen_max_nr_phys_cpus;
-struct resource xen_hypervisor_res;
-struct resource *xen_phys_cpus;
+static int xen_max_nr_phys_cpus;
+static struct resource __init xen_hypervisor_res;
+static struct resource __init *xen_phys_cpus;
 
-void xen_machine_kexec_setup_resources(void)
+void __init xen_machine_kexec_setup_resources(void)
 {
 	xen_kexec_range_t range;
 	struct resource *res;
@@ -104,7 +104,7 @@ void xen_machine_kexec_setup_resources(v
 	return;
 }
 
-void xen_machine_kexec_register_resources(struct resource *res)
+void __init xen_machine_kexec_register_resources(struct resource *res)
 {
 	int k;

Re: [Patch v2] static and __init symbols in machine_kexec.c

[Keir Fraser]

On 16/5/07 07:49, "Simon Horman" <horms@verge.net.au> wrote:

> Curiously, making xen_max_nr_phys_cpus __init causes a section missmatch
> error at compile time. I guess this is to do with the way global ints
> are handled. I confirmed that its consistent across x86_32, x86_64 and
> ia64. So I guess its ok to leave it as is.

What if you make it __initdata, as it (and the other data variables) should
be?

 -- Keir

Re: [Patch v2] static and __init symbols in machine_kexec.c

[Simon Horman]

On Wed, May 16, 2007 at 08:21:41AM +0100, Keir Fraser wrote:
> 
> 
> 
> On 16/5/07 07:49, "Simon Horman" <horms@verge.net.au> wrote:
> 
> > Curiously, making xen_max_nr_phys_cpus __init causes a section missmatch
> > error at compile time. I guess this is to do with the way global ints
> > are handled. I confirmed that its consistent across x86_32, x86_64 and
> > ia64. So I guess its ok to leave it as is.
> 
> What if you make it __initdata, as it (and the other data variables) should
> be?

I'll take a look into that and get back to you.

-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/

[Patch v3] static, __init and __initdata symbols in machine_kexec.c

[Simon Horman, Keir Fraser]

Some symbols in machine_kexec.c really ought to be static and,
marked __init or __initdata.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 
 linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Update: put __init somehere a bit less silly for
	the struct resource symbols

Update: Use __initdata instead of __init for variables,
	as suggested by Keir Fraser

Index: xen-ia64-unstable.hg/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c
===================================================================
--- xen-ia64-unstable.hg.orig/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c	2007-05-16 16:37:04.000000000 +0900
+++ xen-ia64-unstable.hg/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c	2007-05-16 16:44:26.000000000 +0900
@@ -11,11 +11,11 @@
 extern void machine_kexec_setup_load_arg(xen_kexec_image_t *xki, 
 					 struct kimage *image);
 
-int xen_max_nr_phys_cpus;
-struct resource xen_hypervisor_res;
-struct resource *xen_phys_cpus;
+static int __initdata xen_max_nr_phys_cpus;
+static struct resource __initdata xen_hypervisor_res;
+static struct resource __initdata *xen_phys_cpus;
 
-void xen_machine_kexec_setup_resources(void)
+void __init xen_machine_kexec_setup_resources(void)
 {
 	xen_kexec_range_t range;
 	struct resource *res;
@@ -104,7 +104,7 @@ void xen_machine_kexec_setup_resources(v
 	return;
 }
 
-void xen_machine_kexec_register_resources(struct resource *res)
+void __init xen_machine_kexec_register_resources(struct resource *res)
 {
 	int k;

[Patch v4] static, __init and __initdata symbols in machine_kexec.c

[Simon Horman, Keir Fraser]

Some symbols in machine_kexec.c really ought to be static or,
marked __init or __initdata.

Signed-off-by: Simon Horman <horms@verge.net.au>

--- 
 linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Update: put __init somehere a bit less silly for
	the struct resource symbols

Update: Use __initdata instead of __init for variables,
	as suggested by Keir Fraser

Update: On further investigation it seems that xen_hypervisor_res and
	xen_phys_cpus should not be __initdata as they  are inserted into
	iomem_resource and can be accessed via /proc/iomem's proc handler,
	and likely other palces too.

Index: xen-ia64-unstable.hg/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c
===================================================================
--- xen-ia64-unstable.hg.orig/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c	2007-05-16 17:30:50.000000000 +0900
+++ xen-ia64-unstable.hg/linux-2.6-xen-sparse/drivers/xen/core/machine_kexec.c	2007-05-16 17:31:36.000000000 +0900
@@ -11,11 +11,11 @@
 extern void machine_kexec_setup_load_arg(xen_kexec_image_t *xki, 
 					 struct kimage *image);
 
-int xen_max_nr_phys_cpus;
-struct resource xen_hypervisor_res;
-struct resource *xen_phys_cpus;
+static int __initdata xen_max_nr_phys_cpus;
+static struct resource xen_hypervisor_res;
+static struct resource *xen_phys_cpus;
 
-void xen_machine_kexec_setup_resources(void)
+void __init xen_machine_kexec_setup_resources(void)
 {
 	xen_kexec_range_t range;
 	struct resource *res;
@@ -104,7 +104,7 @@ void xen_machine_kexec_setup_resources(v
 	return;
 }
 
-void xen_machine_kexec_register_resources(struct resource *res)
+void __init xen_machine_kexec_register_resources(struct resource *res)
 {
 	int k;

Re: [Patch v3] static, __init and __initdata symbols in machine_kexec.c

[Ian Campbell]

On Wed, 2007-05-16 at 15:49 +0900, Simon Horman wrote:
> Some symbols in machine_kexec.c really ought to be static and,
> marked __init or __initdata.

Applied, thanks.

Ian.

Re: [Patch v3] static, __init and __initdata symbols in machine_kexec.c

[Simon Horman]

On Wed, May 16, 2007 at 09:16:36AM +0100, Ian Campbell wrote:
> On Wed, 2007-05-16 at 15:49 +0900, Simon Horman wrote:
> > Some symbols in machine_kexec.c really ought to be static and,
> > marked __init or __initdata.
> 
> Applied, thanks.

Sorry, I was a bit hasty with sending in this patch :(

As per the subseqent v4, I discovered that xen_hypervisor_res and
xen_phys_cpus really ought not to be __initdata because they get
inserted into resource_iomem, which is accessed after init. Should I
send in an additional patch to reverse that part of the change?

-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/

Re: [Patch v3] static, __init and __initdata symbols in machine_kexec.c

[Keir Fraser]

On 16/5/07 10:07, "Simon Horman" <horms@verge.net.au> wrote:

> Sorry, I was a bit hasty with sending in this patch :(
> 
> As per the subseqent v4, I discovered that xen_hypervisor_res and
> xen_phys_cpus really ought not to be __initdata because they get
> inserted into resource_iomem, which is accessed after init. Should I
> send in an additional patch to reverse that part of the change?

No, I fixed it now.

 K.

Re: [Patch v3] static, __init and __initdata symbols in machine_kexec.c

[Simon Horman]

On Wed, May 16, 2007 at 10:11:43AM +0100, Keir Fraser wrote:
> 
> 
> 
> On 16/5/07 10:07, "Simon Horman" <horms@verge.net.au> wrote:
> 
> > Sorry, I was a bit hasty with sending in this patch :(
> > 
> > As per the subseqent v4, I discovered that xen_hypervisor_res and
> > xen_phys_cpus really ought not to be __initdata because they get
> > inserted into resource_iomem, which is accessed after init. Should I
> > send in an additional patch to reverse that part of the change?
> 
> No, I fixed it now.

Thanks

-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/

Re: [Patch v2] static and __init symbols in machine_kexec.c

[Simon Horman]

On Wed, May 16, 2007 at 04:31:33PM +0900, Simon Horman wrote:
> On Wed, May 16, 2007 at 08:21:41AM +0100, Keir Fraser wrote:
> > 
> > 
> > 
> > On 16/5/07 07:49, "Simon Horman" <horms@verge.net.au> wrote:
> > 
> > > Curiously, making xen_max_nr_phys_cpus __init causes a section missmatch
> > > error at compile time. I guess this is to do with the way global ints
> > > are handled. I confirmed that its consistent across x86_32, x86_64 and
> > > ia64. So I guess its ok to leave it as is.
> > 
> > What if you make it __initdata, as it (and the other data variables) should
> > be?
> 
> I'll take a look into that and get back to you.

That ideed does solve the problem, silly me.
I'll send an updated patch in a moment.

-- 
Horms
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/