* [PATCH] niceness magic numbers, 2.4.20-pre11
@ 2002-10-23 4:59 Kristis Makris
2002-10-23 6:37 ` Randy.Dunlap
0 siblings, 1 reply; 8+ messages in thread
From: Kristis Makris @ 2002-10-23 4:59 UTC (permalink / raw)
To: linux-kernel
[-- Attachment #1: Type: text/plain, Size: 219 bytes --]
This untested patch removes use of process priority magic numbers in
sys_nice, sys_setpriority, sys_getpriority, and properly uses their
#define'd values.
It would be nice if someone tested if it applies against 2.5.
[-- Attachment #2: niceness_magic_numbers.patch --]
[-- Type: text/x-patch, Size: 2060 bytes --]
diff -ur linux-2.4.20-pre11.orig/include/linux/resource.h linux/include/linux/resource.h
--- linux-2.4.20-pre11.orig/include/linux/resource.h Tue Jun 18 20:10:36 2002
+++ linux/include/linux/resource.h Sat Oct 19 13:55:10 2002
@@ -43,7 +43,7 @@
};
#define PRIO_MIN (-20)
-#define PRIO_MAX 20
+#define PRIO_MAX 19
#define PRIO_PROCESS 0
#define PRIO_PGRP 1
diff -ur linux-2.4.20-pre11.orig/kernel/sched.c linux/kernel/sched.c
--- linux-2.4.20-pre11.orig/kernel/sched.c Sat Oct 19 13:26:59 2002
+++ linux/kernel/sched.c Sat Oct 19 13:41:19 2002
@@ -870,17 +870,19 @@
if (increment < 0) {
if (!capable(CAP_SYS_NICE))
return -EPERM;
- if (increment < -40)
- increment = -40;
+
+ /* +1 to account for 0 in min<-->max range */
+ if (increment < PRIO_MIN - (PRIO_MAX + 1))
+ increment = PRIO_MIN - (PRIO_MAX + 1);
}
- if (increment > 40)
- increment = 40;
+ if (increment > -(PRIO_MIN - (PRIO_MAX + 1)))
+ increment = -(PRIO_MIN - (PRIO_MAX + 1));
newprio = current->nice + increment;
- if (newprio < -20)
- newprio = -20;
- if (newprio > 19)
- newprio = 19;
+ if (newprio < PRIO_MIN)
+ newprio = PRIO_MIN;
+ if (newprio > PRIO_MAX)
+ newprio = PRIO_MAX;
current->nice = newprio;
return 0;
}
diff -ur linux-2.4.20-pre11.orig/kernel/sys.c linux/kernel/sys.c
--- linux-2.4.20-pre11.orig/kernel/sys.c Sat Oct 19 11:58:48 2002
+++ linux/kernel/sys.c Sat Oct 19 13:37:48 2002
@@ -204,10 +204,10 @@
/* normalize: avoid signed division (rounding problems) */
error = -ESRCH;
- if (niceval < -20)
- niceval = -20;
- if (niceval > 19)
- niceval = 19;
+ if (niceval < PRIO_MIN)
+ niceval = PRIO_MIN;
+ if (niceval > PRIO_MAX)
+ niceval = PRIO_MAX;
read_lock(&tasklist_lock);
for_each_task(p) {
@@ -249,7 +249,8 @@
long niceval;
if (!proc_sel(p, which, who))
continue;
- niceval = 20 - p->nice;
+ niceval = PRIO_MAX + 1 - p->nice; /* +1 to account for 0
+ * in 0<-->max range */
if (niceval > retval)
retval = niceval;
}
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] niceness magic numbers, 2.4.20-pre11 2002-10-23 4:59 [PATCH] niceness magic numbers, 2.4.20-pre11 Kristis Makris @ 2002-10-23 6:37 ` Randy.Dunlap 2002-10-23 8:16 ` Alan Cox 0 siblings, 1 reply; 8+ messages in thread From: Randy.Dunlap @ 2002-10-23 6:37 UTC (permalink / raw) To: Kristis Makris; +Cc: linux-kernel On 22 Oct 2002, Kristis Makris wrote: | This untested patch removes use of process priority magic numbers in | sys_nice, sys_setpriority, sys_getpriority, and properly uses their | #define'd values. | | It would be nice if someone tested if it applies against 2.5. Nope, it doesn't apply cleanly to 2.5.44: [rddunlap@midway linux-2544]$ patch -p1 -b --dry-run < ~/patches/niceness_magic_numbers.patch patching file include/linux/resource.h patching file kernel/sched.c Hunk #1 FAILED at 870. 1 out of 1 hunk FAILED -- saving rejects to file kernel/sched.c.rej patching file kernel/sys.c Hunk #1 succeeded at 236 with fuzz 1 (offset 32 lines). Hunk #2 FAILED at 281. 1 out of 2 hunks FAILED -- saving rejects to file kernel/sys.c.rej Here is a working version for 2.5.44, with a small change so that -(A - (B + 1)) is written as B - A + 1 so go push it. :) --- ./include/linux/resource.h.nice Tue Jun 18 20:10:36 2002 +++ ./include/linux/resource.h Sat Oct 19 13:55:10 2002 @@ -43,7 +43,7 @@ }; #define PRIO_MIN (-20) -#define PRIO_MAX 20 +#define PRIO_MAX 19 #define PRIO_PROCESS 0 #define PRIO_PGRP 1 --- ./kernel/sys.c.nice Fri Oct 18 21:01:11 2002 +++ ./kernel/sys.c Tue Oct 22 22:57:49 2002 @@ -236,10 +236,10 @@ /* normalize: avoid signed division (rounding problems) */ error = -ESRCH; - if (niceval < -20) - niceval = -20; - if (niceval > 19) - niceval = 19; + if (niceval < PRIO_MIN) + niceval = PRIO_MIN; + if (niceval > PRIO_MAX) + niceval = PRIO_MAX; read_lock(&tasklist_lock); switch (which) { @@ -301,7 +301,7 @@ who = current->pid; p = find_task_by_pid(who); if (p) { - niceval = 20 - task_nice(p); + niceval = PRIO_MAX + 1 - task_nice(p); if (niceval > retval) retval = niceval; } @@ -310,7 +310,7 @@ if (!who) who = current->pgrp; for_each_task_pid(who, PIDTYPE_PGID, p, l, pid) { - niceval = 20 - task_nice(p); + niceval = PRIO_MAX + 1 - task_nice(p); if (niceval > retval) retval = niceval; } @@ -326,7 +326,7 @@ do_each_thread(g, p) if (p->uid == who) { - niceval = 20 - task_nice(p); + niceval = PRIO_MAX + 1 - task_nice(p); if (niceval > retval) retval = niceval; } --- ./kernel/sched.c.nice Fri Oct 18 21:02:28 2002 +++ ./kernel/sched.c Tue Oct 22 22:51:43 2002 @@ -1317,17 +1317,19 @@ if (increment < 0) { if (!capable(CAP_SYS_NICE)) return -EPERM; - if (increment < -40) - increment = -40; + + /* +1 to account for 0 in min<-->max range */ + if (increment < PRIO_MIN - PRIO_MAX + 1) + increment = PRIO_MIN - PRIO_MAX + 1; } - if (increment > 40) - increment = 40; + if (increment > (PRIO_MAX - PRIO_MIN + 1)) + increment = PRIO_MAX - PRIO_MIN + 1; nice = PRIO_TO_NICE(current->static_prio) + increment; - if (nice < -20) - nice = -20; - if (nice > 19) - nice = 19; + if (nice < PRIO_MIN) + nice = PRIO_MIN; + if (nice > PRIO_MAX) + nice = PRIO_MAX; retval = security_ops->task_setnice(current, nice); if (retval) -- ~Randy ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] niceness magic numbers, 2.4.20-pre11 2002-10-23 6:37 ` Randy.Dunlap @ 2002-10-23 8:16 ` Alan Cox 2002-10-23 19:52 ` Robert Love 0 siblings, 1 reply; 8+ messages in thread From: Alan Cox @ 2002-10-23 8:16 UTC (permalink / raw) To: Randy.Dunlap; +Cc: Kristis Makris, Linux Kernel Mailing List On Wed, 2002-10-23 at 07:37, Randy.Dunlap wrote: > --- ./include/linux/resource.h.nice Tue Jun 18 20:10:36 2002 > +++ ./include/linux/resource.h Sat Oct 19 13:55:10 2002 > @@ -43,7 +43,7 @@ > }; > > #define PRIO_MIN (-20) > -#define PRIO_MAX 20 > +#define PRIO_MAX 19 I suspect this isnt correct ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] niceness magic numbers, 2.4.20-pre11 2002-10-23 8:16 ` Alan Cox @ 2002-10-23 19:52 ` Robert Love 2002-10-23 23:50 ` [PATCH] niceness magic numbers, 2.5.44 now Randy.Dunlap 2002-10-24 3:22 ` [PATCH] niceness magic numbers, 2.4.20-pre11 Kristis Makris 0 siblings, 2 replies; 8+ messages in thread From: Robert Love @ 2002-10-23 19:52 UTC (permalink / raw) To: Alan Cox; +Cc: Randy.Dunlap, Kristis Makris, Linux Kernel Mailing List On Wed, 2002-10-23 at 04:16, Alan Cox wrote: > On Wed, 2002-10-23 at 07:37, Randy.Dunlap wrote: > > --- ./include/linux/resource.h.nice Tue Jun 18 20:10:36 2002 > > +++ ./include/linux/resource.h Sat Oct 19 13:55:10 2002 > > @@ -43,7 +43,7 @@ > > }; > > > > #define PRIO_MIN (-20) > > -#define PRIO_MAX 20 > > +#define PRIO_MAX 19 > > I suspect this isnt correct Agreed. Its not. It should remain 20 and places that truly want 19 should do PRIO_MAX-1. The idea of cleaning up the magic numbers is fine, otherwise.. Robert Love ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] niceness magic numbers, 2.5.44 now. 2002-10-23 19:52 ` Robert Love @ 2002-10-23 23:50 ` Randy.Dunlap 2002-10-24 0:11 ` Robert Love 2002-10-24 3:22 ` [PATCH] niceness magic numbers, 2.4.20-pre11 Kristis Makris 1 sibling, 1 reply; 8+ messages in thread From: Randy.Dunlap @ 2002-10-23 23:50 UTC (permalink / raw) To: Robert Love; +Cc: Alan Cox, Kristis Makris, Linux Kernel Mailing List On 23 Oct 2002, Robert Love wrote: | On Wed, 2002-10-23 at 04:16, Alan Cox wrote: | | > On Wed, 2002-10-23 at 07:37, Randy.Dunlap wrote: | > > --- ./include/linux/resource.h.nice Tue Jun 18 20:10:36 2002 | > > +++ ./include/linux/resource.h Sat Oct 19 13:55:10 2002 | > > @@ -43,7 +43,7 @@ | > > }; | > > | > > #define PRIO_MIN (-20) | > > -#define PRIO_MAX 20 | > > +#define PRIO_MAX 19 | > | > I suspect this isnt correct | | Agreed. Its not. | | It should remain 20 and places that truly want 19 should do PRIO_MAX-1. | | The idea of cleaning up the magic numbers is fine, otherwise.. Thanks Alan and Robert. Yes, I also prefer to remove magic numbers like these. Here's a corrected patch to 2.5.44 with an additional magic# instance removal. See any other problems with it? -- ~Randy --- ./kernel/sys.c.nice Fri Oct 18 21:01:11 2002 +++ ./kernel/sys.c Wed Oct 23 16:20:54 2002 @@ -236,10 +236,10 @@ /* normalize: avoid signed division (rounding problems) */ error = -ESRCH; - if (niceval < -20) - niceval = -20; - if (niceval > 19) - niceval = 19; + if (niceval < PRIO_MIN) + niceval = PRIO_MIN; + if (niceval > PRIO_MAX - 1) + niceval = PRIO_MAX - 1; read_lock(&tasklist_lock); switch (which) { @@ -301,7 +301,7 @@ who = current->pid; p = find_task_by_pid(who); if (p) { - niceval = 20 - task_nice(p); + niceval = PRIO_MAX - task_nice(p); if (niceval > retval) retval = niceval; } @@ -310,7 +310,7 @@ if (!who) who = current->pgrp; for_each_task_pid(who, PIDTYPE_PGID, p, l, pid) { - niceval = 20 - task_nice(p); + niceval = PRIO_MAX - task_nice(p); if (niceval > retval) retval = niceval; } @@ -326,7 +326,7 @@ do_each_thread(g, p) if (p->uid == who) { - niceval = 20 - task_nice(p); + niceval = PRIO_MAX - task_nice(p); if (niceval > retval) retval = niceval; } --- ./kernel/sched.c.nice Tue Oct 22 17:56:13 2002 +++ ./kernel/sched.c Wed Oct 23 16:28:01 2002 @@ -45,7 +45,7 @@ /* * 'User priority' is the nice value converted to something we * can work with better when scaling various scheduler parameters, - * it's a [ 0 ... 39 ] range. + * it's in [ 0 ... 39 ] range. */ #define USER_PRIO(p) ((p)-MAX_RT_PRIO) #define TASK_USER_PRIO(p) USER_PRIO((p)->static_prio) @@ -1265,7 +1265,7 @@ prio_array_t *array; runqueue_t *rq; - if (TASK_NICE(p) == nice || nice < -20 || nice > 19) + if (TASK_NICE(p) == nice || nice < PRIO_MIN || nice > PRIO_MAX - 1) return; /* * We have to be careful, if called from sys_setpriority(), @@ -1317,17 +1317,18 @@ if (increment < 0) { if (!capable(CAP_SYS_NICE)) return -EPERM; - if (increment < -40) - increment = -40; + + if (increment < PRIO_MIN - PRIO_MAX) + increment = PRIO_MIN - PRIO_MAX; } - if (increment > 40) - increment = 40; + if (increment > (PRIO_MAX - PRIO_MIN)) + increment = PRIO_MAX - PRIO_MIN; nice = PRIO_TO_NICE(current->static_prio) + increment; - if (nice < -20) - nice = -20; - if (nice > 19) - nice = 19; + if (nice < PRIO_MIN) + nice = PRIO_MIN; + if (nice > PRIO_MAX - 1) + nice = PRIO_MAX - 1; retval = security_ops->task_setnice(current, nice); if (retval) ### ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] niceness magic numbers, 2.5.44 now. 2002-10-23 23:50 ` [PATCH] niceness magic numbers, 2.5.44 now Randy.Dunlap @ 2002-10-24 0:11 ` Robert Love 0 siblings, 0 replies; 8+ messages in thread From: Robert Love @ 2002-10-24 0:11 UTC (permalink / raw) To: Randy.Dunlap; +Cc: Alan Cox, Kristis Makris, Linux Kernel Mailing List On Wed, 2002-10-23 at 19:50, Randy.Dunlap wrote: > Thanks Alan and Robert. > Yes, I also prefer to remove magic numbers like these. > Here's a corrected patch to 2.5.44 with an additional magic# > instance removal. See any other problems with it? Nope, looks good to me. Robert Love ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] niceness magic numbers, 2.4.20-pre11 2002-10-23 19:52 ` Robert Love 2002-10-23 23:50 ` [PATCH] niceness magic numbers, 2.5.44 now Randy.Dunlap @ 2002-10-24 3:22 ` Kristis Makris 2002-10-24 3:32 ` Robert Love 1 sibling, 1 reply; 8+ messages in thread From: Kristis Makris @ 2002-10-24 3:22 UTC (permalink / raw) To: Robert Love; +Cc: Alan Cox, Randy.Dunlap, Linux Kernel Mailing List > > > -#define PRIO_MAX 20 > > > +#define PRIO_MAX 19 > > > > I suspect this isnt correct > > Agreed. Its not. > > It should remain 20 and places that truly want 19 should do PRIO_MAX-1. Please excuse my inexperience; I don't understand why a 41 value range is defined instead, and faked as a 40. Is this merely for the convinience of ignoring 0 ? From the nice manpage: "Range goes from -20 (highest priority) to 19 (lowest)." Thanks, Kristis ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] niceness magic numbers, 2.4.20-pre11 2002-10-24 3:22 ` [PATCH] niceness magic numbers, 2.4.20-pre11 Kristis Makris @ 2002-10-24 3:32 ` Robert Love 0 siblings, 0 replies; 8+ messages in thread From: Robert Love @ 2002-10-24 3:32 UTC (permalink / raw) To: Kristis Makris; +Cc: Alan Cox, Randy.Dunlap, Linux Kernel Mailing List On Wed, 2002-10-23 at 23:22, Kristis Makris wrote: excuse my inexperience; I don't understand why a 41 value range > is defined instead, and faked as a 40. Is this merely for the > convinience of ignoring 0 ? From the nice manpage: > > "Range goes from -20 (highest priority) to 19 (lowest)." It is exported to user-space. Changing it will break things. Robert Love ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2002-10-24 3:26 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2002-10-23 4:59 [PATCH] niceness magic numbers, 2.4.20-pre11 Kristis Makris 2002-10-23 6:37 ` Randy.Dunlap 2002-10-23 8:16 ` Alan Cox 2002-10-23 19:52 ` Robert Love 2002-10-23 23:50 ` [PATCH] niceness magic numbers, 2.5.44 now Randy.Dunlap 2002-10-24 0:11 ` Robert Love 2002-10-24 3:22 ` [PATCH] niceness magic numbers, 2.4.20-pre11 Kristis Makris 2002-10-24 3:32 ` Robert Love
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox