Messages in this thread Patch in this message | | | Date | Tue, 23 Apr 2002 09:53:54 +0200 (CEST) | From | Ingo Molnar <> | Subject | Re: [PATCH] 2.5: MAX_PRIO cleanup |
| |
On 22 Apr 2002, Robert Love wrote:
> Attached patch replaces occurrences of the magic numbers representing > maximum priority / maximum RT priority with the (already defined and > used) MAX_PRIO and MAX_RT_PRIO defines. The patch also contains some > comment additions/changes (particularly to address the double_rq_lock > ambiguity). Very simple.
i agree that this area needs cleaning up, but i dont agree with all aspects of your patch. I intentionally left the user-space API side separate, MAX_RT can in fact be higher than 100 (without changing the user-space API), the only rule is that it must not be smaller. We in fact had such a situation once. It's a perfectly valid goal to have 'super high prio' kernel-space threads in the future that have in fact some priority that cannot be reached by user-space threads.
so i've re-done a variation of your patch, which defines USER_MAX_RT_PRIO, so the user-space API can still stay separate from the kernel-internal representation.
i've also done some other changes:
> /* > - * Priority of a process goes from 0 to 139. The 0-99 > - * priority range is allocated to RT tasks, the 100-139 > - * range is for SCHED_OTHER tasks. Priority values are > - * inverted: lower p->prio value means higher priority. > + * Priority of a process goes from 0 to MAX_PRIO-1. The > + * 0 to MAX_RT_PRIO-1 priority range is allocated to RT tasks, > + * the MAX_RT_PRIO to MAX_PRIO range is for SCHED_OTHER tasks. > + * Priority values are inverted: lower p->prio value means higher > + * priority.
this i dont agree with either. The point of comments is easy understanding, so i intentionally kept the 'hard' constants and i'm updating them constantly - it's much easier to understand how things happen if it does not happen via a define. The code itself i agree should stay abstract, but the comments should stay as humanly readable as possible.
(the set|get_affinity comment fixes i kept, plus the runqueue double-lock/unlock comments as well, see the attached patch.)
Ingo
# This is a BitKeeper generated patch for the following project: # Project Name: Linux kernel tree # This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas: # ChangeSet 1.544 -> 1.545 # kernel/sched.c 1.71 -> 1.72 # # The following is the BitKeeper ChangeSet Log # -------------------------------------------- # 02/04/23 mingo@elte.hu 1.545 # - introduce MAX_USER_RT_PRIO # - comment fixes from rml # -------------------------------------------- # diff -Nru a/kernel/sched.c b/kernel/sched.c --- a/kernel/sched.c Tue Apr 23 09:47:59 2002 +++ b/kernel/sched.c Tue Apr 23 09:47:59 2002 @@ -28,8 +28,11 @@ * priority range is allocated to RT tasks, the 100-139 * range is for SCHED_OTHER tasks. Priority values are * inverted: lower p->prio value means higher priority. + * + * NOTE: MAX_RT_PRIO must not be smaller than MAX_USER_RT_PRIO. */ #define MAX_RT_PRIO 100 +#define MAX_USER_RT_PRIO 100 #define MAX_PRIO (MAX_RT_PRIO + 40) /* @@ -1071,7 +1074,7 @@ */ int task_prio(task_t *p) { - return p->prio - 100; + return p->prio - MAX_USER_RT_PRIO; } int task_nice(task_t *p) @@ -1137,7 +1140,7 @@ * priority for SCHED_OTHER is 0. */ retval = -EINVAL; - if (lp.sched_priority < 0 || lp.sched_priority > 99) + if (lp.sched_priority < 0 || lp.sched_priority > MAX_USER_RT_PRIO-1) goto out_unlock; if ((policy == SCHED_OTHER) != (lp.sched_priority == 0)) goto out_unlock; @@ -1157,7 +1160,7 @@ p->policy = policy; p->rt_priority = lp.sched_priority; if (policy != SCHED_OTHER) - p->prio = 99 - p->rt_priority; + p->prio = MAX_USER_RT_PRIO-1 - p->rt_priority; else p->prio = p->static_prio; if (array) @@ -1237,7 +1240,7 @@ /** * sys_sched_setaffinity - set the cpu affinity of a process * @pid: pid of the process - * @len: length of the bitmask pointed to by user_mask_ptr + * @len: length in bytes of the bitmask pointed to by user_mask_ptr * @user_mask_ptr: user-space pointer to the new cpu mask */ asmlinkage int sys_sched_setaffinity(pid_t pid, unsigned int len, @@ -1289,7 +1292,7 @@ /** * sys_sched_getaffinity - get the cpu affinity of a process * @pid: pid of the process - * @len: length of the bitmask pointed to by user_mask_ptr + * @len: length in bytes of the bitmask pointed to by user_mask_ptr * @user_mask_ptr: user-space pointer to hold the current cpu mask */ asmlinkage int sys_sched_getaffinity(pid_t pid, unsigned int len, @@ -1371,7 +1374,7 @@ switch (policy) { case SCHED_FIFO: case SCHED_RR: - ret = 99; + ret = MAX_USER_RT_PRIO-1; break; case SCHED_OTHER: ret = 0; @@ -1511,6 +1514,12 @@ read_unlock(&tasklist_lock); } +/* + * double_rq_lock - safely lock two runqueues + * + * Note this does not disable interrupts like task_rq_lock, + * you need to do so manually before calling. + */ static inline void double_rq_lock(runqueue_t *rq1, runqueue_t *rq2) { if (rq1 == rq2) @@ -1526,6 +1535,12 @@ } } +/* + * double_rq_unlock - safely unlock two runqueues + * + * Note this does not restore interrupts like task_rq_unlock, + * you need to do so manually after calling. + */ static inline void double_rq_unlock(runqueue_t *rq1, runqueue_t *rq2) { spin_unlock(&rq1->lock); @@ -1675,7 +1690,7 @@ static int migration_thread(void * bind_cpu) { int cpu = cpu_logical_map((int) (long) bind_cpu); - struct sched_param param = { sched_priority: 99 }; + struct sched_param param = { sched_priority: MAX_RT_PRIO-1 }; runqueue_t *rq; int ret; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |