[opensuse-factory] New package numad
I'd like to submit a new package called numad to factory. This is a system daemon that attempts to identify processes that should be bound to NUMA nodes to improve overall performance on machines with multiple nodes. In preparation for working on the next stage of in-kernel automatic NUMA balancing support I compared current in-kernel automatic NUMA balancing, numad and manual tuning. In some cases numad can out-perform the current in-kernel implementation although in other cases it will actually perform worse than running with defaults. The expected cases where it performs better and worse are documented in the RPM description and the manual page. https://build.opensuse.org/package/show?package=numad&project=home%3Amgorman What is less clear to me is what project I should submit this to. I set the RPM group to System/Daemons and the Base:System devel project looked like a reasonable fit. Any suggestions on a suitable alternative or on how the package can be improved before submissions are welcome. -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
El 10/06/13 09:44, Mel Gorman escribió:
I'd like to submit a new package called numad to factory.
This is a system daemon that attempts to identify processes that should be bound to NUMA nodes to improve overall performance on machines with multiple nodes. In preparation for working on the next stage of in-kernel automatic NUMA balancing support I compared current in-kernel automatic NUMA balancing, numad and manual tuning. In some cases numad can out-perform the current in-kernel implementation although in other cases it will actually perform worse than running with defaults. The expected cases where it performs better and worse are documented in the RPM description and the manual page.
https://build.opensuse.org/package/show?package=numad&project=home%3Amgorman
What is less clear to me is what project I should submit this to. I set the RPM group to System/Daemons and the Base:System devel project looked like a reasonable fit. Any suggestions on a suitable alternative or on how the package can be improved before submissions are welcome.
Just a few objections/observations, - please do not add new init scripts to factory. - the systemd service file : (comments inline [Unit] Description=numad - The NUMA daemon that manages application locality. After=syslog.target --> Not needed or used anymore, does nothing. [Service] Type=forking Can you make the service of type "simple" ? that is, a non-forking service ? the number of "forking" services implementing all the needed step correctly can be counted with one hand and there will be free fingers to hold a glass of wine. (read: all are racy at startup) If that's not possible , ensure you use the PidFile directive as well. ExecStop=/usr/sbin/numad -i 0 If the daemon reacts properly to SIGTERM please completely omit that line and let systemd to take care of the process. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Mon, Jun 10, 2013 at 04:24:42PM -0400, Cristian Rodr?guez wrote:
El 10/06/13 09:44, Mel Gorman escribió:
I'd like to submit a new package called numad to factory.
This is a system daemon that attempts to identify processes that should be bound to NUMA nodes to improve overall performance on machines with multiple nodes. In preparation for working on the next stage of in-kernel automatic NUMA balancing support I compared current in-kernel automatic NUMA balancing, numad and manual tuning. In some cases numad can out-perform the current in-kernel implementation although in other cases it will actually perform worse than running with defaults. The expected cases where it performs better and worse are documented in the RPM description and the manual page.
https://build.opensuse.org/package/show?package=numad&project=home%3Amgorman
What is less clear to me is what project I should submit this to. I set the RPM group to System/Daemons and the Base:System devel project looked like a reasonable fit. Any suggestions on a suitable alternative or on how the package can be improved before submissions are welcome.
Just a few objections/observations,
- please do not add new init scripts to factory.
Seems fair but I was aiming to create a package that would also be compatible with older versions of openSUSEi (12.2 in particular). Is the following diff the correct way of doing that? --- numad.spec (revision 3) +++ numad.spec (working copy) @@ -68,22 +68,24 @@ %makeinstall prefix=$RPM_BUILD_ROOT%{_prefix} install gzip $RPM_BUILD_ROOT%{_mandir}/man8/numad.8 %{__install} -D -m 644 numad.conf $RPM_BUILD_ROOT/etc/numad.conf +%if 0%{?has_systemd} +install -D -m 0644 numad.service %{buildroot}%{_unitdir}/numad.service +%else %{__install} -d $RPM_BUILD_ROOT%{_sysconfdir}/init.d %{__install} -m 744 numad.init $RPM_BUILD_ROOT%{_sysconfdir}/init.d/numad ln -sf ../../etc/init.d/numad $RPM_BUILD_ROOT/usr/sbin/rcnumad -%if 0%{?has_systemd} -install -D -m 0644 numad.service %{buildroot}%{_unitdir}/numad.service %endif %files %defattr(-,root,root) %{_sbindir}/numad -%{_sbindir}/rcnumad %{_mandir}/man8/numad.8.gz %config /etc/numad.conf -%config /etc/init.d/numad %if 0%{?has_systemd} %{_unitdir}/numad.service +%else +%{_sbindir}/rcnumad +%config /etc/init.d/numad %endif %pre
- the systemd service file : (comments inline
[Unit] Description=numad - The NUMA daemon that manages application locality.
After=syslog.target --> Not needed or used anymore, does nothing.
Removed.
[Service] Type=forking
Can you make the service of type "simple" ? that is, a non-forking service ?
Easily, I should have caught that. The only forking it does is related to becoming a daemon.
the number of "forking" services implementing all the needed step correctly can be counted with one hand and there will be free fingers to hold a glass of wine. (read: all are racy at startup)
If that's not possible , ensure you use the PidFile directive as well.
ExecStop=/usr/sbin/numad -i 0
If the daemon reacts properly to SIGTERM please completely omit that line and let systemd to take care of the process.
The daemon does not handle SIGTERM properly. If it's killed with TERM then it'll leave the pidfile behind and complain about it the next time it starts up. I was surprised by this because the upstream init script assumes that SIGTERM is handled properly even though the manual page states Setting a <max_interval> of zero will cause the daemon to exit. (This is the normal mechanism to terminate the daemon.) Thanks for the review. -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
В Tue, 11 Jun 2013 09:17:11 +0100 Mel Gorman <mgorman@suse.de> пишет:
On Mon, Jun 10, 2013 at 04:24:42PM -0400, Cristian Rodr?guez wrote:
El 10/06/13 09:44, Mel Gorman escribió:
I'd like to submit a new package called numad to factory.
This is a system daemon that attempts to identify processes that should be bound to NUMA nodes to improve overall performance on machines with multiple nodes. In preparation for working on the next stage of in-kernel automatic NUMA balancing support I compared current in-kernel automatic NUMA balancing, numad and manual tuning. In some cases numad can out-perform the current in-kernel implementation although in other cases it will actually perform worse than running with defaults. The expected cases where it performs better and worse are documented in the RPM description and the manual page.
https://build.opensuse.org/package/show?package=numad&project=home%3Amgorman
What is less clear to me is what project I should submit this to. I set the RPM group to System/Daemons and the Base:System devel project looked like a reasonable fit. Any suggestions on a suitable alternative or on how the package can be improved before submissions are welcome.
Just a few objections/observations,
- please do not add new init scripts to factory.
Seems fair but I was aiming to create a package that would also be compatible with older versions of openSUSEi (12.2 in particular). Is the following diff the correct way of doing that?
--- numad.spec (revision 3) +++ numad.spec (working copy) @@ -68,22 +68,24 @@ %makeinstall prefix=$RPM_BUILD_ROOT%{_prefix} install gzip $RPM_BUILD_ROOT%{_mandir}/man8/numad.8 %{__install} -D -m 644 numad.conf $RPM_BUILD_ROOT/etc/numad.conf +%if 0%{?has_systemd} +install -D -m 0644 numad.service %{buildroot}%{_unitdir}/numad.service +%else %{__install} -d $RPM_BUILD_ROOT%{_sysconfdir}/init.d %{__install} -m 744 numad.init $RPM_BUILD_ROOT%{_sysconfdir}/init.d/numad ln -sf ../../etc/init.d/numad $RPM_BUILD_ROOT/usr/sbin/rcnumad -%if 0%{?has_systemd} -install -D -m 0644 numad.service %{buildroot}%{_unitdir}/numad.service %endif
%files %defattr(-,root,root) %{_sbindir}/numad -%{_sbindir}/rcnumad %{_mandir}/man8/numad.8.gz %config /etc/numad.conf -%config /etc/init.d/numad %if 0%{?has_systemd} %{_unitdir}/numad.service +%else +%{_sbindir}/rcnumad +%config /etc/init.d/numad %endif
If you aim at 12.2 you should install both because it can run both with sysvinit and systemd. May be if 0%{suse_version} < 1230 %{_sbindir}/rcnumad %config /etc/init.d/numad %endif etc -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, Jun 11, 2013 at 06:52:13PM +0400, Andrey Borzenkov wrote:
?? Tue, 11 Jun 2013 09:17:11 +0100 Mel Gorman <mgorman@suse.de> ??????????:
On Mon, Jun 10, 2013 at 04:24:42PM -0400, Cristian Rodr?guez wrote:
El 10/06/13 09:44, Mel Gorman escribió:
I'd like to submit a new package called numad to factory.
<SNIP>
Just a few objections/observations,
- please do not add new init scripts to factory.
Seems fair but I was aiming to create a package that would also be compatible with older versions of openSUSEi (12.2 in particular). Is the following diff the correct way of doing that?
<SNIP>
If you aim at 12.2 you should install both because it can run both with sysvinit and systemd. May be
if 0%{suse_version} < 1230 %{_sbindir}/rcnumad %config /etc/init.d/numad %endif
Good point. How about this then? Index: numad.spec =================================================================== --- numad.spec (revision 3) +++ numad.spec (working copy) @@ -68,22 +68,26 @@ %makeinstall prefix=$RPM_BUILD_ROOT%{_prefix} install gzip $RPM_BUILD_ROOT%{_mandir}/man8/numad.8 %{__install} -D -m 644 numad.conf $RPM_BUILD_ROOT/etc/numad.conf +%if 0%{?has_systemd} +install -D -m 0644 numad.service %{buildroot}%{_unitdir}/numad.service +%endif +%if 0%{suse_version} < 1230 %{__install} -d $RPM_BUILD_ROOT%{_sysconfdir}/init.d %{__install} -m 744 numad.init $RPM_BUILD_ROOT%{_sysconfdir}/init.d/numad ln -sf ../../etc/init.d/numad $RPM_BUILD_ROOT/usr/sbin/rcnumad -%if 0%{?has_systemd} -install -D -m 0644 numad.service %{buildroot}%{_unitdir}/numad.service %endif %files %defattr(-,root,root) %{_sbindir}/numad -%{_sbindir}/rcnumad %{_mandir}/man8/numad.8.gz %config /etc/numad.conf -%config /etc/init.d/numad %if 0%{?has_systemd} %{_unitdir}/numad.service +%endif +%if 0%{suse_version} < 1230 +%{_sbindir}/rcnumad +%config /etc/init.d/numad %endif %pre -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Hello, Am Dienstag, 11. Juni 2013 schrieb Mel Gorman:
+%if 0%{suse_version} < 1230 %{__install} -d $RPM_BUILD_ROOT%{_sysconfdir}/init.d %{__install} -m 744 numad.init $RPM_BUILD_ROOT%{_sysconfdir}/init.d/numad ln -sf ../../etc/init.d/numad $RPM_BUILD_ROOT/usr/sbin/rcnumad
You can also create a rcnumad symlink on systemd-only releases. Just create a symlink rcnumad -> /usr/sbin/service ;-) Regards, Christian Boltz -- oh, wieder ein auto/pc-vergleich :-)
Ein Auto und die StVO sind wahrlich primitiv im Vergleich. das mag sein. aber wieviele menschen sterben jährlich durch computerunfälle? [gefunden auf http://www.pro-linux.de/news/2005/8509.html]
-- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Tue, Jun 11, 2013 at 10:04:09PM +0200, Christian Boltz wrote:
Hello,
Am Dienstag, 11. Juni 2013 schrieb Mel Gorman:
+%if 0%{suse_version} < 1230 %{__install} -d $RPM_BUILD_ROOT%{_sysconfdir}/init.d %{__install} -m 744 numad.init $RPM_BUILD_ROOT%{_sysconfdir}/init.d/numad ln -sf ../../etc/init.d/numad $RPM_BUILD_ROOT/usr/sbin/rcnumad
You can also create a rcnumad symlink on systemd-only releases. Just create a symlink rcnumad -> /usr/sbin/service ;-)
Very good point. That's done now. -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Mon, Jun 10, 2013 at 04:24:42PM -0400, Cristian Rodr?guez wrote:
El 10/06/13 09:44, Mel Gorman escribió:
I'd like to submit a new package called numad to factory.
This is a system daemon that attempts to identify processes that should be bound to NUMA nodes to improve overall performance on machines with multiple nodes. In preparation for working on the next stage of in-kernel automatic NUMA balancing support I compared current in-kernel automatic NUMA balancing, numad and manual tuning. In some cases numad can out-perform the current in-kernel implementation although in other cases it will actually perform worse than running with defaults. The expected cases where it performs better and worse are documented in the RPM description and the manual page.
https://build.opensuse.org/package/show?package=numad&project=home%3Amgorman
What is less clear to me is what project I should submit this to. I set the RPM group to System/Daemons and the Base:System devel project looked like a reasonable fit. Any suggestions on a suitable alternative or on how the package can be improved before submissions are welcome.
Just a few objections/observations,
- please do not add new init scripts to factory. - the systemd service file : (comments inline
[Unit] Description=numad - The NUMA daemon that manages application locality.
After=syslog.target --> Not needed or used anymore, does nothing.
[Service] Type=forking
Can you make the service of type "simple" ? that is, a non-forking service ?
Actually I got this wrong. Even though the daemon does not receive socket connects and forks to handle each connection, numad does daemonize itself. If the service is of type simple then systemd does not start the process properly. I have two choices here. I can make it forking and specify the PidFile= or I can make it oneshot and specify RemainAfterExit=true. Which is preferred? -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
В Wed, 12 Jun 2013 14:45:34 +0100 Mel Gorman <mgorman@suse.de> пишет:
On Mon, Jun 10, 2013 at 04:24:42PM -0400, Cristian Rodr?guez wrote:
El 10/06/13 09:44, Mel Gorman escribió:
I'd like to submit a new package called numad to factory.
This is a system daemon that attempts to identify processes that should be bound to NUMA nodes to improve overall performance on machines with multiple nodes. In preparation for working on the next stage of in-kernel automatic NUMA balancing support I compared current in-kernel automatic NUMA balancing, numad and manual tuning. In some cases numad can out-perform the current in-kernel implementation although in other cases it will actually perform worse than running with defaults. The expected cases where it performs better and worse are documented in the RPM description and the manual page.
https://build.opensuse.org/package/show?package=numad&project=home%3Amgorman
What is less clear to me is what project I should submit this to. I set the RPM group to System/Daemons and the Base:System devel project looked like a reasonable fit. Any suggestions on a suitable alternative or on how the package can be improved before submissions are welcome.
Just a few objections/observations,
- please do not add new init scripts to factory. - the systemd service file : (comments inline
[Unit] Description=numad - The NUMA daemon that manages application locality.
After=syslog.target --> Not needed or used anymore, does nothing.
[Service] Type=forking
Can you make the service of type "simple" ? that is, a non-forking service ?
Actually I got this wrong. Even though the daemon does not receive socket connects and forks to handle each connection, numad does daemonize itself. If the service is of type simple then systemd does not start the process properly. I have two choices here. I can make it forking and specify the PidFile= or I can make it oneshot and specify RemainAfterExit=true. Which is preferred?
Of course PidFile is preferred in this case than lying about service state. This a at least will properly track daemon status. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, Jun 12, 2013 at 05:50:43PM +0400, Andrey Borzenkov wrote:
<SNIP>
- please do not add new init scripts to factory. - the systemd service file : (comments inline
[Unit] Description=numad - The NUMA daemon that manages application locality.
After=syslog.target --> Not needed or used anymore, does nothing.
[Service] Type=forking
Can you make the service of type "simple" ? that is, a non-forking service ?
Actually I got this wrong. Even though the daemon does not receive socket connects and forks to handle each connection, numad does daemonize itself. If the service is of type simple then systemd does not start the process properly. I have two choices here. I can make it forking and specify the PidFile= or I can make it oneshot and specify RemainAfterExit=true. Which is preferred?
Of course PidFile is preferred in this case than lying about service state. This a at least will properly track daemon status.
Ok done and preliminary testing looks ok. The package should be up to date with all feedback. Is there anything else that should be fixed up? Any suggestions on what devel project to submit it to before pushing to factory? Thanks all for the review. -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wednesday 2013-06-12 16:21, Mel Gorman wrote:
Of course PidFile is preferred in this case than lying about service state. This a at least will properly track daemon status.
Ok done and preliminary testing looks ok. The package should be up to date with all feedback. Is there anything else that should be fixed up? Any suggestions on what devel project to submit it to before pushing to factory?
Well, making the daemonization optional, perhaps through the use of a flag like smbd -F would allow you to use the simple ForkingType. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, Jun 12, 2013 at 04:33:37PM +0200, Jan Engelhardt wrote:
On Wednesday 2013-06-12 16:21, Mel Gorman wrote:
Of course PidFile is preferred in this case than lying about service state. This a at least will properly track daemon status.
Ok done and preliminary testing looks ok. The package should be up to date with all feedback. Is there anything else that should be fixed up? Any suggestions on what devel project to submit it to before pushing to factory?
Well, making the daemonization optional, perhaps through the use of a flag like smbd -F would allow you to use the simple ForkingType.
Unlike the other patches, that would be a fairly sizable deviation from upstream for opensuse. It would be a fairly straight-forward patch but is the forking type so bad that such a move is justified? -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, Jun 12, 2013 at 03:43:38PM +0100, Mel Gorman wrote:
On Wed, Jun 12, 2013 at 04:33:37PM +0200, Jan Engelhardt wrote:
On Wednesday 2013-06-12 16:21, Mel Gorman wrote:
Of course PidFile is preferred in this case than lying about service state. This a at least will properly track daemon status.
Ok done and preliminary testing looks ok. The package should be up to date with all feedback. Is there anything else that should be fixed up? Any suggestions on what devel project to submit it to before pushing to factory?
Well, making the daemonization optional, perhaps through the use of a flag like smbd -F would allow you to use the simple ForkingType.
Unlike the other patches, that would be a fairly sizable deviation from upstream for opensuse. It would be a fairly straight-forward patch but is the forking type so bad that such a move is justified?
This is what the numad patch ends up looking like complete with update to service file. I've no problem carrying it as part of the package but if I would appreciate being slapped with a cluebat as to why the systemd forking type is so bad. I guess the obvious explanation is that if there are two services A and B where A is forking and B depends on A then there is a race where B can be up and running before A is ready to accept requests. This potentially leads to errors during startup that are very hard to trigger. Such a scenario should not be a problem with numad but maybe there are other cases. ---8<--- Give an option of running in foreground mode systemd forking service type smells of something unfortunate apparently and a desire was expressed to have it run as a simple service with a foreground mode option. This patch implements a -F flag and uses it with systemd. Signed-off-by: Mel Gorman <mgorman@suse.de> --- numad.8 | 5 ++++- numad.c | 30 ++++++++++++++++++++---------- numad.service | 5 ++--- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/numad.8 b/numad.8 index 2c46f55..73a0bc9 100644 --- a/numad.8 +++ b/numad.8 @@ -5,7 +5,7 @@ numad \- A user\-level daemon that provides placement advice and process management for efficient use of CPUs and memory on systems with NUMA topology. .SH "SYNTAX" .LP -numad [\fI\-dhvV\fP] +numad [\fI\-dFhvV\fP] .br .LP numad [\fI\-D non-standard-cgroup-mount-point\fP] @@ -61,6 +61,9 @@ Debug output in log, sets the log level to LOG_DEBUG. Same effect as \fI\-l 7\f \fB\-D\fR <\fInon-standard-cgroup-mount-point\fP> This option can be used to communicate a non-standard cgroup mount point to numad. This is not normally necessary. +.TP +\fB\-F\fR +Run in foreground mode, do not daemonize\fP. .TP \fB\-h\fR Display usage help information and then exit. diff --git a/numad.c b/numad.c index d08b622..f6581c0 100644 --- a/numad.c +++ b/numad.c @@ -680,6 +680,7 @@ void print_usage_and_exit(char *prog_name) { fprintf(stderr, "Usage: %s <options> ...\n", prog_name); fprintf(stderr, "-d for debug logging (same effect as '-l 7')\n"); fprintf(stderr, "-D <CGROUP_MOUNT_POINT> to specify cgroup mount point\n"); + fprintf(stderr, "-F to run in the foreground\n"); fprintf(stderr, "-h to print this usage info\n"); fprintf(stderr, "-i [<MIN>:]<MAX> to specify interval seconds\n"); fprintf(stderr, "-K 1 to keep interleaved memory spread across nodes\n"); @@ -2145,8 +2146,9 @@ int main(int argc, char *argv[]) { int v_flag = 0; int w_flag = 0; int x_flag = 0; + int F_flag = 0; long list_pid = 0; - while ((opt = getopt(argc, argv, "dD:hi:K:l:p:r:S:u:vVw:x:")) != -1) { + while ((opt = getopt(argc, argv, "dFD:hi:K:l:p:r:S:u:vVw:x:")) != -1) { switch (opt) { case 'd': d_flag = 1; @@ -2155,6 +2157,9 @@ int main(int argc, char *argv[]) { case 'D': cpuset_dir_list[0] = strdup(optarg); break; + case 'F': + F_flag = 1; + break; case 'h': print_usage_and_exit(argv[0]); break; @@ -2278,15 +2283,20 @@ int main(int argc, char *argv[]) { } else if (max_interval > 0) { // Start the numad daemon... check_prereqs(argv[0]); - // Daemonize self... - daemon_pid = fork(); - if (daemon_pid < 0) { numad_log(LOG_CRIT, "fork() failed\n"); exit(EXIT_FAILURE); } - // Parent process now exits - if (daemon_pid > 0) { exit(EXIT_SUCCESS); } - // Child process continues... - umask(S_IWGRP | S_IWOTH); // Reset the file mode - int sid = setsid(); // Start a new session - if (sid < 0) { numad_log(LOG_CRIT, "setsid() failed\n"); exit(EXIT_FAILURE); } + + // Daemonize self if requested + if (F_flag == 0) { + daemon_pid = fork(); + if (daemon_pid < 0) { numad_log(LOG_CRIT, "fork() failed\n"); exit(EXIT_FAILURE); } + // Parent process now exits + if (daemon_pid > 0) { exit(EXIT_SUCCESS); } + + // Child process continues... + umask(S_IWGRP | S_IWOTH); // Reset the file mode + int sid = setsid(); // Start a new session + if (sid < 0) { numad_log(LOG_CRIT, "setsid() failed\n"); exit(EXIT_FAILURE); } + } + if ((chdir("/")) < 0) { numad_log(LOG_CRIT, "chdir() failed"); exit(EXIT_FAILURE); } daemon_pid = register_numad_pid(); if (daemon_pid != getpid()) { diff --git a/numad.service b/numad.service index d0c8c5b..0223ca9 100644 --- a/numad.service +++ b/numad.service @@ -2,10 +2,9 @@ Description=numad - The NUMA daemon that manages application locality. [Service] -Type=forking -PIDFile=/var/run/numad.pid +Type=simple EnvironmentFile=/etc/numad.conf -ExecStart=/usr/sbin/numad -i $INTERVAL +ExecStart=/usr/sbin/numad -i $INTERVAL -F ExecStop=/usr/sbin/numad -i 0 [Install] -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
В Wed, 12 Jun 2013 16:02:51 +0100 Mel Gorman <mgorman@suse.de> пишет:
On Wed, Jun 12, 2013 at 03:43:38PM +0100, Mel Gorman wrote:
On Wed, Jun 12, 2013 at 04:33:37PM +0200, Jan Engelhardt wrote:
On Wednesday 2013-06-12 16:21, Mel Gorman wrote:
Of course PidFile is preferred in this case than lying about service state. This a at least will properly track daemon status.
Ok done and preliminary testing looks ok. The package should be up to date with all feedback. Is there anything else that should be fixed up? Any suggestions on what devel project to submit it to before pushing to factory?
Well, making the daemonization optional, perhaps through the use of a flag like smbd -F would allow you to use the simple ForkingType.
Unlike the other patches, that would be a fairly sizable deviation from upstream for opensuse. It would be a fairly straight-forward patch but is the forking type so bad that such a move is justified?
This is what the numad patch ends up looking like complete with update to service file. I've no problem carrying it as part of the package but if I would appreciate being slapped with a cluebat as to why the systemd forking type is so bad.
Both simple and forking are bad actually :) Both have the same problem - they assume service is available as soon as program is started/forked. Which is often wrong - some programs may need long time to start up. Best is socket activation, but it requires comprehensive support from application. Second best is notify. This is technically trivial to add, it is a single call to inform systemd that service has finished initialization and is ready. simple is OK for "leaf" services that do not provide services themselves. But I think you mentioned that your program accepts requests. Forking is OK when done right, but I have seen programs that fork multiple times and this may confuse systemd as to which one is the main process. That is where PID file comes handy.
I guess the obvious explanation is that if there are two services A and B where A is forking and B depends on A then there is a race where B can be up and running before A is ready to accept requests. This potentially leads to errors during startup that are very hard to trigger. Such a scenario should not be a problem with numad but maybe there are other cases.
If you are sure numad never races with other programs that require it - just use forking, it has no real advantage over simple and simplifies maintenance. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Le mercredi 12 juin 2013 à 21:04 +0400, Andrey Borzenkov a écrit :
В Wed, 12 Jun 2013 16:02:51 +0100 Mel Gorman <mgorman@suse.de> пишет:
On Wed, Jun 12, 2013 at 03:43:38PM +0100, Mel Gorman wrote:
On Wed, Jun 12, 2013 at 04:33:37PM +0200, Jan Engelhardt wrote:
On Wednesday 2013-06-12 16:21, Mel Gorman wrote:
>
Of course PidFile is preferred in this case than lying about service state. This a at least will properly track daemon status.
Ok done and preliminary testing looks ok. The package should be up to date with all feedback. Is there anything else that should be fixed up? Any suggestions on what devel project to submit it to before pushing to factory?
Well, making the daemonization optional, perhaps through the use of a flag like smbd -F would allow you to use the simple ForkingType.
Unlike the other patches, that would be a fairly sizable deviation from upstream for opensuse. It would be a fairly straight-forward patch but is the forking type so bad that such a move is justified?
This is what the numad patch ends up looking like complete with update to service file. I've no problem carrying it as part of the package but if I would appreciate being slapped with a cluebat as to why the systemd forking type is so bad.
Both simple and forking are bad actually :) Both have the same problem - they assume service is available as soon as program is started/forked. Which is often wrong - some programs may need long time to start up.
Not exactly : forking + PID is considered OK, since systemd considers the service is "available" only when PID file is being created. Of course, it implies the daemon is creating PID file at the right time and not too early. -- Frederic Crozat <fcrozat@suse.com> SUSE -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, Jun 12, 2013 at 09:04:24PM +0400, Andrey Borzenkov wrote:
? Wed, 12 Jun 2013 16:02:51 +0100 Mel Gorman <mgorman@suse.de> ?????:
On Wed, Jun 12, 2013 at 03:43:38PM +0100, Mel Gorman wrote:
On Wed, Jun 12, 2013 at 04:33:37PM +0200, Jan Engelhardt wrote:
On Wednesday 2013-06-12 16:21, Mel Gorman wrote:
>
Of course PidFile is preferred in this case than lying about service state. This a at least will properly track daemon status.
Ok done and preliminary testing looks ok. The package should be up to date with all feedback. Is there anything else that should be fixed up? Any suggestions on what devel project to submit it to before pushing to factory?
Well, making the daemonization optional, perhaps through the use of a flag like smbd -F would allow you to use the simple ForkingType.
Unlike the other patches, that would be a fairly sizable deviation from upstream for opensuse. It would be a fairly straight-forward patch but is the forking type so bad that such a move is justified?
This is what the numad patch ends up looking like complete with update to service file. I've no problem carrying it as part of the package but if I would appreciate being slapped with a cluebat as to why the systemd forking type is so bad.
Both simple and forking are bad actually :) Both have the same problem - they assume service is available as soon as program is started/forked. Which is often wrong - some programs may need long time to start up.
Best is socket activation, but it requires comprehensive support from application. Second best is notify. This is technically trivial to add, it is a single call to inform systemd that service has finished initialization and is ready.
simple is OK for "leaf" services that do not provide services themselves. But I think you mentioned that your program accepts requests.
Not as such. It monitors the systems and shoves processes into cpusets to improve memory locality. It receives messages but only from an invocation of numad to alter settings at runtime. I'm not anticipating any weird races from this.
Forking is OK when done right, but I have seen programs that fork multiple times and this may confuse systemd as to which one is the main process. That is where PID file comes handy.
Ok.
I guess the obvious explanation is that if there are two services A and B where A is forking and B depends on A then there is a race where B can be up and running before A is ready to accept requests. This potentially leads to errors during startup that are very hard to trigger. Such a scenario should not be a problem with numad but maybe there are other cases.
If you are sure numad never races with other programs that require it - just use forking, it has no real advantage over simple and simplifies maintenance.
From a maintenance point of view it's roughly equal difficulty to me. I'll leave it as a simple service running in foreground for now simply because there is no real advantage between this and running as forking for such a simple daemon.
Thanks. -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wednesday 2013-06-12 16:43, Mel Gorman wrote:
Well, making the daemonization optional, perhaps through the use of a flag like smbd -F would allow you to use the simple ForkingType.
Unlike the other patches, that would be a fairly sizable deviation from upstream for opensuse. It would be a fairly straight-forward patch but is the forking type so bad that such a move is justified?
I had assumed you were upstream, apparently that is not the case. Yes, it is "bad" - upstream should fix it ;-) Redundant (even the venerable old startproc did the daemonization for the program), error-prone, archaic, etc. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, Jun 12, 2013 at 12:06 PM, Jan Engelhardt <jengelh@inai.de> wrote:
On Wednesday 2013-06-12 16:43, Mel Gorman wrote:
Well, making the daemonization optional, perhaps through the use of a flag like smbd -F would allow you to use the simple ForkingType.
Unlike the other patches, that would be a fairly sizable deviation from upstream for opensuse. It would be a fairly straight-forward patch but is the forking type so bad that such a move is justified?
I had assumed you were upstream, apparently that is not the case.
Yes, it is "bad" - upstream should fix it ;-) Redundant (even the venerable old startproc did the daemonization for the program), error-prone, archaic, etc.
Yeah, daemonization is hard to get right (I should know, I wrote my share of daemons), and relying on some common system tool to do it right, be it startproc or systemd, is a better way. And, in any case, a foreground mode is very useful for debugging and in general development. I'm surprised numad doesn't have on already. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, Jun 12, 2013 at 05:06:31PM +0200, Jan Engelhardt wrote:
On Wednesday 2013-06-12 16:43, Mel Gorman wrote:
Well, making the daemonization optional, perhaps through the use of a flag like smbd -F would allow you to use the simple ForkingType.
Unlike the other patches, that would be a fairly sizable deviation from upstream for opensuse. It would be a fairly straight-forward patch but is the forking type so bad that such a move is justified?
I had assumed you were upstream, apparently that is not the case.
No, I'm not a maintainer of upstream in this case.
Yes, it is "bad" - upstream should fix it ;-)
Obviously I can punt patches upstream and this one would be a candidate but I'm not sure what the reception will be. The upstream changelogs are monolithic patches with no documentation and very sporadic. If they take it, great. If not, I can carry it as numad is a very straight-forward program and I'm not expecting it to be much of a maintenance headache.
Redundant (even the venerable old startproc did the daemonization for the program), error-prone, archaic, etc.
Indeed. I've no idea what the original motivation was. Jollies maybe. I've updated the package now with the new patch and thanks for the review and pointing out the option. As before, suggestions on what devel project to submit this to before pushing to factory are welcome. -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wednesday 2013-06-12 17:24, Mel Gorman wrote:
The upstream changelogs are monolithic patches with no documentation and very sporadic.
The only hope is that it will improve. (Redhat can do better, rrrright?) If necessary, through some future fork or alternate solution (like irqbalance -> irqd). Or it just dies out.
Indeed. I've no idea what the original motivation was. Jollies maybe. I've updated the package now with the new patch and thanks for the review and pointing out the option. As before, suggestions on what devel project to submit this to before pushing to factory are welcome.
"hardware"/"science" :p Ore more serisouly, utilities. Base:System already feels like a dumping ground but is not wrong either. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wed, Jun 12, 2013 at 06:28:31PM +0200, Jan Engelhardt wrote:
On Wednesday 2013-06-12 17:24, Mel Gorman wrote:
The upstream changelogs are monolithic patches with no documentation and very sporadic.
The only hope is that it will improve. (Redhat can do better, rrrright?)
I'm not touching that with a barge pole :) . On a semi-serious note I think numad was a stop-gap while in-kernel support was being sorted out. The program is sufficiently small that it can be understood without changelog messages anyway.
If necessary, through some future fork or alternate solution (like irqbalance -> irqd). Or it just dies out.
It's a possibility!
Indeed. I've no idea what the original motivation was. Jollies maybe. I've updated the package now with the new patch and thanks for the review and pointing out the option. As before, suggestions on what devel project to submit this to before pushing to factory are welcome.
"hardware"/"science" :p
Ore more serisouly, utilities. Base:System already feels like a dumping ground but is not wrong either.
numad is a daemon and the utilities project is described as "all the little shell tools that dont fit in other projects". It's certainly not a small shell tool so I do not think it is a good fit. You were joking but it looks like "hardware" would be a better fit than Base:System. It is concerned with the physical topology of the machine. Critically, cpuset is already in that project and in a sense these are sortof related in that numad depends on cpusets. -- Mel Gorman SUSE Labs -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
El 12/06/13 11:24, Mel Gorman escribió:
Redundant (even the venerable old startproc did the daemonization for the program), error-prone, archaic, etc.
Indeed. I've no idea what the original motivation was.
The racy daemonization issue is one of the many things that due to systemd aggressive paralellization and speed has been brought to the surface after living under the rug for decades, unfortunately there is tendency to shoot the messenger ;-) The correct steps for daemonization are documented in daemon(7) either for traditional forking daemons in the section "SysV Daemons" (hint: I have reviewed hundreds of packages, none implements the 15 steps correctly :-( saaad state of reality..) or for native systemd services in the section "New-Style Daemons". -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Wednesday 2013-06-12 21:50, Cristian Rodríguez wrote:
The correct steps for daemonization are documented in daemon(7) either for traditional forking daemons in the section "SysV Daemons" (hint: I have reviewed hundreds of packages, none implements the 15 steps correctly :-(
With *15* steps, it is no wonder that it is not done according to daemon(7). That, and of course not knowing that it takes that much steps to begin with. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On 6/12/2013 3:50 PM, Cristian Rodríguez wrote:
El 12/06/13 11:24, Mel Gorman escribió:
Redundant (even the venerable old startproc did the daemonization for the program), error-prone, archaic, etc.
Indeed. I've no idea what the original motivation was.
The racy daemonization issue is one of the many things that due to systemd aggressive paralellization and speed has been brought to the surface after living under the rug for decades, unfortunately there is tendency to shoot the messenger ;-)
The correct steps for daemonization are documented in daemon(7) either for traditional forking daemons in the section "SysV Daemons" (hint: I have reviewed hundreds of packages, none implements the 15 steps correctly :-( saaad state of reality..) or for native systemd services in the section "New-Style Daemons".
At some point long before "hundreds of packages" don't do what you consider correct, maybe it's time to say "maybe it's me?". -- bkw -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
El 12/06/13 21:55, Brian K. White escribió:
At some point long before "hundreds of packages" don't do what you consider correct, maybe it's time to say "maybe it's me?".
In this case, it is not ;). it is most likely the result of poor documentation and the lack of suitable, hard-to-misuse APIs across operating systems. Of course the init system has to deal with real life software so for "forking" services there are a few checks for buggy daemons in place. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On 6/12/2013 10:44 PM, Cristian Rodríguez wrote:
El 12/06/13 21:55, Brian K. White escribió:
At some point long before "hundreds of packages" don't do what you consider correct, maybe it's time to say "maybe it's me?".
In this case, it is not ;). it is most likely the result of poor documentation and the lack of suitable, hard-to-misuse APIs across operating systems.
Of course the init system has to deal with real life software so for "forking" services there are a few checks for buggy daemons in place.
What I was getting at was, those are the software that already exists, therefore, the useful system is prepared to deal with them gracefully, working _at least_ as well as init did, instead of requiring them all to be other than whatever they already are. -- bkw -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
"Brian K. White" <brian@aljex.com> writes:
At some point long before "hundreds of packages" don't do what you consider correct, maybe it's time to say "maybe it's me?".
With sysvinit those races where just ignored or hacked around ("let's sleep a little bit"). Andreas. -- Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different." -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
Am 13.06.2013 03:55, schrieb Brian K. White:
At some point long before "hundreds of packages" don't do what you consider correct, maybe it's time to say "maybe it's me?".
Of course it's you. You are not obeying systemds law which states that only they know how daemon() has to work. There is also daemon(3), and if this one is not doing what's necessary, maybe it is simply a bug in glibc. But of course, the glibc guys do not really know how to daemon(), thus the manpage in section 7 telling them what to do is necessary... daemon(7) might be relevant to systemd, but not necessarily outside and in the real world. -- Stefan Seyfried "If your lighter runs out of fluid or flint and stops making fire, and you can't be bothered to figure out about lighter fluid or flint, that is not Zippo's fault." -- bkw -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
El 13/06/13 04:28, Stefan Seyfried escribió:
Am 13.06.2013 03:55, schrieb Brian K. White:
At some point long before "hundreds of packages" don't do what you consider correct, maybe it's time to say "maybe it's me?".
Of course it's you.
NO it is not, you are shooting the messenger !
There is also daemon(3), and if this one is not doing what's necessary, maybe it is simply a bug in glibc.
glibc 's daemon(3) follows BSD semantics, you cannot just fix daemon(3) without also importing BSD 's pidfile API and modifing the function in a way that is not behavior-compatible (effectively requiring a new daemon2() or linux_daemon() )
But of course, the glibc guys do not really know how to daemon(), thus the manpage in section 7 telling them what to do is necessary...
No one has ever said that, but that it does not implement all the needed steps to daemonize a complex piece of software in a race-free fashion. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Thu, Jun 13, 2013 at 1:58 PM, Cristian Rodríguez <crrodriguez@opensuse.org> wrote:
El 13/06/13 04:28, Stefan Seyfried escribió:
Am 13.06.2013 03:55, schrieb Brian K. White:
At some point long before "hundreds of packages" don't do what you consider correct, maybe it's time to say "maybe it's me?".
Of course it's you.
NO it is not, you are shooting the messenger !
There is also daemon(3), and if this one is not doing what's necessary, maybe it is simply a bug in glibc.
glibc 's daemon(3) follows BSD semantics, you cannot just fix daemon(3) without also importing BSD 's pidfile API and modifing the function in a way that is not behavior-compatible (effectively requiring a new daemon2() or linux_daemon() )
But of course, the glibc guys do not really know how to daemon(), thus the manpage in section 7 telling them what to do is necessary...
No one has ever said that, but that it does not implement all the needed steps to daemonize a complex piece of software in a race-free fashion.
This is so moot. Of all those 15 points in daemon(7), I've faced each and every one of them. They're necessary at some point or another. Maybe not all daemons care for all issues, but it's no exaggeration that they are all points to ponder when writing a daemon. It's not a systemd thing at all. -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
On Thursday 2013-06-13 18:58, Cristian Rodríguez wrote:
El 13/06/13 04:28, Stefan Seyfried escribió:
Am 13.06.2013 03:55, schrieb Brian K. White:
At some point long before "hundreds of packages" don't do what you consider correct, maybe it's time to say "maybe it's me?".
Of course it's you.
NO it is not, you are shooting the messenger !
"""When the messenger comes to appropriate your profits ... kill the messenger.""" ;) -- To unsubscribe, e-mail: opensuse-factory+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-factory+owner@opensuse.org
participants (10)
-
Andreas Schwab -
Andrey Borzenkov -
Brian K. White -
Christian Boltz -
Claudio Freire -
Cristian Rodríguez -
Frederic Crozat -
Jan Engelhardt -
Mel Gorman -
Stefan Seyfried