Util-Linux package development
 help / color / mirror / Atom feed
* [PATCH] lscpu: add support for books
@ 2011-07-05 11:29 Heiko Carstens
  2011-07-11 10:40 ` Karel Zak
  0 siblings, 1 reply; 7+ messages in thread
From: Heiko Carstens @ 2011-07-05 11:29 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

From: Heiko Carstens <heiko.carstens@de.ibm.com>

This patch adds support for books in cpu topology output. Books are
currently only present on the s390 architecture, however it looks like
others will follow to use the extra scheduling domain of the kernel.

Books are logically between sockets and nodes. Therefore change the
parseable output accordingly to reflect this. Please note that I assume
that other programs that get fed by the output of lscpu actually parse
the last comment line and react in a sane way if new entries appear in
the cpu list.

Also the readable output is changed from
"CPU socket(s):" to "Socket(s) per book:" or simply "Socket(s):" in the
absence of books.
    
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---

 sys-utils/lscpu.c |   46 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/sys-utils/lscpu.c b/sys-utils/lscpu.c
index 0c49b54..91cf97d 100644
--- a/sys-utils/lscpu.c
+++ b/sys-utils/lscpu.c
@@ -113,6 +113,11 @@ struct lscpu_desc {
 	int		nnodes;		/* number of NUMA modes */
 	cpu_set_t	**nodemaps;	/* array with NUMA nodes */
 
+	/* books -- based on book_siblings (internal kernel map of cpuX's
+	 * hardware threads within the same book */
+	int		nbooks;		/* number of all online books */
+	cpu_set_t	**bookmaps;	/* unique book_siblings */
+
 	/* sockets -- based on core_siblings (internal kernel map of cpuX's
 	 * hardware threads within the same physical_package_id (socket)) */
 	int		nsockets;	/* number of all online sockets */
@@ -583,7 +588,7 @@ static int add_cpuset_to_array(cpu_set_t **ary, int *items, cpu_set_t *set)
 static void
 read_topology(struct lscpu_desc *desc, int num)
 {
-	cpu_set_t *thread_siblings, *core_siblings;
+	cpu_set_t *thread_siblings, *core_siblings, *book_siblings;
 
 	if (!path_exist(_PATH_SYS_CPU "/cpu%d/topology/thread_siblings", num))
 		return;
@@ -592,17 +597,24 @@ read_topology(struct lscpu_desc *desc, int num)
 					"/cpu%d/topology/thread_siblings", num);
 	core_siblings = path_cpuset(_PATH_SYS_CPU
 					"/cpu%d/topology/core_siblings", num);
+	book_siblings = NULL;
+	if (path_exist(_PATH_SYS_CPU "/cpu%d/topology/book_siblings", num)) {
+		book_siblings = path_cpuset(_PATH_SYS_CPU
+					    "/cpu%d/topology/book_siblings", num);
+	}
 
 	if (!desc->coremaps) {
-		int ncores, nsockets, nthreads;
+		int nbooks, nsockets, ncores, nthreads;
 		size_t setsize = CPU_ALLOC_SIZE(maxcpus);
 
 		/* threads within one core */
 		nthreads = CPU_COUNT_S(setsize, thread_siblings);
 		/* cores within one socket */
 		ncores = CPU_COUNT_S(setsize, core_siblings) / nthreads;
-		/* number of sockets */
+		/* number of sockets within one book */
 		nsockets = desc->ncpus / nthreads / ncores;
+		/* number of books */
+		nbooks = desc->ncpus / nthreads / ncores / nsockets;
 
 		/* all threads, see also read_basicinfo()
 		 * -- this is fallback for kernels where is not
@@ -610,7 +622,11 @@ read_topology(struct lscpu_desc *desc, int num)
 		 */
 		if (!desc->nthreads)
 			desc->nthreads = nsockets * ncores * nthreads;
-
+		if (book_siblings) {
+			desc->bookmaps = calloc(nbooks, sizeof(cpu_set_t *));
+			if (!desc->bookmaps)
+				err(EXIT_FAILURE, _("error: calloc failed"));
+		}
 		desc->socketmaps = calloc(nsockets, sizeof(cpu_set_t *));
 		if (!desc->socketmaps)
 			err(EXIT_FAILURE, _("error: calloc failed"));
@@ -621,6 +637,8 @@ read_topology(struct lscpu_desc *desc, int num)
 
 	add_cpuset_to_array(desc->socketmaps, &desc->nsockets, core_siblings);
 	add_cpuset_to_array(desc->coremaps, &desc->ncores, thread_siblings);
+	if (book_siblings)
+		add_cpuset_to_array(desc->bookmaps, &desc->nbooks, book_siblings);
 }
 
 static int
@@ -732,7 +750,7 @@ print_parsable(struct lscpu_desc *desc)
 	"# The following is the parsable format, which can be fed to other\n"
 	"# programs. Each different item in every column has an unique ID\n"
 	"# starting from zero.\n"
-	"# CPU,Core,Socket,Node"));
+	"# CPU,Core,Socket,Book,Node"));
 
 	if (desc->ncaches) {
 		/* separator between CPU topology and cache information */
@@ -771,6 +789,16 @@ print_parsable(struct lscpu_desc *desc)
 		if (j == desc->nsockets)
 			putchar(',');
 
+		/* Book */
+		for (j = 0; j < desc->nbooks; j++) {
+			if (CPU_ISSET_S(i, setsize, desc->bookmaps[j])) {
+				printf(",%d", j);
+				break;
+			}
+		}
+		if (j == desc->nbooks)
+			putchar(',');
+
 		/* Nodes */
 		for (j = 0; j < desc->nnodes; j++) {
 			if (CPU_ISSET_S(i, setsize, desc->nodemaps[j])) {
@@ -883,9 +911,13 @@ print_readable(struct lscpu_desc *desc, int hex)
 	if (desc->nsockets) {
 		print_n(_("Thread(s) per core:"), desc->nthreads / desc->ncores);
 		print_n(_("Core(s) per socket:"), desc->ncores / desc->nsockets);
-		print_n(_("CPU socket(s):"), desc->nsockets);
+		if (desc->nbooks) {
+			print_n(_("Socket(s) per book:"), desc->nsockets / desc->nbooks);
+ 			print_n(_("Book(s):"), desc->nbooks);
+		} else {
+			print_n(_("Socket(s):"), desc->nsockets);
+		}
 	}
-
 	if (desc->nnodes)
 		print_n(_("NUMA node(s):"), desc->nnodes);
 	if (desc->vendor)

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

* Re: [PATCH] lscpu: add support for books
  2011-07-05 11:29 [PATCH] lscpu: add support for books Heiko Carstens
@ 2011-07-11 10:40 ` Karel Zak
  2011-07-13  3:46   ` Heiko Carstens
  0 siblings, 1 reply; 7+ messages in thread
From: Karel Zak @ 2011-07-11 10:40 UTC (permalink / raw)
  To: Heiko Carstens; +Cc: util-linux

On Tue, Jul 05, 2011 at 01:29:10PM +0200, Heiko Carstens wrote:
> This patch adds support for books in cpu topology output. 

 Thanks!

> Please note that I assume that other programs that get fed by the
> output of lscpu actually parse the last comment line and react in a
> sane way if new entries appear in the cpu list.

 Never assume anything :-)

> Also the readable output is changed from
> "CPU socket(s):" to "Socket(s) per book:" or simply "Socket(s):" in the
> absence of books.

 You're right, the "Socket(s)" is probably better. Unfortunately it's
 not backwardly compatible (for people who use "lscpu | grep ..."), so
 I'll add a note to the ReleaseNotes...

> @@ -732,7 +750,7 @@ print_parsable(struct lscpu_desc *desc)
>  	"# The following is the parsable format, which can be fed to other\n"
>  	"# programs. Each different item in every column has an unique ID\n"
>  	"# starting from zero.\n"
> -	"# CPU,Core,Socket,Node"));
> +	"# CPU,Core,Socket,Book,Node"));

 It would be better to use

    # CPU,Core,Socket,Node,Book

 to keep it usable for stupid scripts where the header is not parsed.

    Karel

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

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

* Re: [PATCH] lscpu: add support for books
  2011-07-11 10:40 ` Karel Zak
@ 2011-07-13  3:46   ` Heiko Carstens
  2011-07-18  9:10     ` Karel Zak
  0 siblings, 1 reply; 7+ messages in thread
From: Heiko Carstens @ 2011-07-13  3:46 UTC (permalink / raw)
  To: Karel Zak; +Cc: util-linux

On Mon, Jul 11, 2011 at 12:40:35PM +0200, Karel Zak wrote:
> > -	"# CPU,Core,Socket,Node"));
> > +	"# CPU,Core,Socket,Book,Node"));
> 
>  It would be better to use
> 
>     # CPU,Core,Socket,Node,Book
> 
>  to keep it usable for stupid scripts where the header is not parsed.

Hi Karel,

thanks. I updated the patch accordingly (see below).
Just another thing: there are more per cpu informations that are present
on s390 that I would also like to the parseable output. However, somehow
it won't fit to the current approach that lscpu -p prints everything
with a unique id starting from zero.
For example the cpus on s390 can be in any of one of the states
horizontal,vertical:low,vertical:medium or vertical:high (that's just
an information of how the hypervisor schedules the cpus).
How is that supposed to be mapped to current approach?
Map these simply to numbers? E.g. horizontal would be mapped to 0,
vertical:low would be mapped to 1 and so on?
Also would I also need a new seperation character between caches and
new information?
I'm asking because the output of caches is optional and if something
new would be added, it seem to get messy in the long term because of
all seperation characters that may or may not be there. No?

Anyway, here is the updated patch for book support:

Subject: [PATCH] lscpu: add support for books

From: Heiko Carstens <heiko.carstens@de.ibm.com>

This patch adds support for books in cpu topology output. Books are
currently only present on the s390 architecture, however it looks like
others will follow to use the extra scheduling domain of the kernel.

Books are logically between sockets and nodes. In order to not break
any existing tools that might parse the output of lscpu the output
is changed so that books will follow nodes:
CPU,Core,Socket,Node,Book

In addition the readable output is changed from
"CPU socket(s):" to "Socket(s) per book:" or simply "Socket(s):" in the
absence of books.
    
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---

 sys-utils/lscpu.c |   46 +++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 7 deletions(-)

Index: util-linux-ng/sys-utils/lscpu.c
===================================================================
--- util-linux-ng.orig/sys-utils/lscpu.c
+++ util-linux-ng/sys-utils/lscpu.c
@@ -113,6 +113,11 @@ struct lscpu_desc {
 	int		nnodes;		/* number of NUMA modes */
 	cpu_set_t	**nodemaps;	/* array with NUMA nodes */
 
+	/* books -- based on book_siblings (internal kernel map of cpuX's
+	 * hardware threads within the same book */
+	int		nbooks;		/* number of all online books */
+	cpu_set_t	**bookmaps;	/* unique book_siblings */
+
 	/* sockets -- based on core_siblings (internal kernel map of cpuX's
 	 * hardware threads within the same physical_package_id (socket)) */
 	int		nsockets;	/* number of all online sockets */
@@ -583,7 +588,7 @@ static int add_cpuset_to_array(cpu_set_t
 static void
 read_topology(struct lscpu_desc *desc, int num)
 {
-	cpu_set_t *thread_siblings, *core_siblings;
+	cpu_set_t *thread_siblings, *core_siblings, *book_siblings;
 
 	if (!path_exist(_PATH_SYS_CPU "/cpu%d/topology/thread_siblings", num))
 		return;
@@ -592,17 +597,24 @@ read_topology(struct lscpu_desc *desc, i
 					"/cpu%d/topology/thread_siblings", num);
 	core_siblings = path_cpuset(_PATH_SYS_CPU
 					"/cpu%d/topology/core_siblings", num);
+	book_siblings = NULL;
+	if (path_exist(_PATH_SYS_CPU "/cpu%d/topology/book_siblings", num)) {
+		book_siblings = path_cpuset(_PATH_SYS_CPU
+					    "/cpu%d/topology/book_siblings", num);
+	}
 
 	if (!desc->coremaps) {
-		int ncores, nsockets, nthreads;
+		int nbooks, nsockets, ncores, nthreads;
 		size_t setsize = CPU_ALLOC_SIZE(maxcpus);
 
 		/* threads within one core */
 		nthreads = CPU_COUNT_S(setsize, thread_siblings);
 		/* cores within one socket */
 		ncores = CPU_COUNT_S(setsize, core_siblings) / nthreads;
-		/* number of sockets */
+		/* number of sockets within one book */
 		nsockets = desc->ncpus / nthreads / ncores;
+		/* number of books */
+		nbooks = desc->ncpus / nthreads / ncores / nsockets;
 
 		/* all threads, see also read_basicinfo()
 		 * -- this is fallback for kernels where is not
@@ -610,7 +622,11 @@ read_topology(struct lscpu_desc *desc, i
 		 */
 		if (!desc->nthreads)
 			desc->nthreads = nsockets * ncores * nthreads;
-
+		if (book_siblings) {
+			desc->bookmaps = calloc(nbooks, sizeof(cpu_set_t *));
+			if (!desc->bookmaps)
+				err(EXIT_FAILURE, _("error: calloc failed"));
+		}
 		desc->socketmaps = calloc(nsockets, sizeof(cpu_set_t *));
 		if (!desc->socketmaps)
 			err(EXIT_FAILURE, _("error: calloc failed"));
@@ -621,6 +637,8 @@ read_topology(struct lscpu_desc *desc, i
 
 	add_cpuset_to_array(desc->socketmaps, &desc->nsockets, core_siblings);
 	add_cpuset_to_array(desc->coremaps, &desc->ncores, thread_siblings);
+	if (book_siblings)
+		add_cpuset_to_array(desc->bookmaps, &desc->nbooks, book_siblings);
 }
 
 static int
@@ -732,7 +750,7 @@ print_parsable(struct lscpu_desc *desc)
 	"# The following is the parsable format, which can be fed to other\n"
 	"# programs. Each different item in every column has an unique ID\n"
 	"# starting from zero.\n"
-	"# CPU,Core,Socket,Node"));
+	"# CPU,Core,Socket,Node,Book"));
 
 	if (desc->ncaches) {
 		/* separator between CPU topology and cache information */
@@ -781,6 +799,16 @@ print_parsable(struct lscpu_desc *desc)
 		if (j == desc->nnodes)
 			putchar(',');
 
+		/* Book */
+		for (j = 0; j < desc->nbooks; j++) {
+			if (CPU_ISSET_S(i, setsize, desc->bookmaps[j])) {
+				printf(",%d", j);
+				break;
+			}
+		}
+		if (j == desc->nbooks)
+			putchar(',');
+
 		if (desc->ncaches)
 			putchar(',');
 
@@ -883,9 +911,13 @@ print_readable(struct lscpu_desc *desc, 
 	if (desc->nsockets) {
 		print_n(_("Thread(s) per core:"), desc->nthreads / desc->ncores);
 		print_n(_("Core(s) per socket:"), desc->ncores / desc->nsockets);
-		print_n(_("CPU socket(s):"), desc->nsockets);
+		if (desc->nbooks) {
+			print_n(_("Socket(s) per book:"), desc->nsockets / desc->nbooks);
+ 			print_n(_("Book(s):"), desc->nbooks);
+		} else {
+			print_n(_("Socket(s):"), desc->nsockets);
+		}
 	}
-
 	if (desc->nnodes)
 		print_n(_("NUMA node(s):"), desc->nnodes);
 	if (desc->vendor)

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

* Re: [PATCH] lscpu: add support for books
  2011-07-13  3:46   ` Heiko Carstens
@ 2011-07-18  9:10     ` Karel Zak
  2011-07-18 15:17       ` Jon Stanley
  0 siblings, 1 reply; 7+ messages in thread
From: Karel Zak @ 2011-07-18  9:10 UTC (permalink / raw)
  To: Heiko Carstens; +Cc: util-linux

On Wed, Jul 13, 2011 at 09:16:18AM +0530, Heiko Carstens wrote:
> thanks. I updated the patch accordingly (see below).

 Applied, thanks.

> Just another thing: there are more per cpu informations that are present
> on s390 that I would also like to the parseable output. However, somehow
> it won't fit to the current approach that lscpu -p prints everything
> with a unique id starting from zero.
> For example the cpus on s390 can be in any of one of the states
> horizontal,vertical:low,vertical:medium or vertical:high (that's just
> an information of how the hypervisor schedules the cpus).
> How is that supposed to be mapped to current approach?
> Map these simply to numbers? E.g. horizontal would be mapped to 0,
> vertical:low would be mapped to 1 and so on?

 Probably, the most important is keep it backwardly compatible ;-)

> Also would I also need a new seperation character between caches and
> new information?

 I'm not sure if the currently used extra separators (,,) for the
 caches is a good idea. Maybe it would be better to force people to
 parse the last comment line where is the header for the columns.

> I'm asking because the output of caches is optional and if something
> new would be added, it seem to get messy in the long term because of
> all seperation characters that may or may not be there. No?

 I agree. Maybe you can add the new things before the caches (as you
 already added 'Book' column).

 The ideal solution is to extend the "-p" functionality and allow to 
 specify expected columns at command line, something like:

    lscpu -p -o cpu,core,book,socket

 We already use the same idea for findmnt and lscpu.

    Karel

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

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

* Re: [PATCH] lscpu: add support for books
  2011-07-18  9:10     ` Karel Zak
@ 2011-07-18 15:17       ` Jon Stanley
  2011-07-18 22:21         ` Karel Zak
  0 siblings, 1 reply; 7+ messages in thread
From: Jon Stanley @ 2011-07-18 15:17 UTC (permalink / raw)
  To: Karel Zak; +Cc: Heiko Carstens, util-linux

On Mon, Jul 18, 2011 at 5:10 AM, Karel Zak <kzak@redhat.com> wrote:

> =A0I'm not sure if the currently used extra separators (,,) for the
> =A0caches is a good idea. Maybe it would be better to force people to
> =A0parse the last comment line where is the header for the columns.

Speaking as a user of lscpu, I think that forcing people to parse the
last comment line is not a particularly good idea - no one is used to
doing it, and I'm not sure of any other utility that forces you to
parse it's "machine-readable" output. Stability of this format is key.

> =A0The ideal solution is to extend the "-p" functionality and allow to
> =A0specify expected columns at command line, something like:
>
> =A0 =A0lscpu -p -o cpu,core,book,socket

I think that this is the only long-term supportable way to do this.
CPU architectures (even in the x86 world that I'm interested in) ARE
going to change and evolve. Heck, it's even possible that concepts
like the hypervisor scheduling parameters that you mentioned on s390
could eventually make their way down to x86 virtualization, and
exposing stuff like that in lscpu would be nice.

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

* Re: [PATCH] lscpu: add support for books
  2011-07-18 15:17       ` Jon Stanley
@ 2011-07-18 22:21         ` Karel Zak
  2011-07-27 21:35           ` Karel Zak
  0 siblings, 1 reply; 7+ messages in thread
From: Karel Zak @ 2011-07-18 22:21 UTC (permalink / raw)
  To: Jon Stanley; +Cc: Heiko Carstens, util-linux

On Mon, Jul 18, 2011 at 11:17:24AM -0400, Jon Stanley wrote:
> On Mon, Jul 18, 2011 at 5:10 AM, Karel Zak <kzak@redhat.com> wrote:
> 
> >  I'm not sure if the currently used extra separators (,,) for the
> >  caches is a good idea. Maybe it would be better to force people to
> >  parse the last comment line where is the header for the columns.
> 
> Speaking as a user of lscpu, I think that forcing people to parse the
> last comment line is not a particularly good idea - no one is used to
> doing it, and I'm not sure of any other utility that forces you to
> parse it's "machine-readable" output. Stability of this format is key.

 Yes, -p sucks ;-)

> >  The ideal solution is to extend the "-p" functionality and allow to
> >  specify expected columns at command line, something like:
> >
> >    lscpu -p -o cpu,core,book,socket
> 
> I think that this is the only long-term supportable way to do this.
> CPU architectures (even in the x86 world that I'm interested in) ARE
> going to change and evolve. Heck, it's even possible that concepts
> like the hypervisor scheduling parameters that you mentioned on s390
> could eventually make their way down to x86 virtualization, and
> exposing stuff like that in lscpu would be nice.

 I'll try to add this functionality in next days.

    Karel

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

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

* Re: [PATCH] lscpu: add support for books
  2011-07-18 22:21         ` Karel Zak
@ 2011-07-27 21:35           ` Karel Zak
  0 siblings, 0 replies; 7+ messages in thread
From: Karel Zak @ 2011-07-27 21:35 UTC (permalink / raw)
  To: Jon Stanley; +Cc: Heiko Carstens, util-linux

On Tue, Jul 19, 2011 at 12:21:30AM +0200, Karel Zak wrote:
> On Mon, Jul 18, 2011 at 11:17:24AM -0400, Jon Stanley wrote:
> > On Mon, Jul 18, 2011 at 5:10 AM, Karel Zak <kzak@redhat.com> wrote:
> > 
> > >  I'm not sure if the currently used extra separators (,,) for the
> > >  caches is a good idea. Maybe it would be better to force people to
> > >  parse the last comment line where is the header for the columns.
> > 
> > Speaking as a user of lscpu, I think that forcing people to parse the
> > last comment line is not a particularly good idea - no one is used to
> > doing it, and I'm not sure of any other utility that forces you to
> > parse it's "machine-readable" output. Stability of this format is key.
> 
>  Yes, -p sucks ;-)
> 
> > >  The ideal solution is to extend the "-p" functionality and allow to
> > >  specify expected columns at command line, something like:
> > >
> > >    lscpu -p -o cpu,core,book,socket
> > 
> > I think that this is the only long-term supportable way to do this.
> > CPU architectures (even in the x86 world that I'm interested in) ARE
> > going to change and evolve. Heck, it's even possible that concepts
> > like the hypervisor scheduling parameters that you mentioned on s390
> > could eventually make their way down to x86 virtualization, and
> > exposing stuff like that in lscpu would be nice.
> 
>  I'll try to add this functionality in next days.

Implemented.

The command "lscpu -p" without any other argument is backwardly
compatible and does not include Books in the output, for example:

  $ lscpu -p
  # CPU,Core,Socket,Node,,L1d,L1i,L2
  0,0,0,0,,0,0,0
  1,1,0,0,,1,1,0

The command "lscpu -p=<columns>" prints always all requested 
columns in the defined order. The caches are separated by ':', for
example:

  $ lscpu -p=CPU,NODE,CACHE
  # CPU,Node,L1d:L1i:L2
  0,0,0:0:0
  1,0,1:1:0

  $ lscpu -p=CPU,NODE,CACHE,BOOK,SOCKET
  # CPU,Node,L1d:L1i:L2,Book,Socket
  0,0,0:0:0,,0
  1,0,1:1:0,,0

(yeah, no Books on my ThinkPad :-)

    Karel

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

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

end of thread, other threads:[~2011-07-27 21:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-05 11:29 [PATCH] lscpu: add support for books Heiko Carstens
2011-07-11 10:40 ` Karel Zak
2011-07-13  3:46   ` Heiko Carstens
2011-07-18  9:10     ` Karel Zak
2011-07-18 15:17       ` Jon Stanley
2011-07-18 22:21         ` Karel Zak
2011-07-27 21:35           ` Karel Zak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox