lkml.org 
[lkml]   [2002]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-03-22 13:25    [W:0.095 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site