From: Cliff Wickman <cpw@sgi.com>
To: Bill Gray <bgray@redhat.com>
Cc: linux-numa@vger.kernel.org
Subject: Re: [PATCH] Fix numactl --show preferred node for case MPOL_BIND
Date: Wed, 16 Jul 2014 07:24:50 -0500 [thread overview]
Message-ID: <20140716122450.GA14268@sgi.com> (raw)
In-Reply-To: <53C56A8A.8070207@redhat.com>
Hi Bill,
Thanks for the fix. It works great for me.
I've started a 2.0.10 version. Your patch is in
ftp://oss.sgi.com/www/projects/libnuma/download/ numactl-2.0.10-rc1.tar.gz
(I was beginning to think that numactl/libnuma were perfect -- no one else
had submitted a patch since last September!)
-Cliff
On Tue, Jul 15, 2014 at 01:53:14PM -0400, Bill Gray wrote:
>
> This patch is mostly to fix an issue in "numactl --show" which did
> not print the correct preferred node:
>
> # for i in 0 1 2 3 4 5 6 7 ; do numactl -N$i -m$i numactl -s; done |
> grep preferred
> preferred node: 4
> preferred node: 4
> preferred node: 4
> preferred node: 4
> preferred node: 4
> preferred node: 4
> preferred node: 4
> preferred node: 4
>
> # for i in 0 1 2 3 4 5 6 7 ; do ./numactl -N$i -m$i ./numactl -s;
> done | grep preferred
> preferred node: 0
> preferred node: 1
> preferred node: 2
> preferred node: 3
> preferred node: 4
> preferred node: 5
> preferred node: 6
> preferred node: 7
>
>
>
> Patch Changes:
> - eliminated bitops.[cho]
> - eliminated redundant printcpumask() (which duplicated printmask())
> - added find_first() to util.[ch]
> - fixed some compiler warnings
>
> These files can be deleted:
> - Only in numactl-2.0.9-orig/: bitops.c
> - Only in numactl-2.0.9-orig/: bitops.h
> - Only in numactl-2.0.9-orig/: numastat
> - Only in numactl-2.0.9-orig/test: move_pages
>
> diffstats:
> Makefile | 12 ++++++------
> bitops.c | 13 -------------
> bitops.h | 13 -------------
> migspeed.c | 4 +---
> numactl.c | 5 ++---
> numaint.h | 2 +-
> test/move_pages |binary
> test/nodemap.c | 1 -
> test/tbitmap.c | 10 +++++++++-
> util.c | 14 +++++---------
> util.h | 2 +-
> 11 files changed, 25 insertions(+), 51 deletions(-)
>
> Signed-off-by: Bill Gray <bgray@redhat.com>
>
>
>
> diff -purN numactl-2.0.9-orig/bitops.c numactl-2.0.9-new/bitops.c
> --- numactl-2.0.9-orig/bitops.c 2013-10-08 17:34:57.000000000 -0400
> +++ numactl-2.0.9-new/bitops.c 1969-12-31 19:00:00.000000000 -0500
> @@ -1,13 +0,0 @@
> -#include "bitops.h"
> -
> -/* extremly dumb */
> -int find_first_bit(void *m, int max)
> -{
> - unsigned long *mask = m;
> - int i;
> - for (i = 0; i < max; i++) {
> - if (test_bit(i, mask))
> - break;
> - }
> - return i;
> -}
> diff -purN numactl-2.0.9-orig/bitops.h numactl-2.0.9-new/bitops.h
> --- numactl-2.0.9-orig/bitops.h 2013-10-08 17:34:57.000000000 -0400
> +++ numactl-2.0.9-new/bitops.h 1969-12-31 19:00:00.000000000 -0500
> @@ -1,13 +0,0 @@
> -#ifndef BITOPS_H
> -#define BITOPS_H 1
> -
> -#define BITS_PER_LONG (sizeof(unsigned long) * 8)
> -#define BYTES_PER_LONG (sizeof(long))
> -
> -#define test_bit(i,p) ((p)[(i) / BITS_PER_LONG] & (1UL <<
> ((i)%BITS_PER_LONG)))
> -#define set_bit(i,p) ((p)[(i) / BITS_PER_LONG] |= (1UL <<
> ((i)%BITS_PER_LONG)))
> -#define clear_bit(i,p) ((p)[(i) / BITS_PER_LONG] &= ~(1UL <<
> ((i)%BITS_PER_LONG)))
> -
> -extern int find_first_bit(void *mask, int max);
> -
> -#endif
> diff -purN numactl-2.0.9-orig/Makefile numactl-2.0.9-new/Makefile
> --- numactl-2.0.9-orig/Makefile 2013-10-08 17:34:57.000000000 -0400
> +++ numactl-2.0.9-new/Makefile 2014-07-15 10:04:59.782383271 -0400
> @@ -23,7 +23,7 @@ ifeq ($(THREAD_SUPPORT),yes)
> endif
>
> CLEANFILES := numactl.o libnuma.o numactl numademo numademo.o distance.o \
> - memhog libnuma.so libnuma.so.1 numamon numamon.o syscall.o
> bitops.o \
> + memhog libnuma.so libnuma.so.1 numamon numamon.o syscall.o \
> memhog.o util.o stream_main.o stream_lib.o shm.o stream
> clearcache.o \
> test/pagesize test/tshared test/mynode.o test/tshared.o mt.o
> empty.o empty.c \
> test/mynode test/ftok test/prefered test/randmap \
> @@ -32,8 +32,8 @@ CLEANFILES := numactl.o libnuma.o numact
> test/mbind_mig_pages test/migrate_pages \
> migratepages migspeed migspeed.o libnuma.a \
> test/move_pages test/realloc_test sysfs.o affinity.o \
> - test/node-parse rtnetlink.o test/A numastat
> -SOURCES := bitops.c libnuma.c distance.c memhog.c numactl.c numademo.c \
> + test/node-parse rtnetlink.o test/A numastat numastat.o
> +SOURCES := libnuma.c distance.c memhog.c numactl.c numademo.c \
> numamon.c shm.c stream_lib.c stream_main.c syscall.c util.c mt.c \
> clearcache.c test/*.c affinity.c sysfs.c rtnetlink.c numastat.c
>
> @@ -51,11 +51,11 @@ all: numactl migratepages migspeed libnu
> test/mbind_mig_pages test/migrate_pages test/realloc_test libnuma.a \
> test/node-parse numastat
>
> -numactl: numactl.o util.o shm.o bitops.o libnuma.so
> +numactl: numactl.o util.o shm.o libnuma.so
>
> numastat: CFLAGS += -std=gnu99
>
> -migratepages: migratepages.c util.o bitops.o libnuma.so
> +migratepages: migratepages.c util.o libnuma.so
>
> migspeed: LDLIBS += -lrt
> migspeed: migspeed.o util.o libnuma.so
> @@ -129,7 +129,7 @@ test/nodemap: test/nodemap.c libnuma.so
>
> test/distance: test/distance.c libnuma.so
>
> -test/tbitmap: test/tbitmap.c libnuma.so
> +test/tbitmap: test/tbitmap.c libnuma.so util.o
>
> test/move_pages: test/move_pages.c libnuma.so
>
> diff -purN numactl-2.0.9-orig/migspeed.c numactl-2.0.9-new/migspeed.c
> --- numactl-2.0.9-orig/migspeed.c 2013-10-08 17:34:57.000000000 -0400
> +++ numactl-2.0.9-new/migspeed.c 2014-07-15 00:32:09.557437964 -0400
> @@ -65,7 +65,6 @@ int main(int argc, char *argv[])
> {
> char *p;
> int option;
> - int error = 0;
> struct timespec result;
> unsigned long bytes;
> double duration, mbytes;
> @@ -82,8 +81,7 @@ int main(int argc, char *argv[])
> switch (option) {
> case 'h' :
> case '?' :
> - error = 1;
> - break;
> + usage();
> case 'v' :
> verbose++;
> break;
> diff -purN numactl-2.0.9-orig/numactl.c numactl-2.0.9-new/numactl.c
> --- numactl-2.0.9-orig/numactl.c 2013-10-08 17:34:57.000000000 -0400
> +++ numactl-2.0.9-new/numactl.c 2014-07-15 00:23:45.234430937 -0400
> @@ -119,7 +119,7 @@ void show_physcpubind(void)
> }
> err("sched_get_affinity");
> }
> - printcpumask("physcpubind", cpubuf);
> + printmask("physcpubind", cpubuf);
> break;
> }
> }
> @@ -130,7 +130,6 @@ void show(void)
> struct bitmask *membind, *interleave, *cpubind;
> unsigned long cur;
> int policy;
> - int numa_num_nodes = numa_num_possible_nodes();
>
> if (numa_available() < 0) {
> show_physcpubind();
> @@ -166,7 +165,7 @@ void show(void)
> printf("%ld (interleave next)\n",cur);
> break;
> case MPOL_BIND:
> - printf("%d\n", find_first_bit(&membind, numa_num_nodes));
> + printf("%d\n", find_first(membind));
> break;
> }
> if (policy == MPOL_INTERLEAVE) {
> diff -purN numactl-2.0.9-orig/numaint.h numactl-2.0.9-new/numaint.h
> --- numactl-2.0.9-orig/numaint.h 2013-10-08 17:34:57.000000000 -0400
> +++ numactl-2.0.9-new/numaint.h 2014-07-15 00:28:55.156446951 -0400
> @@ -1,5 +1,4 @@
> /* Internal interfaces of libnuma */
> -#include "bitops.h"
>
> extern int numa_sched_setaffinity_v1(pid_t pid, unsigned len, const
> unsigned long *mask);
> extern int numa_sched_getaffinity_v1(pid_t pid, unsigned len, const
> unsigned long *mask);
> @@ -12,6 +11,7 @@ extern int numa_sched_getaffinity_v2_int
>
> #define SHM_HUGETLB 04000 /* segment will use huge TLB pages */
>
> +#define BITS_PER_LONG (sizeof(unsigned long) * 8)
> #define CPU_BYTES(x) (round_up(x, BITS_PER_LONG)/8)
> #define CPU_LONGS(x) (CPU_BYTES(x) / sizeof(long))
>
> Binary files numactl-2.0.9-orig/test/move_pages and
> numactl-2.0.9-new/test/move_pages differ
> diff -purN numactl-2.0.9-orig/test/nodemap.c
> numactl-2.0.9-new/test/nodemap.c
> --- numactl-2.0.9-orig/test/nodemap.c 2013-10-08 17:34:58.000000000 -0400
> +++ numactl-2.0.9-new/test/nodemap.c 2014-07-15 00:23:02.183439045 -0400
> @@ -1,5 +1,4 @@
> #include "numa.h"
> -#include "bitops.h"
> #include <stdio.h>
> #include <stdlib.h>
>
> diff -purN numactl-2.0.9-orig/test/tbitmap.c
> numactl-2.0.9-new/test/tbitmap.c
> --- numactl-2.0.9-orig/test/tbitmap.c 2013-10-08 17:34:58.000000000 -0400
> +++ numactl-2.0.9-new/test/tbitmap.c 2014-07-15 13:28:35.060368436 -0400
> @@ -7,6 +7,13 @@
> #include <stdlib.h>
> #include <ctype.h>
> #include "numa.h"
> +#include "util.h"
> +
> +/* For util.c. Fixme. */
> +void usage(void)
> +{
> + exit(1);
> +}
>
> #define ALIGN(x,a) (((x)+(a)-1)&~((a)-1))
>
> @@ -81,11 +88,12 @@ int main(void)
> numa_bitmask_clearall(mask);
> numa_bitmask_clearall(mask2);
> numa_bitmask_setbit(mask, i);
> + assert(find_first(mask) == i);
> bitmap_scnprintf(buf, sizeof(buf), mask);
> strcat(buf,"\n");
> if (numa_parse_bitmap(buf, mask2) < 0)
> assert(0);
> - if (memcmp(mask, mask2, sizeof(mask))) {
> + if (memcmp(mask->maskp, mask2->maskp, numa_bitmask_nbytes(mask))) {
> bitmap_scnprintf(buf, sizeof(buf), mask2);
> printf("mask2 differs: %s\n", buf);
> assert(0);
> diff -purN numactl-2.0.9-orig/util.c numactl-2.0.9-new/util.c
> --- numactl-2.0.9-orig/util.c 2013-10-08 17:34:59.000000000 -0400
> +++ numactl-2.0.9-new/util.c 2014-07-15 12:31:04.782342890 -0400
> @@ -16,7 +16,6 @@
> #include "numa.h"
> #include "numaif.h"
> #include "util.h"
> -#include "bitops.h"
> #include <stdio.h>
> #include <string.h>
> #include <stdlib.h>
> @@ -28,23 +27,20 @@
> void printmask(char *name, struct bitmask *mask)
> {
> int i;
> -
> printf("%s: ", name);
> - for (i = 0; i <= mask->size; i++)
> + for (i = 0; i < mask->size; i++)
> if (numa_bitmask_isbitset(mask, i))
> printf("%d ", i);
> putchar('\n');
> }
>
> -void printcpumask(char *name, struct bitmask *mask)
> +int find_first(struct bitmask *mask)
> {
> int i;
> - printf("%s: ", name);
> - for (i = 0; i < mask->size; i++) {
> + for (i = 0; i < mask->size; i++)
> if (numa_bitmask_isbitset(mask, i))
> - printf("%d ", i);
> - }
> - putchar('\n');
> + return i;
> + return -1;
> }
>
> void complain(char *fmt, ...)
> diff -purN numactl-2.0.9-orig/util.h numactl-2.0.9-new/util.h
> --- numactl-2.0.9-orig/util.h 2013-10-08 17:34:59.000000000 -0400
> +++ numactl-2.0.9-new/util.h 2014-07-15 12:31:07.990383476 -0400
> @@ -1,5 +1,5 @@
> extern void printmask(char *name, struct bitmask *mask);
> -extern void printcpumask(char *name, struct bitmask *mask);
> +extern int find_first(struct bitmask *mask);
> extern struct bitmask *nodemask(char *s);
> extern struct bitmask *cpumask(char *s, int *ncpus);
> extern int read_sysctl(char *name);
--
Cliff Wickman
SGI
cpw@sgi.com
(651) 683-3824
(651) 482-9347h
prev parent reply other threads:[~2014-07-16 12:24 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-15 17:53 [PATCH] Fix numactl --show preferred node for case MPOL_BIND Bill Gray
2014-07-16 12:24 ` Cliff Wickman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140716122450.GA14268@sgi.com \
--to=cpw@sgi.com \
--cc=bgray@redhat.com \
--cc=linux-numa@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.