linux-next: tip tree build warning

linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1018 bytes --]

Hi all,

Today's linux-next build (x86_64 allmodconfig) produced this warning:

In file included from arch/x86/include/asm/smp.h:13,
                 from arch/x86/include/asm/mmzone_64.h:12,
                 from arch/x86/include/asm/mmzone.h:4,
                 from include/linux/mmzone.h:783,
                 from include/linux/gfp.h:4,
                 from include/linux/kmod.h:22,
                 from include/linux/module.h:13,
                 from arch/x86/kernel/apic/apic_noop.c:14:
arch/x86/include/asm/apic.h: In function 'default_apicid_to_cpu_present':
arch/x86/include/asm/apic.h:591: warning: the frame size of 8192 bytes is larger than 2048 bytes

It may not have been caused by the tip tree, but I can't find what
changed to cause this and a commit from the tip tree has exposed it
(9844ab11c763bfed9f054c82366b19dcda66aca9 "x86, apic: Introduce the NOOP
apic driver").
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) produced this warning:
> 
> In file included from arch/x86/include/asm/smp.h:13,
>                  from arch/x86/include/asm/mmzone_64.h:12,
>                  from arch/x86/include/asm/mmzone.h:4,
>                  from include/linux/mmzone.h:783,
>                  from include/linux/gfp.h:4,
>                  from include/linux/kmod.h:22,
>                  from include/linux/module.h:13,
>                  from arch/x86/kernel/apic/apic_noop.c:14:
> arch/x86/include/asm/apic.h: In function 'default_apicid_to_cpu_present':
> arch/x86/include/asm/apic.h:591: warning: the frame size of 8192 bytes is larger than 2048 bytes
> 
> It may not have been caused by the tip tree, but I can't find what 
> changed to cause this and a commit from the tip tree has exposed it 
> (9844ab11c763bfed9f054c82366b19dcda66aca9 "x86, apic: Introduce the 
> NOOP apic driver").

That commit is very simple. Are you sure it's not GCC bogosity?

	Ingo

Re: linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 760 bytes --]

Hi Ingo,

On Wed, 28 Oct 2009 08:31:45 +0100 Ingo Molnar <mingo@elte.hu> wrote:
>
> 
> That commit is very simple. Are you sure it's not GCC bogosity?

That commit just produces the warning (since it introduced the
apic_noop.c file) and probably has nothing directly to do with the
problem (as I said).

The relevant part of apic.h is:

static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
{
        return physid_mask_of_physid(phys_apicid);
}

Where physid_mask_of_physid() manipulates a physid_mask_t which contains
an array of unsigned longs that is BITS_TO_LONGS(MAX_APICS) long and
MAX_APICS is 32768.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

Hi all,

On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
> {
>         return physid_mask_of_physid(phys_apicid);
> }

I just noticed that this function (default_apicid_to_cpu_present) is
declared "static inline in a header" but looks like it is only used by
assigning its address to a function pointer.  Its only use for x86_64 is
in arch/x86/kernel/apic/apic_noop.c ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Ingo Molnar]

* Stephen Rothwell <sfr@canb.auug.org.au> wrote:

> Hi all,
> 
> On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
> > {
> >         return physid_mask_of_physid(phys_apicid);
> > }
> 
> I just noticed that this function (default_apicid_to_cpu_present) is 
> declared "static inline in a header" but looks like it is only used by 
> assigning its address to a function pointer.  Its only use for x86_64 
> is in arch/x86/kernel/apic/apic_noop.c ...

yes, that might be a real problem - returning the mask like that is 
messy. Thanks, will check.

	Ingo

Re: linux-next: tip tree build warning

[Cyrill Gorcunov]

[Ingo Molnar - Wed, Oct 28, 2009 at 08:50:12AM +0100]
| 
| * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
| 
| > Hi all,
| > 
| > On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
| > >
| > > static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
| > > {
| > >         return physid_mask_of_physid(phys_apicid);
| > > }
| > 
| > I just noticed that this function (default_apicid_to_cpu_present) is 
| > declared "static inline in a header" but looks like it is only used by 
| > assigning its address to a function pointer.  Its only use for x86_64 
| > is in arch/x86/kernel/apic/apic_noop.c ...
| 
| yes, that might be a real problem - returning the mask like that is 
| messy. Thanks, will check.
| 
| 	Ingo
| 

Darn, my fault sorry! Here is an update which fixes the issue.
(Btw, Stephen could you CC me next time if you get commit id
 with me in authors, so I wouldn't miss problem).

Please review, comments/complains are quite welcome!

	-- Cyrill
---
x86,apic: Get rid of apicid_to_cpu_present assign on X86-64

In fact it's never get used on x86-64 (for 64 bit platform
we use differ technique to enumerate io-units).

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/kernel/apic/apic_noop.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
@@ -162,7 +162,12 @@ struct apic apic_noop = {
 
 	.cpu_to_logical_apicid		= noop_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
+
+#ifdef CONFIG_X86_32
 	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+#else
+	.apicid_to_cpu_present		= NULL,
+#endif
 
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,

Re: linux-next: tip tree build warning

[Ingo Molnar]

* Cyrill Gorcunov <gorcunov@openvz.org> wrote:

> [Ingo Molnar - Wed, Oct 28, 2009 at 08:50:12AM +0100]
> | 
> | * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> | 
> | > Hi all,
> | > 
> | > On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> | > >
> | > > static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
> | > > {
> | > >         return physid_mask_of_physid(phys_apicid);
> | > > }
> | > 
> | > I just noticed that this function (default_apicid_to_cpu_present) is 
> | > declared "static inline in a header" but looks like it is only used by 
> | > assigning its address to a function pointer.  Its only use for x86_64 
> | > is in arch/x86/kernel/apic/apic_noop.c ...
> | 
> | yes, that might be a real problem - returning the mask like that is 
> | messy. Thanks, will check.
> | 
> | 	Ingo
> | 
> 
> Darn, my fault sorry! Here is an update which fixes the issue.
> (Btw, Stephen could you CC me next time if you get commit id
>  with me in authors, so I wouldn't miss problem).
> 
> Please review, comments/complains are quite welcome!
> 
> 	-- Cyrill
> ---
> x86,apic: Get rid of apicid_to_cpu_present assign on X86-64
> 
> In fact it's never get used on x86-64 (for 64 bit platform
> we use differ technique to enumerate io-units).
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> ---
>  arch/x86/kernel/apic/apic_noop.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
> =====================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c
> +++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
> @@ -162,7 +162,12 @@ struct apic apic_noop = {
>  
>  	.cpu_to_logical_apicid		= noop_cpu_to_logical_apicid,
>  	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
> +
> +#ifdef CONFIG_X86_32
>  	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
> +#else
> +	.apicid_to_cpu_present		= NULL,
> +#endif

would be better to unify this instead ...

	Ingo

Re: linux-next: tip tree build warning

[Cyrill Gorcunov]

[Ingo Molnar - Sun, Nov 08, 2009 at 02:32:47PM +0100]
...
| 
| > +
| > +#ifdef CONFIG_X86_32
| >  	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
| > +#else
| > +	.apicid_to_cpu_present		= NULL,
| > +#endif
| 
| would be better to unify this instead ...
| 
| 	Ingo
| 

OK, wait a bit.

	-- Cyrill

Re: linux-next: tip tree build warning

[Cyrill Gorcunov]

[Ingo Molnar - Sun, Nov 08, 2009 at 02:32:47PM +0100]
| 
...
| > +
| > +#ifdef CONFIG_X86_32
| >  	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
| > +#else
| > +	.apicid_to_cpu_present		= NULL,
| > +#endif
| 
| would be better to unify this instead ...
| 
| 	Ingo
| 

[not for inclusion]

Here is what I'm going to implement (it is not finished yet,
but just to show the idea -- ie to get rid of physid_mask_t
passed as an argument at all but use pointers instead since
callers already have bitmask allocated).

And physid_set_mask_of_physid already do the work for us.
Hmm?

(Ingo, I think you may apply the former patch just to
 have the issue fixed this way temporary)

	-- Cyrill
---
 arch/x86/include/asm/apic.h      |    7 +------
 arch/x86/kernel/apic/apic_noop.c |    2 +-
 arch/x86/kernel/apic/bigsmp_32.c |    7 +------
 arch/x86/kernel/apic/probe_32.c  |    2 +-
 4 files changed, 4 insertions(+), 14 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/apic.h
=====================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/apic.h
+++ linux-2.6.git/arch/x86/include/asm/apic.h
@@ -310,7 +310,7 @@ struct apic {
 	int (*apicid_to_node)(int logical_apicid);
 	int (*cpu_to_logical_apicid)(int cpu);
 	int (*cpu_present_to_apicid)(int mps_cpu);
-	physid_mask_t (*apicid_to_cpu_present)(int phys_apicid);
+	void (*apicid_to_cpu_present)(int phys_apicid, physid_mask_t *bitmap);
 	void (*setup_portio_remap)(void);
 	int (*check_phys_apicid_present)(int phys_apicid);
 	void (*enable_apic_mode)(void);
@@ -585,11 +585,6 @@ extern int default_cpu_present_to_apicid
 extern int default_check_phys_apicid_present(int phys_apicid);
 #endif
 
-static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
-{
-	return physid_mask_of_physid(phys_apicid);
-}
-
 #endif /* CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_32
Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
@@ -162,7 +162,7 @@ struct apic apic_noop = {
 
 	.cpu_to_logical_apicid		= noop_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,
Index: linux-2.6.git/arch/x86/kernel/apic/bigsmp_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/bigsmp_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/bigsmp_32.c
@@ -93,11 +93,6 @@ static int bigsmp_cpu_present_to_apicid(
 	return BAD_APICID;
 }
 
-static physid_mask_t bigsmp_apicid_to_cpu_present(int phys_apicid)
-{
-	return physid_mask_of_physid(phys_apicid);
-}
-
 /* Mapping from cpu number to logical apicid */
 static inline int bigsmp_cpu_to_logical_apicid(int cpu)
 {
@@ -230,7 +225,7 @@ struct apic apic_bigsmp = {
 	.apicid_to_node			= bigsmp_apicid_to_node,
 	.cpu_to_logical_apicid		= bigsmp_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= bigsmp_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= bigsmp_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= bigsmp_check_phys_apicid_present,
 	.enable_apic_mode		= NULL,
Index: linux-2.6.git/arch/x86/kernel/apic/probe_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/probe_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/probe_32.c
@@ -108,7 +108,7 @@ struct apic apic_default = {
 	.apicid_to_node			= default_apicid_to_node,
 	.cpu_to_logical_apicid		= default_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,
 	.enable_apic_mode		= NULL,

Re: linux-next: tip tree build warning

[Cyrill Gorcunov]

[Ingo Molnar - Wed, Oct 28, 2009 at 08:50:12AM +0100]
| 
| * Stephen Rothwell <sfr@canb.auug.org.au> wrote:
| 
| > Hi all,
| > 
| > On Wed, 28 Oct 2009 18:41:26 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
| > >
| > > static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
| > > {
| > >         return physid_mask_of_physid(phys_apicid);
| > > }
| > 
| > I just noticed that this function (default_apicid_to_cpu_present) is 
| > declared "static inline in a header" but looks like it is only used by 
| > assigning its address to a function pointer.  Its only use for x86_64 
| > is in arch/x86/kernel/apic/apic_noop.c ...
| 
| yes, that might be a real problem - returning the mask like that is 
| messy. Thanks, will check.
| 
| 	Ingo
|

ok, here is what I've just cooked. Please review, I've CC'ed a few
"knowing-apic-code-quite-well" persons just to be sure.

Note that we still have a few items in "struct apic" which
operate with physid_mask_t on stack.

I think perhaps it's a good idea to touch this data via pointers
povided by caller:

	physid_mask_t (*ioapic_phys_id_map)(physid_mask_t map);

I didn't manage to cover it today -- will do tomorrow if patch
approach would be approved.

Also, it's just warning (yet) since we don't use those routines
in x86-64 so it doesn't harm.

Anyway, please review, comment, complain and etc... would appreciate.

	-- Cyrill

p.s.: i don't have inet access in office so will be able to reply
at tomorrow evening only.
---
x86,apic: Do not use stack'ed physid_mask_t in apicid_to_cpu_present

Stephen Rothwell pointed out that apic-noop (when gets compiled
in x86-84 environment) potentially may consume too much stack space.

|
| Hi all,
|
| Today's linux-next build (x86_64 allmodconfig) produced this warning:
|
| In file included from arch/x86/include/asm/smp.h:13,
|                 from arch/x86/include/asm/mmzone_64.h:12,
|                 from arch/x86/include/asm/mmzone.h:4,
|                 from include/linux/mmzone.h:783,
|                 from include/linux/gfp.h:4,
|                 from include/linux/kmod.h:22,
|                 from include/linux/module.h:13,
|                 from arch/x86/kernel/apic/apic_noop.c:14:
| arch/x86/include/asm/apic.h: In function 'default_apicid_to_cpu_present':
| arch/x86/include/asm/apic.h:591: warning: the frame size of 8192 bytes is larger than 2048 bytes
|
| It may not have been caused by the tip tree, but I can't find what
| changed to cause this and a commit from the tip tree has exposed it
| (9844ab11c763bfed9f054c82366b19dcda66aca9 "x86, apic: Introduce the NOOP
| apic driver").
|

So I would say this is a bug in apic-noop (in fact we don't use
default_apicid_to_cpu_present if operate in 64bit mode but it's a sign
that something is wrong with code design). The key problem is that
physid_mask_t is an array with a size depending on MAX_APICS, which
in turn is big enough on x86-64 to trigger compiler warning.

So to prevent such a situation in future we should use physid_mask_t
pointer leaving apic driver with a task to operate over data but not
allocate it. Caller should instead.

This allow us throw out some code as well since physid_set_mask_of_physid
already implement the functionality we need.

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 arch/x86/include/asm/apic.h      |    7 +------
 arch/x86/kernel/apic/apic_noop.c |    2 +-
 arch/x86/kernel/apic/bigsmp_32.c |    7 +------
 arch/x86/kernel/apic/es7000_32.c |    8 ++------
 arch/x86/kernel/apic/io_apic.c   |    4 ++--
 arch/x86/kernel/apic/numaq_32.c  |    4 ++--
 arch/x86/kernel/apic/probe_32.c  |    2 +-
 arch/x86/kernel/apic/summit_32.c |    4 ++--
 arch/x86/kernel/visws_quirks.c   |    2 +-
 9 files changed, 13 insertions(+), 27 deletions(-)

Index: linux-2.6.git/arch/x86/include/asm/apic.h
=====================================================================
--- linux-2.6.git.orig/arch/x86/include/asm/apic.h
+++ linux-2.6.git/arch/x86/include/asm/apic.h
@@ -310,7 +310,7 @@ struct apic {
 	int (*apicid_to_node)(int logical_apicid);
 	int (*cpu_to_logical_apicid)(int cpu);
 	int (*cpu_present_to_apicid)(int mps_cpu);
-	physid_mask_t (*apicid_to_cpu_present)(int phys_apicid);
+	void (*apicid_to_cpu_present)(int phys_apicid, physid_mask_t *map);
 	void (*setup_portio_remap)(void);
 	int (*check_phys_apicid_present)(int phys_apicid);
 	void (*enable_apic_mode)(void);
@@ -585,11 +585,6 @@ extern int default_cpu_present_to_apicid
 extern int default_check_phys_apicid_present(int phys_apicid);
 #endif
 
-static inline physid_mask_t default_apicid_to_cpu_present(int phys_apicid)
-{
-	return physid_mask_of_physid(phys_apicid);
-}
-
 #endif /* CONFIG_X86_LOCAL_APIC */
 
 #ifdef CONFIG_X86_32
Index: linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic_noop.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic_noop.c
@@ -162,7 +162,7 @@ struct apic apic_noop = {
 
 	.cpu_to_logical_apicid		= noop_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,
Index: linux-2.6.git/arch/x86/kernel/apic/bigsmp_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/bigsmp_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/bigsmp_32.c
@@ -93,11 +93,6 @@ static int bigsmp_cpu_present_to_apicid(
 	return BAD_APICID;
 }
 
-static physid_mask_t bigsmp_apicid_to_cpu_present(int phys_apicid)
-{
-	return physid_mask_of_physid(phys_apicid);
-}
-
 /* Mapping from cpu number to logical apicid */
 static inline int bigsmp_cpu_to_logical_apicid(int cpu)
 {
@@ -230,7 +225,7 @@ struct apic apic_bigsmp = {
 	.apicid_to_node			= bigsmp_apicid_to_node,
 	.cpu_to_logical_apicid		= bigsmp_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= bigsmp_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= bigsmp_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= bigsmp_check_phys_apicid_present,
 	.enable_apic_mode		= NULL,
Index: linux-2.6.git/arch/x86/kernel/apic/es7000_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/es7000_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/es7000_32.c
@@ -539,14 +539,10 @@ static int es7000_cpu_present_to_apicid(
 
 static int cpu_id;
 
-static physid_mask_t es7000_apicid_to_cpu_present(int phys_apicid)
+static void es7000_apicid_to_cpu_present(int phys_apicid, physid_mask_t *map)
 {
-	physid_mask_t mask;
-
-	mask = physid_mask_of_physid(cpu_id);
+	physid_set_mask_of_physid(cpu_id, map);
 	++cpu_id;
-
-	return mask;
 }
 
 /* Mapping from cpu number to logical apicid */
Index: linux-2.6.git/arch/x86/kernel/apic/io_apic.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6.git/arch/x86/kernel/apic/io_apic.c
@@ -2073,7 +2073,7 @@ void __init setup_ioapic_ids_from_mpc(vo
 			mp_ioapics[apic_id].apicid = i;
 		} else {
 			physid_mask_t tmp;
-			tmp = apic->apicid_to_cpu_present(mp_ioapics[apic_id].apicid);
+			apic->apicid_to_cpu_present(mp_ioapics[apic_id].apicid, &tmp);
 			apic_printk(APIC_VERBOSE, "Setting %d in the "
 					"phys_id_present_map\n",
 					mp_ioapics[apic_id].apicid);
@@ -3969,7 +3969,7 @@ int __init io_apic_get_unique_id(int ioa
 		apic_id = i;
 	}
 
-	tmp = apic->apicid_to_cpu_present(apic_id);
+	apic->apicid_to_cpu_present(apic_id, &tmp);
 	physids_or(apic_id_map, apic_id_map, tmp);
 
 	if (reg_00.bits.ID != apic_id) {
Index: linux-2.6.git/arch/x86/kernel/apic/numaq_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/numaq_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/numaq_32.c
@@ -402,12 +402,12 @@ static inline int numaq_apicid_to_node(i
 	return logical_apicid >> 4;
 }
 
-static inline physid_mask_t numaq_apicid_to_cpu_present(int logical_apicid)
+static void numaq_apicid_to_cpu_present(int logical_apicid, physid_mask_t *map)
 {
 	int node = numaq_apicid_to_node(logical_apicid);
 	int cpu = __ffs(logical_apicid & 0xf);
 
-	return physid_mask_of_physid(cpu + 4*node);
+	physid_set_mask_of_physid(cpu + 4*node, map);
 }
 
 /* Where the IO area was mapped on multiquad, always 0 otherwise */
Index: linux-2.6.git/arch/x86/kernel/apic/probe_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/probe_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/probe_32.c
@@ -108,7 +108,7 @@ struct apic apic_default = {
 	.apicid_to_node			= default_apicid_to_node,
 	.cpu_to_logical_apicid		= default_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
-	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+	.apicid_to_cpu_present		= physid_set_mask_of_physid,
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,
 	.enable_apic_mode		= NULL,
Index: linux-2.6.git/arch/x86/kernel/apic/summit_32.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/summit_32.c
+++ linux-2.6.git/arch/x86/kernel/apic/summit_32.c
@@ -267,9 +267,9 @@ static physid_mask_t summit_ioapic_phys_
 	return physids_promote(0x0F);
 }
 
-static physid_mask_t summit_apicid_to_cpu_present(int apicid)
+static void summit_apicid_to_cpu_present(int apicid, physid_mask_t *map)
 {
-	return physid_mask_of_physid(0);
+	physid_set_mask_of_physid(0, map);
 }
 
 static int summit_check_phys_apicid_present(int physical_apicid)
Index: linux-2.6.git/arch/x86/kernel/visws_quirks.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/visws_quirks.c
+++ linux-2.6.git/arch/x86/kernel/visws_quirks.c
@@ -183,7 +183,7 @@ static void __init MP_processor_info(str
 		return;
 	}
 
-	apic_cpus = apic->apicid_to_cpu_present(m->apicid);
+	apic->apicid_to_cpu_present(m->apicid, &apic_cpus);
 	physids_or(phys_cpu_present_map, phys_cpu_present_map, apic_cpus);
 	/*
 	 * Validate version

Re: linux-next: tip tree build warning

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 471 bytes --]

Hi Cyril,

On Sun, 8 Nov 2009 16:16:45 +0300 Cyrill Gorcunov <gorcunov@openvz.org> wrote:
>
> (Btw, Stephen could you CC me next time if you get commit id
>  with me in authors, so I wouldn't miss problem).

Yeah, sorry about that - at the time I didn't know if this was caused
directly by your commit or was just a side effect of some some other tree.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]

Re: linux-next: tip tree build warning

[Ingo Molnar]

* Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> +++ linux-2.6.git/arch/x86/include/asm/apic.h
> @@ -310,7 +310,7 @@ struct apic {
>  	int (*apicid_to_node)(int logical_apicid);
>  	int (*cpu_to_logical_apicid)(int cpu);
>  	int (*cpu_present_to_apicid)(int mps_cpu);
> -	physid_mask_t (*apicid_to_cpu_present)(int phys_apicid);
> +	void (*apicid_to_cpu_present)(int phys_apicid, physid_mask_t *map);

Yep, passing masks by reference is unconditonal goodness - that's the 
cleanup we want.

Mind sending a delta patch against tip:master?

	Ingo

[tip:x86/apic] x86, apic: Get rid of apicid_to_cpu_present assign on 64-bit

[tip-bot for Cyrill Gorcunov]

Commit-ID:  f4a70c55376683213229af7266dc57ad81aee354
Gitweb:     http://git.kernel.org/tip/f4a70c55376683213229af7266dc57ad81aee354
Author:     Cyrill Gorcunov <gorcunov@openvz.org>
AuthorDate: Sun, 8 Nov 2009 16:16:45 +0300
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 8 Nov 2009 19:46:17 +0100

x86, apic: Get rid of apicid_to_cpu_present assign on 64-bit

In fact it's never get used on x86-64 (for 64 bit platform
we use differ technique to enumerate io-units).

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Peter Zijlstra <peterz@infradead.org>
LKML-Reference: <20091108131645.GD5300@lenovo>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/apic/apic_noop.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c
index 9ab6ffb..89629f6 100644
--- a/arch/x86/kernel/apic/apic_noop.c
+++ b/arch/x86/kernel/apic/apic_noop.c
@@ -162,7 +162,12 @@ struct apic apic_noop = {
 
 	.cpu_to_logical_apicid		= noop_cpu_to_logical_apicid,
 	.cpu_present_to_apicid		= default_cpu_present_to_apicid,
+
+#ifdef CONFIG_X86_32
 	.apicid_to_cpu_present		= default_apicid_to_cpu_present,
+#else
+	.apicid_to_cpu_present		= NULL,
+#endif
 
 	.setup_portio_remap		= NULL,
 	.check_phys_apicid_present	= default_check_phys_apicid_present,