All of lore.kernel.org
 help / color / mirror / Atom feed
* static and __init symbols in machine_kexec.c
@ 2007-05-16  6:49 Simon Horman
  2007-05-16  6:49 ` [Patch v2] " Simon Horman
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Horman @ 2007-05-16  6:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

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;

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Patch v2] static and __init symbols in machine_kexec.c
  2007-05-16  6:49 static and __init symbols in machine_kexec.c Simon Horman
@ 2007-05-16  6:49 ` Simon Horman
  2007-05-16  7:21   ` Keir Fraser
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Horman @ 2007-05-16  6:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

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;

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Patch v3] static, __init and __initdata symbols in machine_kexec.c
  2007-05-16  7:31     ` Simon Horman
@ 2007-05-16  6:49       ` Simon Horman, Keir Fraser
  2007-05-16  6:49         ` [Patch v4] " Simon Horman, Keir Fraser
  2007-05-16  8:16         ` [Patch v3] " Ian Campbell
  2007-05-16  7:58       ` [Patch v2] static and __init " Simon Horman
  1 sibling, 2 replies; 11+ messages in thread
From: Simon Horman, Keir Fraser @ 2007-05-16  6:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

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;

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Patch v4] static, __init and __initdata symbols in machine_kexec.c
  2007-05-16  6:49       ` [Patch v3] static, __init and __initdata " Simon Horman, Keir Fraser
@ 2007-05-16  6:49         ` Simon Horman, Keir Fraser
  2007-05-16  8:16         ` [Patch v3] " Ian Campbell
  1 sibling, 0 replies; 11+ messages in thread
From: Simon Horman, Keir Fraser @ 2007-05-16  6:49 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Campbell

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;

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch v2] static and __init symbols in machine_kexec.c
  2007-05-16  6:49 ` [Patch v2] " Simon Horman
@ 2007-05-16  7:21   ` Keir Fraser
  2007-05-16  7:31     ` Simon Horman
  0 siblings, 1 reply; 11+ messages in thread
From: Keir Fraser @ 2007-05-16  7:21 UTC (permalink / raw)
  To: Simon Horman, xen-devel; +Cc: Ian Campbell




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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch v2] static and __init symbols in machine_kexec.c
  2007-05-16  7:21   ` Keir Fraser
@ 2007-05-16  7:31     ` Simon Horman
  2007-05-16  6:49       ` [Patch v3] static, __init and __initdata " Simon Horman, Keir Fraser
  2007-05-16  7:58       ` [Patch v2] static and __init " Simon Horman
  0 siblings, 2 replies; 11+ messages in thread
From: Simon Horman @ 2007-05-16  7:31 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel, Ian Campbell

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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch v2] static and __init symbols in machine_kexec.c
  2007-05-16  7:31     ` Simon Horman
  2007-05-16  6:49       ` [Patch v3] static, __init and __initdata " Simon Horman, Keir Fraser
@ 2007-05-16  7:58       ` Simon Horman
  1 sibling, 0 replies; 11+ messages in thread
From: Simon Horman @ 2007-05-16  7:58 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel, Ian Campbell

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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch v3] static, __init and __initdata symbols in machine_kexec.c
  2007-05-16  6:49       ` [Patch v3] static, __init and __initdata " Simon Horman, Keir Fraser
  2007-05-16  6:49         ` [Patch v4] " Simon Horman, Keir Fraser
@ 2007-05-16  8:16         ` Ian Campbell
  2007-05-16  9:07           ` Simon Horman
  1 sibling, 1 reply; 11+ messages in thread
From: Ian Campbell @ 2007-05-16  8:16 UTC (permalink / raw)
  To: Simon Horman, Keir Fraser; +Cc: xen-devel

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch v3] static, __init and __initdata symbols in machine_kexec.c
  2007-05-16  8:16         ` [Patch v3] " Ian Campbell
@ 2007-05-16  9:07           ` Simon Horman
  2007-05-16  9:11             ` Keir Fraser
  0 siblings, 1 reply; 11+ messages in thread
From: Simon Horman @ 2007-05-16  9:07 UTC (permalink / raw)
  To: Ian Campbell; +Cc: xen-devel

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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch v3] static, __init and __initdata symbols in machine_kexec.c
  2007-05-16  9:07           ` Simon Horman
@ 2007-05-16  9:11             ` Keir Fraser
  2007-05-16  9:32               ` Simon Horman
  0 siblings, 1 reply; 11+ messages in thread
From: Keir Fraser @ 2007-05-16  9:11 UTC (permalink / raw)
  To: Simon Horman, Ian Campbell; +Cc: xen-devel




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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Patch v3] static, __init and __initdata symbols in machine_kexec.c
  2007-05-16  9:11             ` Keir Fraser
@ 2007-05-16  9:32               ` Simon Horman
  0 siblings, 0 replies; 11+ messages in thread
From: Simon Horman @ 2007-05-16  9:32 UTC (permalink / raw)
  To: Keir Fraser; +Cc: xen-devel, Ian Campbell

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/

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2007-05-16  9:32 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-16  6:49 static and __init symbols in machine_kexec.c Simon Horman
2007-05-16  6:49 ` [Patch v2] " Simon Horman
2007-05-16  7:21   ` Keir Fraser
2007-05-16  7:31     ` Simon Horman
2007-05-16  6:49       ` [Patch v3] static, __init and __initdata " Simon Horman, Keir Fraser
2007-05-16  6:49         ` [Patch v4] " Simon Horman, Keir Fraser
2007-05-16  8:16         ` [Patch v3] " Ian Campbell
2007-05-16  9:07           ` Simon Horman
2007-05-16  9:11             ` Keir Fraser
2007-05-16  9:32               ` Simon Horman
2007-05-16  7:58       ` [Patch v2] static and __init " Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.