patch for powerpc lparcfg.c

patch for powerpc lparcfg.c

[Carl Love]

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


I created the attached patch to print the partition name along with the
other partition information reported in /proc/ppc64/lparcfg for the
powerpc systems.  The patch adds a single line of code to the
arch/powerpc/kernel/lparcfg.c file.  I tested the code on a power5 box.
I would suggest adding the same line to the I series code at line 101 so
the partition name is printed on I series hardware as well.  Since I
don't have access to I series hardware to test on I did not include this
change in my patch file.

Carl Love

 

[-- Attachment #2: linux-2.6.17-rc1-git8-lparcfg.patch --]
[-- Type: text/x-patch, Size: 593 bytes --]

diff -rauN -X /home/carll/dontdiff linux-2.6.17-rc1-git8/arch/powerpc/kernel/lparcfg.c linux-2.6.17-rc1-git8-new/arch/powerpc/kernel/lparcfg.c
--- linux-2.6.17-rc1-git8/arch/powerpc/kernel/lparcfg.c	2006-04-13 12:25:11.000000000 -0700
+++ linux-2.6.17-rc1-git8-new/arch/powerpc/kernel/lparcfg.c	2006-04-13 15:29:02.000000000 -0700
@@ -360,6 +360,9 @@
 
 	seq_printf(m, "system_type=%s\n", model);
 
+	seq_printf(m, "partition_name=%s\n",
+		   get_property(rootdn, "ibm,partition-name", NULL));
+
 	seq_printf(m, "partition_id=%d\n", (int)lp_index);
 
 	rtas_node = find_path_device("/rtas");

Re: patch for powerpc lparcfg.c

[Nathan Lynch]

Carl Love wrote:
> 
> I created the attached patch to print the partition name along with the
> other partition information reported in /proc/ppc64/lparcfg for the
> powerpc systems.

> diff -rauN -X /home/carll/dontdiff linux-2.6.17-rc1-git8/arch/powerpc/kernel/lparcfg.c linux-2.6.17-rc1-git8-new/arch/powerpc/kernel/lparcfg.c
> --- linux-2.6.17-rc1-git8/arch/powerpc/kernel/lparcfg.c	2006-04-13 12:25:11.000000000 -0700
> +++ linux-2.6.17-rc1-git8-new/arch/powerpc/kernel/lparcfg.c	2006-04-13 15:29:02.000000000 -0700
> @@ -360,6 +360,9 @@
>  
>  	seq_printf(m, "system_type=%s\n", model);
>  
> +	seq_printf(m, "partition_name=%s\n",
> +		   get_property(rootdn, "ibm,partition-name", NULL));
> +

Who or what is the intended consumer of this information?

You should probably check get_property's return value and print the
partition_name line only if it is non-NULL.

Re: patch for powerpc lparcfg.c

[Carl Love]

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

Nathan Lynch:

Oops, lost the cc line the last time.

I wasn't aware that the partition name was already being printed in
the /proc/device-tree.  Yes, my tool could use that entry.  It just
means opening multiple locations to get all the information.  I will go
ahead and use it since it is there.   

The patch becomes a bit more academic.  The question becomes, should the
partition name also be printed in /proc/ppc64/lparcfg along with the
rest of the partition information?  Seems like a good idea to me since
it makes the information in lparcfg more complete.  I will leave it up
to the maintainers to decide. 

I have reattached the updated patch in case anyone cares. 

Thanks for letting me know where I can get the partition name.

                  Carl Love



On Fri, 2006-04-14 at 12:50 -0500, Nathan Lynch wrote:
> You forgot to cc the list...
> 
> Carl Love wrote:
> > 
> > The intended consumer of this information is a tool that I am working
> > on.  The tool prints the CPU utilization as a function of the
> > partition's processor entitlement, hypervisor call statistics (number of
> > calls, average call time, max call time, min call time) and partition
> > configuration information.  Specifically, the -i option is to print the
> > partition information.  The required information includes the partition
> > name.  Most of the other required information comes from
> > the  /proc/ppc64/lparcfg file.  This seems like a logical place to
> > include the partition name.
> 
> The partition name, if it exists, is already available at
> /proc/device-tree/ibm,partition-name.  Any reason your tool couldn't
> use that?

[-- Attachment #2: linux-2.6.17-rc1-git8-lparcfg.patch --]
[-- Type: text/x-patch, Size: 1155 bytes --]

diff -rauN -X /home/carll/dontdiff linux-2.6.17-rc1-git8/arch/powerpc/kernel/lparcfg.c linux-2.6.17-rc1-git8-new/arch/powerpc/kernel/lparcfg.c
--- linux-2.6.17-rc1-git8/arch/powerpc/kernel/lparcfg.c	2006-04-13 12:25:11.000000000 -0700
+++ linux-2.6.17-rc1-git8-new/arch/powerpc/kernel/lparcfg.c	2006-04-14 09:24:12.000000000 -0700
@@ -340,6 +340,7 @@
 	struct device_node *rootdn;
 	const char *model = "";
 	const char *system_id = "";
+	const char *partition_name = NULL;
 	unsigned int *lp_index_ptr, lp_index = 0;
 	struct device_node *rtas_node;
 	int *lrdrp = NULL;
@@ -347,6 +348,8 @@
 	rootdn = find_path_device("/");
 	if (rootdn) {
 		model = get_property(rootdn, "model", NULL);
+		partition_name = get_property(rootdn, 
+					      "ibm,partition-name", NULL);
 		system_id = get_property(rootdn, "system-id", NULL);
 		lp_index_ptr = (unsigned int *)
 		    get_property(rootdn, "ibm,partition-no", NULL);
@@ -360,6 +363,9 @@
 
 	seq_printf(m, "system_type=%s\n", model);
 
+	if (!NULL)
+		seq_printf(m, "partition_name=%s\n", partition_name);
+
 	seq_printf(m, "partition_id=%d\n", (int)lp_index);
 
 	rtas_node = find_path_device("/rtas");

Re: patch for powerpc lparcfg.c

[Olof Johansson]

On Fri, Apr 14, 2006 at 05:16:02PM -0700, Carl Love wrote:
> Nathan Lynch:
> 
> Oops, lost the cc line the last time.
> 
> I wasn't aware that the partition name was already being printed in
> the /proc/device-tree.  Yes, my tool could use that entry.  It just
> means opening multiple locations to get all the information.  I will go
> ahead and use it since it is there.   
> 
> The patch becomes a bit more academic.  The question becomes, should the
> partition name also be printed in /proc/ppc64/lparcfg along with the
> rest of the partition information?  Seems like a good idea to me since
> it makes the information in lparcfg more complete.  I will leave it up
> to the maintainers to decide. 

IMHO, duplicating the information is just extra overhead. It's not a
piece of data I would expect applications to have performance-critical
access requirements for, so the extra file open isn't that much of a
bother.

In general, adding things to an interface like lparcfg just means we
will need to maintain it there forever. The less we can get away with
in such ways, the better.

I think lparcfg is a leftover from the iseries days, where there was no
more convenient way to pass the information to userspace (since there is
no native device tree in that environment). We have since then added one
in linux, but the hypervisor doesn't provide it to us.


-Olfo