Broke nice range for RLIMIT NICE

PATCH New Michael Kerrisk 7 participants 15 messages Jul 28, 2005 – Aug 15, 2005

Share & tools

Michael Kerrisk

author

2005-07-28 15:06 UTC #0 lore raw

Hello Ingo,

I'm guessing that it was you that added the RLIMIT_NICE resource limit in 2.6.12. (A passing note to all kernel developers: when making changes that affect userland-kernel interfaces, please send me a man-pages patch, or at least a notification of the change, so that some information makes its way into the manual pages).

I started documenting RLIMIT_NICE and then noticed an inconsistency between the use of this limit and the nice value as manipulated by [sg]etpriority().

This is the documentation I've drafted for RLIMIT_NICE in getrlimit.2:

RLIMIT_NICE(since kernel 2.6.12)
Specifies a ceiling to which the process nice
value can be raised using setpriority(2) or
nice(2). The actual ceiling for the nice value is calculated as 19 - rlim_cur.
^^^^^^^^^^^^^

And recently I've redrafted the discussion of the nice value in getpriority.2 and it now reads:

Since kernel 1.3.43 Linux has the range -20..19. Within the kernel, nice values are actually repre- sented using the corresponding range 40..1 (since negative numbers are error codes) and these are the values employed by the setpriority and getpriority
system calls. The glibc wrapper functions for
these system calls handle the translations between the user-land and kernel representations of the
nice value according to the formula
user_nice = 20 - kernel_nice. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

In other words, there is an off-by-one mismatch between these two interfaces: RLIMIT_NICE is expecting to deal with values in the range 39..0, while [gs]etpriority() works with the range 40..1.

I suppose that glibc could paper over the cracks here in a wrapper for getrlimit(), but it seems more sensible to make RLIMIT_NICE consistent with [gs]etpriority() -- i.e., change the RLIMIT_NICE interface in 2.6.13 before it sees wide use in userland. What do you think?

Cheers,

Michael

Signature · 6 lines

Michael Kerrisk
maintainer of Linux man pages Sections 2, 3, 4, 5, and 7 

Want to help with man page maintenance?  Grab the latest
tarball at ftp://ftp.win.tue.nl/pub/linux-local/manpages/
and grep the source files for 'FIXME'.

Andrew Morton

2005-07-28 17:45 UTC #1 lore raw

Quoted 6 lines · Michael Kerrisk

(A passing note to all kernel developers: when
making changes that affect userland-kernel interfaces, please send me a man-pages patch, or at least a notification of the change, so that some information makes its way into the manual pages).

Could I suggest that you send in a MAINTANERS record? That might help a bit.

Michael Kerrisk

author

2005-07-29 11:33 UTC #2 lore raw

PATCH 1/1 MAINTAINERS record -- MAN-PAGES

Hi Andrew,

[Was: Re: Broke nice range for RLIMIT NICE]

Quoted 8 lines

Quoted 5 lines
(A passing note to all kernel developers: when
making changes that affect userland-kernel interfaces, please send me a man-pages patch, or at least a notification of the change, so that some information makes its way into the manual pages).
Could I suggest that you send in a MAINTANERS record? That might help a bit.

Thanks for the idea. Hopefully the following (against the 2.6.12 MAINTAINERS file) suffices...

Cheers,

Michael

MAINTAINERS.new 2005-07-29 13:34:46.000000000 +0200 renamed from MAINTAINERS 2005-06-20 14:37:59.000000000 +0200 +6 -0

@@ -1478,6 +1478,12 @@
 M:     [email protected]
 S:     Odd Fixes
 
+MAN-PAGES: MANUAL PAGES FOR LINUX -- Sections 2, 3, 4, 5, and 7
+P: Michael Kerrisk
+M: [email protected]
+W: ftp://ftp.kernel.org/pub/linux/docs/manpages
+S: Maintained
+
 MARVELL MV64340 ETHERNET DRIVER
 P:     Manish Lachwani
 M:     [email protected]

Signature · 6 lines

Michael Kerrisk
maintainer of Linux man pages Sections 2, 3, 4, 5, and 7 

Want to help with man page maintenance?  Grab the latest
tarball at ftp://ftp.win.tue.nl/pub/linux-local/manpages/
and grep the source files for 'FIXME'.

Matt Mackall

2005-07-29 06:13 UTC #3 lore raw

In reply to Michael Kerrisk

Quoted 4 lines · Michael Kerrisk

Hello Ingo,
I'm guessing that it was you that added the RLIMIT_NICE resource limit in 2.6.12.

The original patch was from Chris Wright, but I did most of the cheerleading for it.

Quoted 5 lines

(A passing note to all kernel developers: when making changes that affect userland-kernel interfaces, please send me a man-pages patch, or at least a notification of the change, so that some information makes its way into the manual pages).

You might want to make an effort to make yourself more visible around here. Most of us have no idea that anyone's actually trying to maintain the manpages or who that might be.

Quoted 34 lines

I started documenting RLIMIT_NICE and then noticed an inconsistency between the use of this limit and the nice value as manipulated by [sg]etpriority().
This is the documentation I've drafted for RLIMIT_NICE in getrlimit.2:
RLIMIT_NICE(since kernel 2.6.12)
Specifies a ceiling to which the process nice
value can be raised using setpriority(2) or
nice(2). The actual ceiling for the nice value is calculated as 19 - rlim_cur.
^^^^^^^^^^^^^
And recently I've redrafted the discussion of the nice value in getpriority.2 and it now reads:
Since kernel 1.3.43 Linux has the range -20..19. Within the kernel, nice values are actually repre- sented using the corresponding range 40..1 (since negative numbers are error codes) and these are the values employed by the setpriority and getpriority
system calls. The glibc wrapper functions for
these system calls handle the translations between the user-land and kernel representations of the
nice value according to the formula
user_nice = 20 - kernel_nice. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
In other words, there is an off-by-one mismatch between these two interfaces: RLIMIT_NICE is expecting to deal with values in the range 39..0, while [gs]etpriority() works with the range 40..1.

Quoted 5 lines

I suppose that glibc could paper over the cracks here in a wrapper for getrlimit(), but it seems more sensible to make RLIMIT_NICE consistent with [gs]etpriority() -- i.e., change the RLIMIT_NICE interface in 2.6.13 before it sees wide use in userland. What do you think?

Well, it's easy enough to do, but some thought has to be given to the corner cases. Specifically, does this do the right thing when the rlimit is set to zero? I think it does, as the nice range is nicely bound here:

        nice = PRIO_TO_NICE(current->static_prio) + increment;
        if (nice < -20)
                nice = -20;
        if (nice > 19)
                nice = 19;

        if (increment < 0 && !can_nice(current, nice))
                return -EPERM;

And we allow task to do negative increment. Chris?

The other downside is, this obviously changes any existing configs actually using this by one nice level..

kernel/sched.c +2 -2

Index: l/kernel/sched.c
===================================================================
@@ -3231,8 +3231,8 @@ EXPORT_SYMBOL(set_user_nice);
  */
 int can_nice(const task_t *p, const int nice)
 {
-	/* convert nice value [19,-20] to rlimit style value [0,39] */
-	int nice_rlim = 19 - nice;
+	/* convert nice value [19,-20] to rlimit style value [1,40] */
+	int nice_rlim = 20 - nice;
 	return (nice_rlim <= p->signal->rlim[RLIMIT_NICE].rlim_cur ||
 		capable(CAP_SYS_NICE));
 }

Signature · 1 line

Mathematics is the supreme nostalgia of our time.

Ingo Molnar

Acked-by

2005-07-29 08:40 UTC #4 lore raw

Quoted 2 lines · Matt Mackall

The other downside is, this obviously changes any existing configs actually using this by one nice level..

it's pretty harmless, and i doubt the use of this is all that wide (if existent at all - utilities have not been updated yet). Lets fix it ASAP, preferably in 2.6.13.

Acked-by Ingo Molnar <[email protected]>

Ingo

Michael Kerrisk

author

2005-07-29 10:42 UTC #5 lore raw

Quoted 6 lines

Quoted 2 lines
The other downside is, this obviously changes any existing configs actually using this by one nice level..
it's pretty harmless, and i doubt the use of this is all that wide (if existent at all - utilities have not been updated yet). Lets fix it ASAP, preferably in 2.6.13.

Yes, as noted in my earlier message -- at the moment RLIMIT_NICE still isn't in the current glibc snapshot...

Cheers,

Michael

Signature · 6 lines

Michael Kerrisk
maintainer of Linux man pages Sections 2, 3, 4, 5, and 7 

Want to help with man page maintenance?  Grab the latest
tarball at ftp://ftp.win.tue.nl/pub/linux-local/manpages/
and grep the source files for 'FIXME'.

Nix

2005-07-29 14:51 UTC #6 lore raw

On 29 Jul 2005, Michael Kerrisk stated:

Quoted 2 lines

Yes, as noted in my earlier message -- at the moment RLIMIT_NICE still isn't in the current glibc snapshot...

According to traffic on libc-hacker, Ulrich committed it on Jun 20 (along with RLIMIT_RTPRIO support).

Signature · 2 lines

`Tor employs several thousand editors who they keep in dank
 subterranean editing facilities not unlike Moria' -- James Nicoll

Michael Kerrisk

author

2005-07-29 15:19 UTC #7 lore raw

Quoted 6 lines

On 29 Jul 2005, Michael Kerrisk stated:
Quoted 2 lines
Yes, as noted in my earlier message -- at the moment RLIMIT_NICE still isn't in the current glibc snapshot...
According to traffic on libc-hacker, Ulrich committed it on Jun 20 (along with RLIMIT_RTPRIO support).

I (now) see the message that you mean on libc-hacker, nevertheless, looking at the glibc-2.3-20050725 snapshot, these two constants do not appear anywhere. (Strange!)

Cheers,

Michael

Signature · 6 lines

Michael Kerrisk
maintainer of Linux man pages Sections 2, 3, 4, 5, and 7 

Want to help with man page maintenance?  Grab the latest
tarball at ftp://ftp.win.tue.nl/pub/linux-local/manpages/
and grep the source files for 'FIXME'.

Nix

2005-07-29 20:59 UTC #8 lore raw

On Fri, 29 Jul 2005, Michael Kerrisk uttered the following:

Quoted 10 lines

Quoted 6 lines
On 29 Jul 2005, Michael Kerrisk stated:
Quoted 2 lines
Yes, as noted in my earlier message -- at the moment RLIMIT_NICE still isn't in the current glibc snapshot...
According to traffic on libc-hacker, Ulrich committed it on Jun 20 (along with RLIMIT_RTPRIO support).
I (now) see the message that you mean on libc-hacker, nevertheless, looking at the glibc-2.3-20050725 snapshot, these two constants do not appear anywhere. (Strange!)

That just means it hasn't gone into the stable branch (yet?); since that's bugfix-only and changes are bursted in intermittently by Roland, that's not very surprising.

But if you look in HEAD (2.4-to-be), you'll see it.

Signature · 2 lines

`Tor employs several thousand editors who they keep in dank
 subterranean editing facilities not unlike Moria' -- James Nicoll

Michael Kerrisk

author

2005-07-29 10:40 UTC #9 lore raw

In reply to Matt Mackall

Hello Matt,

Quoted 5 lines

Quoted 2 lines
I'm guessing that it was you that added the RLIMIT_NICE resource limit in 2.6.12.
The original patch was from Chris Wright, but I did most of the cheerleading for it.

Okay -- thanks for the pointer. There was no record of the pach in the (incomplete-because-of-git-changeover) changelog for 2.6.12...

Quoted 9 lines

Quoted 5 lines
(A passing note to all kernel developers: when making changes that affect userland-kernel interfaces, please send me a man-pages patch, or at least a notification of the change, so that some information makes its way into the manual pages).
You might want to make an effort to make yourself more visible around here. Most of us have no idea that anyone's actually trying to maintain the manpages or who that might be.

Fair comment. I do appear now and then here, but I'll try to be a litle more noisy from now on...

Quoted 34 lines

Quoted 34 lines
I started documenting RLIMIT_NICE and then noticed an inconsistency between the use of this limit and the nice value as manipulated by [sg]etpriority().
This is the documentation I've drafted for RLIMIT_NICE in getrlimit.2:
RLIMIT_NICE(since kernel 2.6.12)
Specifies a ceiling to which the process nice
value can be raised using setpriority(2) or
nice(2). The actual ceiling for the nice value is calculated as 19 - rlim_cur.
^^^^^^^^^^^^^
And recently I've redrafted the discussion of the nice value in getpriority.2 and it now reads:
Since kernel 1.3.43 Linux has the range -20..19. Within the kernel, nice values are actually repre- sented using the corresponding range 40..1 (since negative numbers are error codes) and these are the values employed by the setpriority and getpriority
system calls. The glibc wrapper functions for
these system calls handle the translations between the user-land and kernel representations of the
nice value according to the formula
user_nice = 20 - kernel_nice. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
In other words, there is an off-by-one mismatch between these two interfaces: RLIMIT_NICE is expecting to deal with values in the range 39..0, while [gs]etpriority() works with the range 40..1.

Quoted 5 lines

Quoted 5 lines
I suppose that glibc could paper over the cracks here in a wrapper for getrlimit(), but it seems more sensible to make RLIMIT_NICE consistent with [gs]etpriority() -- i.e., change the RLIMIT_NICE interface in 2.6.13 before it sees wide use in userland. What do you think?

[I I should have added here, that looking at the latest glibc snapshot, RLIMIT_NICE still isn't present, so still not very visible to user-land.]

Quoted 14 lines

Well, it's easy enough to do, but some thought has to be given to the corner cases. Specifically, does this do the right thing when the rlimit is set to zero? I think it does, as the nice range is nicely bound here:
        nice = PRIO_TO_NICE(current->static_prio) + increment;
        if (nice < -20)
                nice = -20;
        if (nice > 19)
                nice = 19;

        if (increment < 0 && !can_nice(current, nice))
                return -EPERM;

Quoted 1 line

And we allow task to do negative increment. Chris?

Yes, I believe it is safely bounded also.

Quoted 2 lines

The other downside is, this obviously changes any existing configs actually using this by one nice level..

I don't expect there are likely to be any existing yet.

Quoted 5 lines

Index: l/kernel/sched.c
===================================================================

kernel/sched.c EXPORT_SYMBOL(set_user_nice); · line 3238

Quoted 10 lines

kernel/sched.c +2 -2

@@ -3231,8 +3231,8 @@ EXPORT_SYMBOL(set_user_nice);
  */
 int can_nice(const task_t *p, const int nice)
 {
-	/* convert nice value [19,-20] to rlimit style value [0,39] */
-	int nice_rlim = 19 - nice;
+	/* convert nice value [19,-20] to rlimit style value [1,40] */
+	int nice_rlim = 20 - nice;
 	return (nice_rlim <= p->signal->rlim[RLIMIT_NICE].rlim_cur ||
 		capable(CAP_SYS_NICE));
 }

Thanks.

Cheers,

Michael

Signature · 6 lines

Michael Kerrisk
maintainer of Linux man pages Sections 2, 3, 4, 5, and 7 

Want to help with man page maintenance?  Grab the latest
tarball at ftp://ftp.win.tue.nl/pub/linux-local/manpages/
and grep the source files for 'FIXME'.

Chris Wright

2005-07-29 20:21 UTC #10 lore raw

In reply to Matt Mackall

Quoted 12 lines · * Matt Mackall ([email protected]) wrote:

Quoted 10 lines · Michael Kerrisk
In other words, there is an off-by-one mismatch between these two interfaces: RLIMIT_NICE is expecting to deal with values in the range 39..0, while [gs]etpriority() works with the range 40..1.
I suppose that glibc could paper over the cracks here in a wrapper for getrlimit(), but it seems more sensible to make RLIMIT_NICE consistent with [gs]etpriority() -- i.e., change the RLIMIT_NICE interface in 2.6.13 before it sees wide use in userland. What do you think?

Quoted 4 lines

Well, it's easy enough to do, but some thought has to be given to the corner cases. Specifically, does this do the right thing when the rlimit is set to zero? I think it does, as the nice range is nicely bound here:

Agreed, it should be fine. The default rlimit of zero is just as effective with the adjust by one semantics.

Quoted 10 lines

        nice = PRIO_TO_NICE(current->static_prio) + increment;
        if (nice < -20)
                nice = -20;
        if (nice > 19)
                nice = 19;

        if (increment < 0 && !can_nice(current, nice))
                return -EPERM;

And we allow task to do negative increment. Chris?

Yes, if the rlimit is permissive enough. So, for example with default, there's no chance for any negative increment. With rlimit of 1 (or 2 with adjust-by-one) you could nice down to 19, then nice back up to a max of 18.

Quoted 2 lines

The other downside is, this obviously changes any existing configs actually using this by one nice level..

Yes, this requires updated pam patch. Otherwise,

ACK

thanks,
-chris

Quoted 15 lines

Index: l/kernel/sched.c
===================================================================

kernel/sched.c +2 -2

@@ -3231,8 +3231,8 @@ EXPORT_SYMBOL(set_user_nice);
  */
 int can_nice(const task_t *p, const int nice)
 {
-	/* convert nice value [19,-20] to rlimit style value [0,39] */
-	int nice_rlim = 19 - nice;
+	/* convert nice value [19,-20] to rlimit style value [1,40] */
+	int nice_rlim = 20 - nice;
 	return (nice_rlim <= p->signal->rlim[RLIMIT_NICE].rlim_cur ||
 		capable(CAP_SYS_NICE));
 }

Chris Wright

2005-07-29 20:54 UTC #11 lore raw

Quoted 1 line · * Chris Wright ([email protected]) wrote:

Yes, this requires updated pam patch.

Here's the updated pam patch. I left the lower end at 0 rather than 1, since it's no harm.

modules/pam_limits/pam_limits.c renamed from modules/pam_limits/pam_limits.c.prio +22 -0

@@ -39,6 +39,11 @@
 #include <grp.h>
 #include <pwd.h>
 
+/* Hack to test new rlimit values */
+#define RLIMIT_NICE	13
+#define RLIMIT_RTPRIO	14
+#define RLIM_NLIMITS	15
+
 /* Module defines */
 #define LINE_LENGTH 1024

@@ -293,6 +298,10 @@ static void process_limit(int source, co
     else if (strcmp(lim_item, "locks") == 0)
 	limit_item = RLIMIT_LOCKS;
 #endif
+    else if (strcmp(lim_item, "rt_priority") == 0)
+	limit_item = RLIMIT_RTPRIO;
+    else if (strcmp(lim_item, "nice") == 0)
+	limit_item = RLIMIT_NICE;
     else if (strcmp(lim_item, "maxlogins") == 0) {
 	limit_item = LIMIT_LOGIN;
 	pl->flag_numsyslogins = 0;

@@ -360,6 +369,19 @@ static void process_limit(int source, co
         case RLIMIT_AS:
             limit_value *= 1024;
             break;
+        case RLIMIT_NICE:
+            limit_value = 20 - limit_value;
+            if (limit_value > 40)
+		limit_value = 40;
+	    if (limit_value < 0)
+		limit_value = 0;
+            break;
+        case RLIMIT_RTPRIO:
+            if (limit_value > 99)
+		limit_value = 99;
+	    if (limit_value < 0)
+		limit_value = 0;
+            break;
     }
 
     if ( (limit_item != LIMIT_LOGIN)

Lee Revell

2005-07-29 21:05 UTC #12 lore raw

Quoted 7 lines · Chris Wright

Quoted 1 line · * Chris Wright ([email protected]) wrote:
Yes, this requires updated pam patch.
Here's the updated pam patch. I left the lower end at 0 rather than 1, since it's no harm.
+/* Hack to test new rlimit values */

Does this still apply? If so what would a better solution look like?

Lee

Chris Wright

2005-07-29 21:10 UTC #13 lore raw

Quoted 10 lines · * Lee Revell ([email protected]) wrote:

Quoted 7 lines · Chris Wright
Quoted 1 line · * Chris Wright ([email protected]) wrote:
Yes, this requires updated pam patch.
Here's the updated pam patch. I left the lower end at 0 rather than 1, since it's no harm.
+/* Hack to test new rlimit values */
Does this still apply? If so what would a better solution look like?

The better solution is to make sure you have new enough glibc-kernheaders to pick up those values directly from resource.h and drop that hunk. On my systems, it's still needed even with rawhide headers.

thanks,
-chris

Matt Mackall

2005-08-15 20:13 UTC #14 lore raw

PATCH 1/1 Fix nice range for RLIMIT NICE

In reply to Matt Mackall

Looks like I let this one slip through the cracks:

Make RLIMIT_NICE ranges consistent with getpriority(2)

As suggested by Michael Kerrisk <[email protected]>, make RLIMIT_NICE consistent with getpriority before it becomes available in released glibc.

Signed-off-by Matt Mackall <[email protected]>

Acked-by Ingo Molnar <[email protected]>

Acked-by Chris Wright <[email protected]>

kernel/sched.c +2 -2

Index: lhg/kernel/sched.c
===================================================================
@@ -3378,8 +3378,8 @@ EXPORT_SYMBOL(set_user_nice);
  */
 int can_nice(const task_t *p, const int nice)
 {
-	/* convert nice value [19,-20] to rlimit style value [0,39] */
-	int nice_rlim = 19 - nice;
+	/* convert nice value [19,-20] to rlimit style value [1,40] */
+	int nice_rlim = 20 - nice;
 	return (nice_rlim <= p->signal->rlim[RLIMIT_NICE].rlim_cur ||
 		capable(CAP_SYS_NICE));
 }

Signature · 1 line

Mathematics is the supreme nostalgia of our time.