[PATCH 1/2] [powerpc] export symbols for use by lparcfg

[PATCH 1/2] [powerpc] export symbols for use by lparcfg

[Will Schmidt]

Updates the Kconfig to allow lparcfg to be built as a module, and
add the necessary EXPORT_SYMBOLS needed for a successful build.

Signed-off-by: Will Schmidt <will_schmidt@vnet.ibm.com>
---

 arch/powerpc/kernel/paca.c             |    1 +
 arch/powerpc/kernel/process.c          |    1 +
 arch/powerpc/kernel/vdso.c             |    1 +
 arch/powerpc/platforms/pseries/Kconfig |    2 +-
 4 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 55f1a25..0b2c875 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -45,6 +45,7 @@ struct lppaca lppaca[] = {
 		.vmxregs_in_use = 0,
 	},
 };
+EXPORT_SYMBOL_GPL(lppaca);
 
 /*
  * 3 persistent SLBs are registered here.  The buffer will be zero
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index f3d4dd5..19b209b 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -233,6 +233,7 @@ #endif
 
 #ifdef CONFIG_PPC64
 DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
+EXPORT_PER_CPU_SYMBOL_GPL(cpu_usage_array);
 static DEFINE_PER_CPU(unsigned long, current_dabr);
 #endif
 
diff --git a/arch/powerpc/kernel/vdso.c b/arch/powerpc/kernel/vdso.c
index e46c31b..ea2e015 100644
--- a/arch/powerpc/kernel/vdso.c
+++ b/arch/powerpc/kernel/vdso.c
@@ -76,6 +76,7 @@ static union {
 	u8			page[PAGE_SIZE];
 } vdso_data_store __attribute__((__section__(".data.page_aligned")));
 struct vdso_data *vdso_data = &vdso_data_store.data;
+EXPORT_SYMBOL_GPL(vdso_data);
 
 /* Format of the patch table */
 struct vdso_patch_def
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index a57032c..4e5c8f8 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -19,7 +19,7 @@ config SCANLOG
 	depends on RTAS_PROC && PPC_PSERIES
 
 config LPARCFG
-	bool "LPAR Configuration Data"
+	tristate "LPAR Configuration Data"
 	depends on PPC_PSERIES || PPC_ISERIES
 	help
 	Provide system capacity information via human readable

[PATCH 2/2] [powerpc] replace if-then-else with a switch statement

[Will Schmidt]

convert a compound if-else blob to a switch statement.
This better fits the kernel coding style.

Signed-off-by: Will Schmidt <will_schmidt@vnet.ibm.com>
---

 arch/powerpc/kernel/lparcfg.c |   43 +++++++++++++++++++++--------------------
 1 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/lparcfg.c b/arch/powerpc/kernel/lparcfg.c
index 89486b6..0a4f9f5 100644
--- a/arch/powerpc/kernel/lparcfg.c
+++ b/arch/powerpc/kernel/lparcfg.c
@@ -130,30 +130,31 @@ #ifdef CONFIG_PPC_PSERIES
 /*
  * Methods used to fetch LPAR data when running on a pSeries platform.
  */
-/* find a better place for this function... */
 static void log_plpar_hcall_return(unsigned long rc, char *tag)
 {
-	if (rc == 0)		/* success, return */
+	switch(rc) {
+	case 0:
 		return;
-/* check for null tag ? */
-	if (rc == H_HARDWARE)
-		printk(KERN_INFO
-		       "plpar-hcall (%s) failed with hardware fault\n", tag);
-	else if (rc == H_FUNCTION)
-		printk(KERN_INFO
-		       "plpar-hcall (%s) failed; function not allowed\n", tag);
-	else if (rc == H_AUTHORITY)
-		printk(KERN_INFO
-		       "plpar-hcall (%s) failed; not authorized to this"
-		       " function\n", tag);
-	else if (rc == H_PARAMETER)
-		printk(KERN_INFO "plpar-hcall (%s) failed; Bad parameter(s)\n",
-		       tag);
-	else
-		printk(KERN_INFO
-		       "plpar-hcall (%s) failed with unexpected rc(0x%lx)\n",
-		       tag, rc);
-
+	case H_HARDWARE:
+		printk(KERN_INFO "plpar-hcall (%s) "
+				"Hardware fault\n", tag);
+		return;
+	case H_FUNCTION:
+		printk(KERN_INFO "plpar-hcall (%s) "
+				"Function not allowed\n", tag);
+		return;
+	case H_AUTHORITY:
+		printk(KERN_INFO "plpar-hcall (%s) "
+				"Not authorized to this function\n", tag);
+		return;
+	case H_PARAMETER:
+		printk(KERN_INFO "plpar-hcall (%s) "
+				"Bad parameter(s)\n",tag);
+		return;
+	default:
+		printk(KERN_INFO "plpar-hcall (%s) "
+				"Unexpected rc(0x%lx)\n", tag, rc);
+	}
 }
 
 /*

Re: [PATCH 1/2] [powerpc] export symbols for use by lparcfg

[Nathan Lynch]

Will Schmidt wrote:
> 
> Updates the Kconfig to allow lparcfg to be built as a module, and
> add the necessary EXPORT_SYMBOLS needed for a successful build.

Well, almost exactly a year ago lparcfg was changed to bool.  Do the
reasons for that change still stand?

commit 82dfdcae0d57c842e02f037758687eef42fb7af6
Author: Paul Mackerras <paulus@samba.org>
Date:   Tue Mar 14 11:35:37 2006 +1100

    powerpc: Disallow lparcfg being a module
    
    The lparcfg code needs several things which are pretty arcane internal
    details and which we don't want to export, which means that lparcfg
    doesn't work when built as a module.  This makes it a bool instead of
    a tristate in the Kconfig so that users can't try to build it as a
    module.

> +EXPORT_SYMBOL_GPL(lppaca);

> +EXPORT_PER_CPU_SYMBOL_GPL(cpu_usage_array);

> +EXPORT_SYMBOL_GPL(vdso_data);

Hmm, I don't think lparcfg needs to access vdso_data at all.

In pseries_lparcfg_data we have:

        lrdrp = get_property(rtas_node, "ibm,lrdr-capacity", NULL);

        if (lrdrp == NULL) {
                partition_potential_processors = vdso_data->processorCount;
        } else {
                partition_potential_processors = *(lrdrp + 4);
        }

        partition_active_processors = lparcfg_count_active_processors();

But if there's no ibm,lrdr-capacity property then the system doesn't
support adding processors, so partition_potential_processors should be
equal to partition_active_processors.

Re: [PATCH 1/2] [powerpc] export symbols for use by lparcfg

[Christoph Hellwig]

On Mon, Mar 12, 2007 at 01:21:14PM -0600, Will Schmidt wrote:
> 
> Updates the Kconfig to allow lparcfg to be built as a module, and
> add the necessary EXPORT_SYMBOLS needed for a successful build.
> 
> Signed-off-by: Will Schmidt <will_schmidt@vnet.ibm.com>

NACK.  This is an awfull lot of deep down internal symbols, and there
is no reason to make this a module.  If you fear about the space useage
on non-phype platforms better reintroduce something like the
__pmac/__chrp markers of the old ppc32 port.

Re: [PATCH 1/2] [powerpc] export symbols for use by lparcfg

[Will Schmidt]

On Mon, 2007-12-03 at 15:24 -0500, Nathan Lynch wrote:
> Will Schmidt wrote:
> > 
> > Updates the Kconfig to allow lparcfg to be built as a module, and
> > add the necessary EXPORT_SYMBOLS needed for a successful build.
> 
> Well, almost exactly a year ago lparcfg was changed to bool.  Do the
> reasons for that change still stand?
Hi Nathan, 

Thanks for pointing this out to me.  I was not aware there was a
specific and deliberate change to bool from tristate.   I thought it was
due to a sloppy change that neglected to export required symbols.

> commit 82dfdcae0d57c842e02f037758687eef42fb7af6
> Author: Paul Mackerras <paulus@samba.org>
> Date:   Tue Mar 14 11:35:37 2006 +1100
> 
>     powerpc: Disallow lparcfg being a module
>     
>     The lparcfg code needs several things which are pretty arcane internal
>     details and which we don't want to export, which means that lparcfg
>     doesn't work when built as a module.  This makes it a bool instead of
>     a tristate in the Kconfig so that users can't try to build it as a
>     module.
> 
> > +EXPORT_SYMBOL_GPL(lppaca);
> 
> > +EXPORT_PER_CPU_SYMBOL_GPL(cpu_usage_array);
> 
> > +EXPORT_SYMBOL_GPL(vdso_data);
> 
> Hmm, I don't think lparcfg needs to access vdso_data at all.

It does via the reference here: 

	partition_potential_processors = vdso_data->processorCount


> 
> In pseries_lparcfg_data we have:
> 
>         lrdrp = get_property(rtas_node, "ibm,lrdr-capacity", NULL);
> 
>         if (lrdrp == NULL) {
>                 partition_potential_processors = vdso_data->processorCount;
>         } else {
>                 partition_potential_processors = *(lrdrp + 4);
>         }
> 
>         partition_active_processors = lparcfg_count_active_processors();
> 
> But if there's no ibm,lrdr-capacity property then the system doesn't
> support adding processors, so partition_potential_processors should be
> equal to partition_active_processors.

Re: [PATCH 1/2] [powerpc] export symbols for use by lparcfg

[Will Schmidt]

On Mon, 2007-12-03 at 22:32 +0100, Christoph Hellwig wrote:
> On Mon, Mar 12, 2007 at 01:21:14PM -0600, Will Schmidt wrote:
> > 
> > Updates the Kconfig to allow lparcfg to be built as a module, and
> > add the necessary EXPORT_SYMBOLS needed for a successful build.
> > 
> > Signed-off-by: Will Schmidt <will_schmidt@vnet.ibm.com>
> 
> NACK.  This is an awfull lot of deep down internal symbols, and there
> is no reason to make this a module.  If you fear about the space useage
> on non-phype platforms better reintroduce something like the
> __pmac/__chrp markers of the old ppc32 port.

Understood..    This used to be a module, I just wanted it to be a
module again.   I'm not worried about the space usage, honest. :-)

With this and Nathans response I'll happily second your NACK. 


-Will

Re: [PATCH 1/2] [powerpc] export symbols for use by lparcfg

[Nathan Lynch]

Will Schmidt wrote:
> On Mon, 2007-12-03 at 15:24 -0500, Nathan Lynch wrote:
> > Will Schmidt wrote:
> > > 
> > > +EXPORT_SYMBOL_GPL(vdso_data);
> > 
> > Hmm, I don't think lparcfg needs to access vdso_data at all.
> 
> It does via the reference here: 
> 
> 	partition_potential_processors = vdso_data->processorCount

I said lparcfg doesn't _need_ to access it :)

I was trying to show how the code could be changed to not use it:

> > 
> > In pseries_lparcfg_data we have:
> > 
> >         lrdrp = get_property(rtas_node, "ibm,lrdr-capacity", NULL);
> > 
> >         if (lrdrp == NULL) {
> >                 partition_potential_processors = vdso_data->processorCount;
> >         } else {
> >                 partition_potential_processors = *(lrdrp + 4);
> >         }
> > 
> >         partition_active_processors = lparcfg_count_active_processors();
> > 
> > But if there's no ibm,lrdr-capacity property then the system doesn't
> > support adding processors, so partition_potential_processors should be
> > equal to partition_active_processors.

Basically, calculate partition_active_processors first, then if !lrdrp,
partition_potential_processors = partition_active_processors.