small patch for numactl. Comments?

Discussion:

Warner Losh

2017-11-14 03:54:05 UTC

.Va NUMACTL=YES

The rest looks good...

Warner

+before running

+.Ar cmd ...
+so that programs that know that they need to be run under
+.Nm
+can check.
.Sh EXAMPLES
Create a
.Pa /bin/sh
--- numactl.c 2017-11-13 16:18:36.134359000 -0800
+++ numactl.c.lm 2017-11-13 16:18:28.530953000 -0800
@@ -231,6 +231,7 @@
(void) set_numa_domain_cpuaffinity(cpu_domain,
CPU_WHICH_PID, -1);
+ putenv("NUMACTL=YES");
errno = 0;
execvp(*argv, argv);
err(errno == ENOENT ? 127 : 126, "%s", *argv);
_______________________________________________
https://lists.freebsd.org/mailman/listinfo/freebsd-arch

Conrad Meyer

2017-11-14 04:28:57 UTC

Permalink

Hi Larry,

What if instead, the benchmark checked its own NUMA parameters? If a
NUMA policy is not set, the benchmark can bail out with an
admonishment.

Best,
Conrad

Hi folks, some CDN people are dragging me out of retirement to work
on FreeBSD. Which I have to say is sort of fun since I started my
kernel hacking on SunOS 4.x which was a very very nice version of
BSD. But FreeBSD has mostly caught up, so it's pleasant.
I'm wacking LMbench to be numa aware and this patch would help me make
sure that when you are a numa machine you could insist that people
run the benchmark via numactl (imma gonna blog about numa, it sucks
unless you are numa aware).
I did some docs but I'm new to the FreeBSD man macros, would love
feedback on how to do that right.
--- numactl.1 2017-11-13 17:51:26.243473000 -0800
+++ numactl.1.lm 2017-11-13 17:51:20.494596000 -0800
@@ -107,6 +107,15 @@
.El
.Sh EXIT STATUS
.Ex -std
+.Sh ENVIRONMENT
+.Nm
+sets the environment variable
+\fINUMACTL=YES\fP
+before running
+.Ar cmd ...
+so that programs that know that they need to be run under
+.Nm
+can check.
.Sh EXAMPLES
Create a
.Pa /bin/sh
--- numactl.c 2017-11-13 16:18:36.134359000 -0800
+++ numactl.c.lm 2017-11-13 16:18:28.530953000 -0800
@@ -231,6 +231,7 @@
(void) set_numa_domain_cpuaffinity(cpu_domain,
CPU_WHICH_PID, -1);
+ putenv("NUMACTL=YES");
errno = 0;
execvp(*argv, argv);
err(errno == ENOENT ? 127 : 126, "%s", *argv);
_______________________________________________
https://lists.freebsd.org/mailman/listinfo/freebsd-arch

Larry McVoy

2017-11-14 14:06:54 UTC

Permalink

Hi Conrad,

Nice idea but pretty awkward compared to checking an env var.
I'd have to #ifdef up the code for numa (the benchmark in question
is portable to all the active Unix based systems and a lot of dead
ones).

Is it not a thing on FreeBSD to use the environment to pass state
like this?

--lm

Post by Conrad Meyer
Hi Larry,
What if instead, the benchmark checked its own NUMA parameters? If a
NUMA policy is not set, the benchmark can bail out with an
admonishment.
Best,
Conrad

--
---
Larry McVoy lm at mcvoy.com http://www.mcvoy.com/lm

Julian Elischer

2017-11-15 13:34:53 UTC

Permalink

Post by Larry McVoy
Hi Conrad,
Nice idea but pretty awkward compared to checking an env var.
I'd have to #ifdef up the code for numa (the benchmark in question
is portable to all the active Unix based systems and a lot of dead
ones).
Is it not a thing on FreeBSD to use the environment to pass state
like this?

it's not unheard of, but it's not common.
I can't think of other examples off the top of my head except for
sshd, sudo and other login type things.
there is no reason NOT to other than the fact that you are the only
user of the feature..
You could make it more useful by putting information more useful than
"YES" there..

It also is only useful if you use numactl to do the work.
Any other methods of setting Numa related settings would not have it.
.. Also environments get cleared in many situations.

It's workable solution for you. I'd go with it for now..

Post by Larry McVoy
--lm

Post by Conrad Meyer
Hi Larry,
What if instead, the benchmark checked its own NUMA parameters? If a
NUMA policy is not set, the benchmark can bail out with an
admonishment.
Best,
Conrad

Ravi Pokala

2017-11-14 04:35:02 UTC

Permalink

Hi Larry,

Might I suggest a small wording change? Three "that"s in the same sentence is a little much. :-)

+so that programs that know that they need to be run under
+.Nm
+can check.

+so programs which know that they need to be run under
+.Nm ,
+can check.

Also, I recommend installing textproc/igor, and using it to scan the manpage for common mistakes.

Thanks,

Ravi (rpokala@)

-----Original Message-----
From: <owner-freebsd-***@freebsd.org> on behalf of Larry McVoy <***@mcvoy.com>
Date: 2017-11-13, Monday at 18:01
To: <freebsd-***@freebsd.org>
Cc: <***@netflix.com>, <***@llnw.com>, <***@netflix.com>
Subject: small patch for numactl. Comments?

Hi folks, some CDN people are dragging me out of retirement to work
on FreeBSD. Which I have to say is sort of fun since I started my
kernel hacking on SunOS 4.x which was a very very nice version of
BSD. But FreeBSD has mostly caught up, so it's pleasant.

I'm wacking LMbench to be numa aware and this patch would help me make
sure that when you are a numa machine you could insist that people
run the benchmark via numactl (imma gonna blog about numa, it sucks
unless you are numa aware).

I did some docs but I'm new to the FreeBSD man macros, would love
feedback on how to do that right.

--- numactl.1 2017-11-13 17:51:26.243473000 -0800
+++ numactl.1.lm 2017-11-13 17:51:20.494596000 -0800
@@ -107,6 +107,15 @@
.El
.Sh EXIT STATUS
.Ex -std
+.Sh ENVIRONMENT
+.Nm
+sets the environment variable
+\fINUMACTL=YES\fP
+before running
+.Ar cmd ...
+so that programs that know that they need to be run under
+.Nm
+can check.
.Sh EXAMPLES
Create a
.Pa /bin/sh
--- numactl.c 2017-11-13 16:18:36.134359000 -0800
+++ numactl.c.lm 2017-11-13 16:18:28.530953000 -0800
@@ -231,6 +231,7 @@
(void) set_numa_domain_cpuaffinity(cpu_domain,
CPU_WHICH_PID, -1);

+ putenv("NUMACTL=YES");
errno = 0;
execvp(*argv, argv);
err(errno == ENOENT ? 127 : 126, "%s", *argv);
_______________________________________________
freebsd-***@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-***@freebsd.org"

Larry McVoy

2017-11-14 14:08:07 UTC

Permalink

Hokay, took the suggestions, new patch for you.

--- numactl.1 2017-11-13 17:51:26.243473000 -0800
+++ numactl.1.lm 2017-11-14 06:03:30.564340000 -0800
@@ -107,6 +107,15 @@
.El
.Sh EXIT STATUS
.Ex -std
+.Sh ENVIRONMENT
+.Nm
+sets the environment variable
+.Va NUMACTL=YES
+before running
+.Ar cmd ...
+so programs that know they need to be run under
+.Nm
+can check.
.Sh EXAMPLES
Create a
.Pa /bin/sh
--- numactl.c 2017-11-13 16:18:36.134359000 -0800
+++ numactl.c.lm 2017-11-13 16:18:28.530953000 -0800
@@ -231,6 +231,7 @@
(void) set_numa_domain_cpuaffinity(cpu_domain,
CPU_WHICH_PID, -1);

+ putenv("NUMACTL=YES");
errno = 0;
execvp(*argv, argv);
err(errno == ENOENT ? 127 : 126, "%s", *argv);

Larry McVoy

2017-11-14 14:42:13 UTC

Permalink

So is there some reason for the igor name? Why not doc-lint or something
that someone would have some chance of finding?

I like the program, especially this:

-e Verify that sentences start on a new line. mdoc(7) does not
enforce this, it is a style issue.

because that style means that if you version control your docs you get
much nicer diffs. I've got a little perl script that does that for me.

Post by Ravi Pokala
Also, I recommend installing textproc/igor, and using it to scan the manpage for common mistakes.

Ravi Pokala

2017-11-14 16:02:21 UTC

Permalink

<shrug> Probably because `igor' helps the mad scientists with doc commit-bits? :-)

-Ravi (rpokala@)

-----Original Message-----
From: <owner-freebsd-***@freebsd.org> on behalf of Larry McVoy <***@mcvoy.com>
Date: 2017-11-14, Tuesday at 06:42
To: Ravi Pokala <***@mac.com>
Cc: <***@netflix.com>, <***@llnw.com>, <***@netflix.com>, Larry McVoy <***@mcvoy.com>, <freebsd-***@freebsd.org>
Subject: Re: small patch for numactl. Comments?

So is there some reason for the igor name? Why not doc-lint or something
that someone would have some chance of finding?

I like the program, especially this:

-e Verify that sentences start on a new line. mdoc(7) does not
enforce this, it is a style issue.

because that style means that if you version control your docs you get
much nicer diffs. I've got a little perl script that does that for me.

Post by Ravi Pokala
Also, I recommend installing textproc/igor, and using it to scan the manpage for common mistakes.

_______________________________________________
freebsd-***@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-arch
To unsubscribe, send any mail to "freebsd-arch-***@freebsd.org"

Larry McVoy

2017-11-14 17:21:34 UTC

Permalink

I'm wacking LMbench to be numa aware and this patch would help me make
sure that when you are a numa machine you could insist that people
run the benchmark via numactl (imma gonna blog about numa, it sucks
unless you are numa aware).

Well, I think the right thing to do is to query the existing policy and
complain when it turns out nothing is set. Perhaps exit by default and
add a switch to proceed anyway.

As already stated, that means #ifdef-ing portable code. Not a fan of that.

I believe someone already approved env var approach anyway.

Scott Long via freebsd-arch

2017-11-14 18:07:18 UTC

Permalink

Post by Larry McVoy

Well, I think the right thing to do is to query the existing policy and
complain when it turns out nothing is set. Perhaps exit by default and
add a switch to proceed anyway.

As already stated, that means #ifdef-ing portable code. Not a fan of that.
I believe someone already approved env var approach anyway.

FWIW, communicating state via environment variables is not a common pattern in
the core FreeBSD userland tools. It’s a bit more common in the contributed tools.
That’s probably why people are wishy-washy about your proposal. That said,
there’s no architectural rule against what you’re proposing, it’s useful, and it’s not
invasive and needing long discussion.

Scott