use task_nice() in tracing to git rid of an open coded impolementation of it.

PATCH Superseded Dongsheng Yang 2 participants 11 messages Mar 12, 2014 – May 9, 2014

Subsystems (2)

Tracing Scheduler

lore

Share & tools

lore

Dongsheng Yang

author

2014-03-12 10:32 UTC #0 lore raw

cover use task_nice() in tracing to git rid of an open coded impolementation of it.

Hi Steven,
The [1/3] in this thread is a copy of

[email protected].

At that time, you said you will take this patch if task_nice()
is implemeted as a inline function. Now it is done, I wish you can give it an ack. If you have a time, please take a look at the other patches in this thread. Thank you.

Dongsheng Yang (3):
tracing: Use inline task_nice() to get rid of an open coded
implementation of it.
sched/prio: Replace hardcoding of 40 with NICE_WIDTH.
sched: Use clamp() and clamp_val() to make it more readable.

 kernel/sched/core.c  | 11 ++---------
 kernel/trace/trace.c |  2 +-
 2 files changed, 3 insertions(+), 10 deletions(-)

Signature · 1 line

1.8.2.1

Dongsheng Yang

author

2014-03-12 10:32 UTC #1 lore raw

PATCH RESEND 1/3 tracing: Use inline task_nice() to get rid of an open coded implementation of it.

Function task_nice() was reimplemented as inline function, we can use it here to replace the open coded implementation.

Signed-off-by Dongsheng Yang <[email protected]>

cc Steven Rostedt <[email protected]>

---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/trace/trace.c +1 -1

index 815c878..dba0e3d 100644
@@ -1003,7 +1003,7 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
 	else
 		max_data->uid = task_uid(tsk);
 
-	max_data->nice = tsk->static_prio - 20 - MAX_RT_PRIO;
+	max_data->nice = task_nice(tsk);
 	max_data->policy = tsk->policy;
 	max_data->rt_priority = tsk->rt_priority;

Signature · 1 line

1.8.2.1

Steven Rostedt

maintainer

2014-03-19 02:15 UTC #2 lore raw

Re: [PATCH 1/3 Resend] tracing: Use inline task_nice() to get rid of an open coded implementation of it.

Quoted 14 lines · On Wed, 12 Mar 2014 18:26:42 +0800 Dongsheng Yang <[email protected]>…

Function task_nice() was reimplemented as inline function, we can use it here to replace the open coded implementation.
Signed-off-by Dongsheng Yang <[email protected]>
cc Steven Rostedt <[email protected]>
---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/trace.c +0 -0
index 815c878..dba0e3d 100644

kernel/trace/trace.c __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu) · line 1006

Quoted 5 lines

kernel/trace/trace.c +1 -1

@@ -1003,7 +1003,7 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
 	else
 		max_data->uid = task_uid(tsk);
 
-	max_data->nice = tsk->static_prio - 20 - MAX_RT_PRIO;
+	max_data->nice = task_nice(tsk);

task_nice() is still an extern function in mainline, and this is the tracing side. I rather wait till it becomes inline in Linus's tree before I take this.

-- Steve

Quoted 3 lines

kernel/trace/trace.c +0 -0

 	max_data->policy = tsk->policy;
 	max_data->rt_priority = tsk->rt_priority;

Dongsheng Yang

author

2014-03-19 02:22 UTC #3 lore raw

Re: [PATCH 1/3 Resend] tracing: Use inline task_nice() to get rid of an open coded implementation of it.

Quoted 20 lines · Steven Rostedt

Quoted 17 lines · On Wed, 12 Mar 2014 18:26:42 +0800 Dongsheng Yang <[email protected]>…
Function task_nice() was reimplemented as inline function, we can use it here to replace the open coded implementation.
Signed-off-by Dongsheng Yang <[email protected]>
cc Steven Rostedt <[email protected]>
---
  kernel/trace/trace.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/trace.c +0 -0
index 815c878..dba0e3d 100644
@@ -1003,7 +1003,7 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
  	else
  		max_data->uid = task_uid(tsk);
 

Quoted 5 lines

kernel/trace/trace.c __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu) · line 1006
Quoted 2 lines
kernel/trace/trace.c +1 -1
-	max_data->nice = tsk->static_prio - 20 - MAX_RT_PRIO;
+	max_data->nice = task_nice(tsk);
task_nice() is still an extern function in mainline, and this is the tracing side. I rather wait till it becomes inline in Linus's tree before I take this.

Sounds reasonable to me.

Thanx.

Quoted 12 lines

-- Steve
kernel/trace/trace.c __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu) · line 1009
Quoted 3 lines
kernel/trace/trace.c +0 -0
  	max_data->policy = tsk->policy;
  	max_data->rt_priority = tsk->rt_priority;
 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Dongsheng Yang

author

2014-05-08 07:38 UTC #4 lore raw

PATCH RESEND 1/1 tracing: Use inline task_nice() to get rid of an open coded implementation of it.

In reply to Steven Rostedt

Function task_nice() was reimplemented as inline function, we can use it here to replace the open coded implementation.

Signed-off-by Dongsheng Yang <[email protected]>

cc Steven Rostedt <[email protected]>

---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/trace/trace.c +1 -1

index 737b0ef..f296adc 100644
@@ -1015,7 +1015,7 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
 	else
 		max_data->uid = task_uid(tsk);
 
-	max_data->nice = tsk->static_prio - 20 - MAX_RT_PRIO;
+	max_data->nice = task_nice(tsk);
 	max_data->policy = tsk->policy;
 	max_data->rt_priority = tsk->rt_priority;

Signature · 1 line

1.8.2.1

Dongsheng Yang

author

2014-05-09 01:17 UTC #5 lore raw

Re: [PATCH RESEND] tracing: Use inline task_nice() to get rid of an open coded implementation of it.

Hi steven, as you request, I resend this patch now when function task_nice() is already in mainline. Do you want to take it?

Quoted 22 lines · Dongsheng Yang

Function task_nice() was reimplemented as inline function, we can use it here to replace the open coded implementation.
Signed-off-by Dongsheng Yang <[email protected]>
cc Steven Rostedt <[email protected]>
---
  kernel/trace/trace.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
kernel/trace/trace.c +1 -1
index 737b0ef..f296adc 100644
@@ -1015,7 +1015,7 @@ __update_max_tr(struct trace_array *tr, struct task_struct *tsk, int cpu)
  	else
  		max_data->uid = task_uid(tsk);
 
-	max_data->nice = tsk->static_prio - 20 - MAX_RT_PRIO;
+	max_data->nice = task_nice(tsk);
  	max_data->policy = tsk->policy;
  	max_data->rt_priority = tsk->rt_priority;
 

Steven Rostedt

maintainer

2014-05-09 01:24 UTC #6 lore raw

Re: [PATCH RESEND] tracing: Use inline task_nice() to get rid of an open coded implementation of it.

Quoted 3 lines · On Fri, 9 May 2014 09:17:27 +0900 Dongsheng Yang <[email protected]>…

Hi steven, as you request, I resend this patch now when function task_nice() is already in mainline. Do you want to take it?

Yeah, I probably will. I've just been a bit busy with other things at the moment.

-- Steve

Dongsheng Yang

author

2014-03-12 11:15 UTC #7 lore raw

PATCH 2/3 sched/prio: Replace hardcoding of 40 with NICE_WIDTH.

In reply to Dongsheng Yang

Signed-off-by Dongsheng Yang <[email protected]>

---
 kernel/sched/core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

kernel/sched/core.c +4 -4

index 9e126a2..259ab85 100644
@@ -3068,10 +3068,10 @@ SYSCALL_DEFINE1(nice, int, increment)
 	 * We don't have to worry. Conceptually one call occurs first
 	 * and we have a single winner.
 	 */
-	if (increment < -40)
-		increment = -40;
-	if (increment > 40)
-		increment = 40;
+	if (increment < -NICE_WIDTH)
+		increment = -NICE_WIDTH;
+	if (increment > NICE_WIDTH)
+		increment = NICE_WIDTH;
 
 	nice = task_nice(current) + increment;
 	if (nice < MIN_NICE)

Signature · 1 line

1.8.2.1

Dongsheng Yang

author

2014-03-12 10:33 UTC #8 lore raw

PATCH 3/3 sched: Use clamp() and clamp_val() to make it more readable.

In reply to Dongsheng Yang

As Kees suggested, I use clamp() function to replace the if and else branch, making it more readable and modular.

Suggested-by Kees Cook <[email protected]>

Signed-off-by Dongsheng Yang <[email protected]>

---
 kernel/sched/core.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

kernel/sched/core.c +2 -9

index 259ab85..85f4231 100644
@@ -3068,17 +3068,10 @@ SYSCALL_DEFINE1(nice, int, increment)
 	 * We don't have to worry. Conceptually one call occurs first
 	 * and we have a single winner.
 	 */
-	if (increment < -NICE_WIDTH)
-		increment = -NICE_WIDTH;
-	if (increment > NICE_WIDTH)
-		increment = NICE_WIDTH;
-
+	increment = clamp(increment, -NICE_WIDTH, NICE_WIDTH);
 	nice = task_nice(current) + increment;
-	if (nice < MIN_NICE)
-		nice = MIN_NICE;
-	if (nice > MAX_NICE)
-		nice = MAX_NICE;
 
+	nice = clamp_val(nice, MIN_NICE, MAX_NICE);
 	if (increment < 0 && !can_nice(current, nice))
 		return -EPERM;

Signature · 1 line

1.8.2.1

Steven Rostedt

maintainer

2014-03-19 02:16 UTC #9 lore raw

Re: [PATCH 3/3] sched: Use clamp() and clamp_val() to make it more readable.

Quoted 14 lines · On Wed, 12 Mar 2014 18:26:44 +0800 Dongsheng Yang <[email protected]>…

As Kees suggested, I use clamp() function to replace the if and else branch, making it more readable and modular.
Suggested-by Kees Cook <[email protected]>
Signed-off-by Dongsheng Yang <[email protected]>
---
 kernel/sched/core.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)
kernel/sched/core.c +0 -0
index 259ab85..85f4231 100644

kernel/sched/core.c SYSCALL_DEFINE1(nice, int, increment) · line 3075

Quoted 8 lines

kernel/sched/core.c +0 -5

@@ -3068,17 +3068,10 @@ SYSCALL_DEFINE1(nice, int, increment)
 	 * We don't have to worry. Conceptually one call occurs first
 	 * and we have a single winner.
 	 */
-	if (increment < -NICE_WIDTH)
-		increment = -NICE_WIDTH;
-	if (increment > NICE_WIDTH)
-		increment = NICE_WIDTH;
-

Patch 2 and 3 need to be merged into a single patch.

-- Steve

Quoted 11 lines

kernel/sched/core.c +2 -4

+	increment = clamp(increment, -NICE_WIDTH, NICE_WIDTH);
 	nice = task_nice(current) + increment;
-	if (nice < MIN_NICE)
-		nice = MIN_NICE;
-	if (nice > MAX_NICE)
-		nice = MAX_NICE;
 
+	nice = clamp_val(nice, MIN_NICE, MAX_NICE);
 	if (increment < 0 && !can_nice(current, nice))
 		return -EPERM;

Dongsheng Yang

author

2014-03-19 02:21 UTC #10 lore raw

Re: [PATCH 3/3] sched: Use clamp() and clamp_val() to make it more readable.

Quoted 12 lines · Steven Rostedt

Quoted 9 lines · On Wed, 12 Mar 2014 18:26:44 +0800 Dongsheng Yang <[email protected]>…
As Kees suggested, I use clamp() function to replace the if and else branch, making it more readable and modular.
Suggested-by Kees Cook <[email protected]>
Signed-off-by Dongsheng Yang <[email protected]>
---
  kernel/sched/core.c | 11 ++---------
  1 file changed, 2 insertions(+), 9 deletions(-)

Quoted 14 lines

kernel/sched/core.c SYSCALL_DEFINE1(nice, int, increment) · line 3075

Quoted 13 lines

kernel/sched/core.c +0 -5

index 259ab85..85f4231 100644
@@ -3068,17 +3068,10 @@ SYSCALL_DEFINE1(nice, int, increment)
  	 * We don't have to worry. Conceptually one call occurs first
  	 * and we have a single winner.
  	 */
-	if (increment < -NICE_WIDTH)
-		increment = -NICE_WIDTH;
-	if (increment > NICE_WIDTH)
-		increment = NICE_WIDTH;
-

Patch 2 and 3 need to be merged into a single patch.

Okey, I will squash them into one.

Thanx

Quoted 15 lines

-- Steve

Quoted 11 lines

kernel/sched/core.c +2 -4

+	increment = clamp(increment, -NICE_WIDTH, NICE_WIDTH);
  	nice = task_nice(current) + increment;
-	if (nice < MIN_NICE)
-		nice = MIN_NICE;
-	if (nice > MAX_NICE)
-		nice = MAX_NICE;
 
+	nice = clamp_val(nice, MIN_NICE, MAX_NICE);
  	if (increment < 0 && !can_nice(current, nice))
  		return -EPERM;