All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Revert "lscpu: keep lscpu usable on snapshots"
@ 2016-03-15 15:31 Ruediger Meier
  2016-03-15 15:31 ` [PATCH 2/2] lscpu: use cpu and revision tag if available Ruediger Meier
  0 siblings, 1 reply; 6+ messages in thread
From: Ruediger Meier @ 2016-03-15 15:31 UTC (permalink / raw)
  To: util-linux

From: Ruediger Meier <ruediger.meier@ga-group.nl>

This reverts commit 641350fe822e7f1ac10873dad9a364bdeaba8083.

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 sys-utils/lscpu.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/sys-utils/lscpu.c b/sys-utils/lscpu.c
index 8e62c64..423bc56 100644
--- a/sys-utils/lscpu.c
+++ b/sys-utils/lscpu.c
@@ -453,36 +453,25 @@ read_basicinfo(struct lscpu_desc *desc, struct lscpu_modifier *mod)
 	char buf[BUFSIZ];
 	struct utsname utsbuf;
 	size_t setsize;
-	int overwrite_model = 0;
 
 	/* architecture */
 	if (uname(&utsbuf) == -1)
 		err(EXIT_FAILURE, _("error: uname failed"));
 	desc->arch = xstrdup(utsbuf.machine);
 
-	/* Use another records from cpuinfo for PPC, on snapshot follow
-	 * standard behavior.
-	 *
-	 * TODO: use runtime detection to make model overwrite possible on
-	 *       snapshots too.
-	 */
-#if defined(__powerpc__) || defined(__powerpc64__)
-	if (mod->system == SYSTEM_LIVE)
-		overwrite_model = 1;
-#endif
 	/* details */
 	while (fgets(buf, sizeof(buf), fp) != NULL) {
 		if (lookup(buf, "vendor", &desc->vendor)) ;
 		else if (lookup(buf, "vendor_id", &desc->vendor)) ;
 		else if (lookup(buf, "family", &desc->family)) ;
 		else if (lookup(buf, "cpu family", &desc->family)) ;
-
-		else if (overwrite_model && lookup(buf, "revision", &desc->model)) ;
-		else if (overwrite_model && lookup(buf, "cpu", &desc->modelname)) ;
-
-		else if (!overwrite_model && lookup(buf, "model", &desc->model)) ;
-		else if (!overwrite_model && lookup(buf, "model name", &desc->modelname)) ;
-
+#if defined(__powerpc__) || defined(__powerpc64__)
+		else if (lookup(buf, "revision", &desc->model)) ;
+		else if (lookup(buf, "cpu", &desc->modelname)) ;
+#else
+		else if (lookup(buf, "model", &desc->model)) ;
+		else if (lookup(buf, "model name", &desc->modelname)) ;
+#endif
 		else if (lookup(buf, "stepping", &desc->stepping)) ;
 		else if (lookup(buf, "cpu MHz", &desc->mhz)) ;
 		else if (lookup(buf, "flags", &desc->flags)) ;		/* x86 */
-- 
1.8.4.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] lscpu: use cpu and revision tag if available
  2016-03-15 15:31 [PATCH 1/2] Revert "lscpu: keep lscpu usable on snapshots" Ruediger Meier
@ 2016-03-15 15:31 ` Ruediger Meier
  2016-03-16  5:35   ` Vasant Hegde
  2016-03-16  7:57   ` Ruediger Meier
  0 siblings, 2 replies; 6+ messages in thread
From: Ruediger Meier @ 2016-03-15 15:31 UTC (permalink / raw)
  To: util-linux; +Cc: Vasant Hegde

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Avoid ifdef which does not work with --sysroot. Our existing test
dumps produce even better output now for ppc and sparc.

CC: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 sys-utils/lscpu.c                             | 18 +++++++++++++-----
 tests/expected/lscpu/lscpu-ppc-qemu           |  3 ++-
 tests/expected/lscpu/lscpu-ppc64-POWER7       |  3 ++-
 tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu |  3 ++-
 tests/expected/lscpu/lscpu-sparc64            |  1 +
 5 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/sys-utils/lscpu.c b/sys-utils/lscpu.c
index 423bc56..498234f 100644
--- a/sys-utils/lscpu.c
+++ b/sys-utils/lscpu.c
@@ -453,6 +453,8 @@ read_basicinfo(struct lscpu_desc *desc, struct lscpu_modifier *mod)
 	char buf[BUFSIZ];
 	struct utsname utsbuf;
 	size_t setsize;
+	char *cpu = 0;
+	char *revision = 0;
 
 	/* architecture */
 	if (uname(&utsbuf) == -1)
@@ -465,13 +467,8 @@ read_basicinfo(struct lscpu_desc *desc, struct lscpu_modifier *mod)
 		else if (lookup(buf, "vendor_id", &desc->vendor)) ;
 		else if (lookup(buf, "family", &desc->family)) ;
 		else if (lookup(buf, "cpu family", &desc->family)) ;
-#if defined(__powerpc__) || defined(__powerpc64__)
-		else if (lookup(buf, "revision", &desc->model)) ;
-		else if (lookup(buf, "cpu", &desc->modelname)) ;
-#else
 		else if (lookup(buf, "model", &desc->model)) ;
 		else if (lookup(buf, "model name", &desc->modelname)) ;
-#endif
 		else if (lookup(buf, "stepping", &desc->stepping)) ;
 		else if (lookup(buf, "cpu MHz", &desc->mhz)) ;
 		else if (lookup(buf, "flags", &desc->flags)) ;		/* x86 */
@@ -479,10 +476,21 @@ read_basicinfo(struct lscpu_desc *desc, struct lscpu_modifier *mod)
 		else if (lookup(buf, "type", &desc->flags)) ;		/* sparc64 */
 		else if (lookup(buf, "bogomips", &desc->bogomips)) ;
 		else if (lookup(buf, "bogomips per cpu", &desc->bogomips)) ; /* s390 */
+		else if (lookup(buf, "cpu", &cpu)) ;          /* modelname, ppc, sparc*/
+		else if (lookup(buf, "revision", &revision)) ;/* model, ppc */
 		else
 			continue;
 	}
 
+	if (cpu) {
+		free(desc->modelname);
+		desc->modelname = cpu;
+	}
+	if (revision) {
+		free(desc->model);
+		desc->model = revision;
+	}
+
 	desc->mode = init_mode(mod);
 
 	if (desc->flags) {
diff --git a/tests/expected/lscpu/lscpu-ppc-qemu b/tests/expected/lscpu/lscpu-ppc-qemu
index b3ea4a7..78b0b29 100644
--- a/tests/expected/lscpu/lscpu-ppc-qemu
+++ b/tests/expected/lscpu/lscpu-ppc-qemu
@@ -3,7 +3,8 @@ On-line CPU(s) list:   0
 Thread(s) per core:    1
 Core(s) per socket:    1
 Socket(s):             1
-Model:                 Power Macintosh
+Model:                 3.1 (pvr 0008 0301)
+Model name:            740/750
 BogoMIPS:              33.25
 L1d cache:             unknown size
 L1i cache:             unknown size
diff --git a/tests/expected/lscpu/lscpu-ppc64-POWER7 b/tests/expected/lscpu/lscpu-ppc64-POWER7
index 0d6c5ba..9a3c0c9 100644
--- a/tests/expected/lscpu/lscpu-ppc64-POWER7
+++ b/tests/expected/lscpu/lscpu-ppc64-POWER7
@@ -4,7 +4,8 @@ Thread(s) per core:    4
 Core(s) per socket:    1
 Socket(s):             4
 NUMA node(s):          1
-Model:                 IBM,8233-E8B
+Model:                 2.1 (pvr 003f 0201)
+Model name:            POWER7 (architected), altivec supported
 L1d cache:             32K
 L1i cache:             32K
 NUMA node0 CPU(s):     0-15
diff --git a/tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu b/tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu
index e48735e..0aed12a 100644
--- a/tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu
+++ b/tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu
@@ -4,7 +4,8 @@ Thread(s) per core:    4
 Core(s) per socket:    1
 Socket(s):             16
 NUMA node(s):          2
-Model:                 IBM,8231-E2B
+Model:                 2.1 (pvr 003f 0201)
+Model name:            POWER7 (architected), altivec supported
 Hypervisor vendor:     pHyp
 Virtualization type:   para
 L1d cache:             32K
diff --git a/tests/expected/lscpu/lscpu-sparc64 b/tests/expected/lscpu/lscpu-sparc64
index 629b399..aacaf11 100644
--- a/tests/expected/lscpu/lscpu-sparc64
+++ b/tests/expected/lscpu/lscpu-sparc64
@@ -4,6 +4,7 @@ On-line CPU(s) list:   6,7,10,11,14,15
 Thread(s) per core:    1
 Core(s) per socket:    1
 Socket(s):             6
+Model name:            TI UltraSparc II  (BlackBird)
 Flags:                 sun4u
 
 # The following is the parsable format, which can be fed to other
-- 
1.8.4.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] lscpu: use cpu and revision tag if available
  2016-03-15 15:31 ` [PATCH 2/2] lscpu: use cpu and revision tag if available Ruediger Meier
@ 2016-03-16  5:35   ` Vasant Hegde
  2016-03-16  7:57   ` Ruediger Meier
  1 sibling, 0 replies; 6+ messages in thread
From: Vasant Hegde @ 2016-03-16  5:35 UTC (permalink / raw)
  To: Ruediger Meier, util-linux

On 03/15/2016 09:01 PM, Ruediger Meier wrote:
> From: Ruediger Meier <ruediger.meier@ga-group.nl>
>
> Avoid ifdef which does not work with --sysroot. Our existing test
> dumps produce even better output now for ppc and sparc.
>
> CC: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>

Ruediger,

Thanks for the fix. Patch looks good. I've tested on Power system and it works fine.

Reviewed-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>


-Vasant


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] lscpu: use cpu and revision tag if available
  2016-03-15 15:31 ` [PATCH 2/2] lscpu: use cpu and revision tag if available Ruediger Meier
  2016-03-16  5:35   ` Vasant Hegde
@ 2016-03-16  7:57   ` Ruediger Meier
  2016-03-16  9:37     ` Karel Zak
  1 sibling, 1 reply; 6+ messages in thread
From: Ruediger Meier @ 2016-03-16  7:57 UTC (permalink / raw)
  To: util-linux; +Cc: Vasant Hegde

On Tuesday 15 March 2016, Ruediger Meier wrote:
> From: Ruediger Meier <ruediger.meier@ga-group.nl>
>
> Avoid ifdef which does not work with --sysroot. Our existing test
> dumps produce even better output now for ppc and sparc.
>
> CC: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
> ---
>  sys-utils/lscpu.c                             | 18
> +++++++++++++----- tests/expected/lscpu/lscpu-ppc-qemu           |  3
> ++-
>  tests/expected/lscpu/lscpu-ppc64-POWER7       |  3 ++-
>  tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu |  3 ++-
>  tests/expected/lscpu/lscpu-sparc64            |  1 +
>  5 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/sys-utils/lscpu.c b/sys-utils/lscpu.c
> index 423bc56..498234f 100644
> --- a/sys-utils/lscpu.c
> +++ b/sys-utils/lscpu.c
> @@ -453,6 +453,8 @@ read_basicinfo(struct lscpu_desc *desc, struct
> lscpu_modifier *mod) char buf[BUFSIZ];
>  	struct utsname utsbuf;
>  	size_t setsize;
> +	char *cpu = 0;
> +	char *revision = 0;
>
>  	/* architecture */
>  	if (uname(&utsbuf) == -1)
> @@ -465,13 +467,8 @@ read_basicinfo(struct lscpu_desc *desc, struct
> lscpu_modifier *mod) else if (lookup(buf, "vendor_id",
> &desc->vendor)) ;
>  		else if (lookup(buf, "family", &desc->family)) ;
>  		else if (lookup(buf, "cpu family", &desc->family)) ;
> -#if defined(__powerpc__) || defined(__powerpc64__)
> -		else if (lookup(buf, "revision", &desc->model)) ;
> -		else if (lookup(buf, "cpu", &desc->modelname)) ;
> -#else
>  		else if (lookup(buf, "model", &desc->model)) ;
>  		else if (lookup(buf, "model name", &desc->modelname)) ;
> -#endif
>  		else if (lookup(buf, "stepping", &desc->stepping)) ;
>  		else if (lookup(buf, "cpu MHz", &desc->mhz)) ;
>  		else if (lookup(buf, "flags", &desc->flags)) ;		/* x86 */
> @@ -479,10 +476,21 @@ read_basicinfo(struct lscpu_desc *desc, struct
> lscpu_modifier *mod) else if (lookup(buf, "type", &desc->flags))
> ;		/* sparc64 */ else if (lookup(buf, "bogomips", &desc->bogomips)) ;
>  		else if (lookup(buf, "bogomips per cpu", &desc->bogomips)) ; /*
> s390 */ +		else if (lookup(buf, "cpu", &cpu)) ;          /*
> modelname, ppc, sparc*/ +		else if (lookup(buf, "revision",
> &revision)) ;/* model, ppc */ else
>  			continue;
>  	}
>
> +	if (cpu) {
> +		free(desc->modelname);
> +		desc->modelname = cpu;
> +	}
> +	if (revision) {
> +		free(desc->model);
> +		desc->model = revision;
> +	}
> +

Watching this again today I think it could be even better to add cpu and 
revision members to the struct and move both if conditions to the 
printing code section.

It would avoid the small risk to break other logic like this one:
  if (desc->modelname && strstr(desc->modelname, "UML")) { ...

>  	desc->mode = init_mode(mod);
>
>  	if (desc->flags) {
> diff --git a/tests/expected/lscpu/lscpu-ppc-qemu
> b/tests/expected/lscpu/lscpu-ppc-qemu index b3ea4a7..78b0b29 100644
> --- a/tests/expected/lscpu/lscpu-ppc-qemu
> +++ b/tests/expected/lscpu/lscpu-ppc-qemu
> @@ -3,7 +3,8 @@ On-line CPU(s) list:   0
>  Thread(s) per core:    1
>  Core(s) per socket:    1
>  Socket(s):             1
> -Model:                 Power Macintosh
> +Model:                 3.1 (pvr 0008 0301)
> +Model name:            740/750
>  BogoMIPS:              33.25
>  L1d cache:             unknown size
>  L1i cache:             unknown size
> diff --git a/tests/expected/lscpu/lscpu-ppc64-POWER7
> b/tests/expected/lscpu/lscpu-ppc64-POWER7 index 0d6c5ba..9a3c0c9
> 100644
> --- a/tests/expected/lscpu/lscpu-ppc64-POWER7
> +++ b/tests/expected/lscpu/lscpu-ppc64-POWER7
> @@ -4,7 +4,8 @@ Thread(s) per core:    4
>  Core(s) per socket:    1
>  Socket(s):             4
>  NUMA node(s):          1
> -Model:                 IBM,8233-E8B
> +Model:                 2.1 (pvr 003f 0201)
> +Model name:            POWER7 (architected), altivec supported
>  L1d cache:             32K
>  L1i cache:             32K
>  NUMA node0 CPU(s):     0-15
> diff --git a/tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu
> b/tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu index
> e48735e..0aed12a 100644
> --- a/tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu
> +++ b/tests/expected/lscpu/lscpu-ppc64-POWER7-64cpu
> @@ -4,7 +4,8 @@ Thread(s) per core:    4
>  Core(s) per socket:    1
>  Socket(s):             16
>  NUMA node(s):          2
> -Model:                 IBM,8231-E2B
> +Model:                 2.1 (pvr 003f 0201)
> +Model name:            POWER7 (architected), altivec supported
>  Hypervisor vendor:     pHyp
>  Virtualization type:   para
>  L1d cache:             32K
> diff --git a/tests/expected/lscpu/lscpu-sparc64
> b/tests/expected/lscpu/lscpu-sparc64 index 629b399..aacaf11 100644
> --- a/tests/expected/lscpu/lscpu-sparc64
> +++ b/tests/expected/lscpu/lscpu-sparc64
> @@ -4,6 +4,7 @@ On-line CPU(s) list:   6,7,10,11,14,15
>  Thread(s) per core:    1
>  Core(s) per socket:    1
>  Socket(s):             6
> +Model name:            TI UltraSparc II  (BlackBird)
>  Flags:                 sun4u
>
>  # The following is the parsable format, which can be fed to other



^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] lscpu: use cpu and revision tag if available
  2016-03-16  7:57   ` Ruediger Meier
@ 2016-03-16  9:37     ` Karel Zak
  2016-03-16 10:23       ` Ruediger Meier
  0 siblings, 1 reply; 6+ messages in thread
From: Karel Zak @ 2016-03-16  9:37 UTC (permalink / raw)
  To: Ruediger Meier; +Cc: util-linux, Vasant Hegde

On Wed, Mar 16, 2016 at 08:57:26AM +0100, Ruediger Meier wrote:
> > +	if (cpu) {
> > +		free(desc->modelname);
> > +		desc->modelname = cpu;
> > +	}
> > +	if (revision) {
> > +		free(desc->model);
> > +		desc->model = revision;
> > +	}
> > +

The problem I see is that Linux kernel does not provide any unified
abstraction for /proc/cpuinfo, the file is generated individually by
architecture specific code, so the field names are very arch specific.
(use "git grep show_cpuinfo" in kernel tree to see more)

For example "revision" is no PPC specific, it's also used by ia64.
Alpha uses "cpu revision" and "cpu model", etc.

The ideal solution (for v2.29) would be to have more read_cpuinfo_<arch>()
functions to hide the differences. 

> Watching this again today I think it could be even better to add cpu and 
> revision members to the struct and move both if conditions to the 
> printing code section.

Do you want to send a new version of the patch?

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] lscpu: use cpu and revision tag if available
  2016-03-16  9:37     ` Karel Zak
@ 2016-03-16 10:23       ` Ruediger Meier
  0 siblings, 0 replies; 6+ messages in thread
From: Ruediger Meier @ 2016-03-16 10:23 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux, Vasant Hegde

On Wednesday 16 March 2016, Karel Zak wrote:
> On Wed, Mar 16, 2016 at 08:57:26AM +0100, Ruediger Meier wrote:
> > > +	if (cpu) {
> > > +		free(desc->modelname);
> > > +		desc->modelname = cpu;
> > > +	}
> > > +	if (revision) {
> > > +		free(desc->model);
> > > +		desc->model = revision;
> > > +	}
> > > +
>
> The problem I see is that Linux kernel does not provide any unified
> abstraction for /proc/cpuinfo, the file is generated individually by
> architecture specific code, so the field names are very arch
> specific. (use "git grep show_cpuinfo" in kernel tree to see more)
>
> For example "revision" is no PPC specific, it's also used by ia64.
> Alpha uses "cpu revision" and "cpu model", etc.

Yes, but the probability is high that the new code will do it better. 
Like in sparc case.

> The ideal solution (for v2.29) would be to have more
> read_cpuinfo_<arch>() functions to hide the differences.
>
> > Watching this again today I think it could be even better to add
> > cpu and revision members to the struct and move both if conditions
> > to the printing code section.
>
> Do you want to send a new version of the patch?

Yep.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-03-16 10:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-15 15:31 [PATCH 1/2] Revert "lscpu: keep lscpu usable on snapshots" Ruediger Meier
2016-03-15 15:31 ` [PATCH 2/2] lscpu: use cpu and revision tag if available Ruediger Meier
2016-03-16  5:35   ` Vasant Hegde
2016-03-16  7:57   ` Ruediger Meier
2016-03-16  9:37     ` Karel Zak
2016-03-16 10:23       ` Ruediger Meier

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.