public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] tracing: Use inline task_nice() to get rid of an open coded implementation of it.
@ 2014-03-05 12:36 Dongsheng Yang
  2014-03-05 12:36 ` [PATCH 2/3] sched: Use clamp() and clamp_val() to make it more readable Dongsheng Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Dongsheng Yang @ 2014-03-05 12:36 UTC (permalink / raw)
  To: linux-kernel; +Cc: fweisbec, rostedt, peterz, mingo, 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 <yangds.fnst@cn.fujitsu.com>
cc: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 815c878..dba0e3d 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -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;
 
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] sched: Use clamp() and clamp_val() to make it more readable.
  2014-03-05 12:36 [PATCH 1/3] tracing: Use inline task_nice() to get rid of an open coded implementation of it Dongsheng Yang
@ 2014-03-05 12:36 ` Dongsheng Yang
  2014-03-05 15:17   ` Joe Perches
  2014-03-05 12:36 ` [PATCH 3/3] sched/prio: Replace hardcoding of 40 with NICE_WIDTH Dongsheng Yang
  2014-03-11  5:04 ` [PATCH 1/3] tracing: Use inline task_nice() to get rid of an open coded implementation of it Dongsheng Yang
  2 siblings, 1 reply; 11+ messages in thread
From: Dongsheng Yang @ 2014-03-05 12:36 UTC (permalink / raw)
  To: linux-kernel; +Cc: fweisbec, rostedt, peterz, mingo, 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 <keescook@chromium.org>
Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 kernel/sched/core.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ee8004c..5273037 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3070,17 +3070,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;
-
+	increment = clamp(increment, -40, 40);
 	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;
 
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] sched/prio: Replace hardcoding of 40 with NICE_WIDTH.
  2014-03-05 12:36 [PATCH 1/3] tracing: Use inline task_nice() to get rid of an open coded implementation of it Dongsheng Yang
  2014-03-05 12:36 ` [PATCH 2/3] sched: Use clamp() and clamp_val() to make it more readable Dongsheng Yang
@ 2014-03-05 12:36 ` Dongsheng Yang
  2014-03-10 22:23   ` [PATCH] kernel/sys: Replace hardcoding of 20 with MAX_NICE + 1 Joe Perches
  2014-03-11  5:04 ` [PATCH 1/3] tracing: Use inline task_nice() to get rid of an open coded implementation of it Dongsheng Yang
  2 siblings, 1 reply; 11+ messages in thread
From: Dongsheng Yang @ 2014-03-05 12:36 UTC (permalink / raw)
  To: linux-kernel; +Cc: fweisbec, rostedt, peterz, mingo, Dongsheng Yang

Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
---
 kernel/sched/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 5273037..7ed08ad 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3070,7 +3070,7 @@ SYSCALL_DEFINE1(nice, int, increment)
 	 * We don't have to worry. Conceptually one call occurs first
 	 * and we have a single winner.
 	 */
-	increment = clamp(increment, -40, 40);
+	increment = clamp(increment, -NICE_WIDTH, NICE_WIDTH);
 	nice = task_nice(current) + increment;
 
 	nice = clamp_val(nice, MIN_NICE, MAX_NICE);
-- 
1.8.2.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] sched: Use clamp() and clamp_val() to make it more readable.
  2014-03-05 12:36 ` [PATCH 2/3] sched: Use clamp() and clamp_val() to make it more readable Dongsheng Yang
@ 2014-03-05 15:17   ` Joe Perches
  2014-03-05 15:26     ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2014-03-05 15:17 UTC (permalink / raw)
  To: Dongsheng Yang; +Cc: linux-kernel, fweisbec, rostedt, peterz, mingo, Kees Cook

On Wed, 2014-03-05 at 20:36 +0800, Dongsheng Yang wrote:
> As Kees suggested, I use clamp() function to replace the if and
> else branch, making it more readable and modular.
> 
> Suggested-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
[]
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
[]
> @@ -3070,17 +3070,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;
> -
> +	increment = clamp(increment, -40, 40);

Maybe:

	increment = clamp(increment, -(NICE_MAX - NICE_MIN + 1), 
				   , NICE_MAX - NICE_MIN + 1)

or add yet another define like #define NICE_RANGE
or #define NICE_MAX_INCREMENT


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] sched: Use clamp() and clamp_val() to make it more readable.
  2014-03-05 15:17   ` Joe Perches
@ 2014-03-05 15:26     ` Steven Rostedt
  2014-03-05 15:29       ` Joe Perches
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2014-03-05 15:26 UTC (permalink / raw)
  To: Joe Perches
  Cc: Dongsheng Yang, linux-kernel, fweisbec, peterz, mingo, Kees Cook

On Wed, 05 Mar 2014 07:17:36 -0800
Joe Perches <joe@perches.com> wrote:

> On Wed, 2014-03-05 at 20:36 +0800, Dongsheng Yang wrote:
> > As Kees suggested, I use clamp() function to replace the if and
> > else branch, making it more readable and modular.
> > 
> > Suggested-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> []
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> []
> > @@ -3070,17 +3070,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;
> > -
> > +	increment = clamp(increment, -40, 40);
> 
> Maybe:
> 
> 	increment = clamp(increment, -(NICE_MAX - NICE_MIN + 1), 
> 				   , NICE_MAX - NICE_MIN + 1)

Ug, that's much harder to read.

> 
> or add yet another define like #define NICE_RANGE
> or #define NICE_MAX_INCREMENT

Sure, if there's a NICE_MAX_INC == 40, then we could do:

	increment = clamp(increment, -NICE_MAX_INC, NICE_MAX_INC);

-- Steve

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] sched: Use clamp() and clamp_val() to make it more readable.
  2014-03-05 15:26     ` Steven Rostedt
@ 2014-03-05 15:29       ` Joe Perches
  0 siblings, 0 replies; 11+ messages in thread
From: Joe Perches @ 2014-03-05 15:29 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Dongsheng Yang, linux-kernel, fweisbec, peterz, mingo, Kees Cook

On Wed, 2014-03-05 at 10:26 -0500, Steven Rostedt wrote:
> On Wed, 05 Mar 2014 07:17:36 -0800 Joe Perches <joe@perches.com> wrote:
[]
> > or add yet another define like #define NICE_RANGE
> > or #define NICE_MAX_INCREMENT
> 
> Sure, if there's a NICE_MAX_INC == 40, then we could do:
> 
> 	increment = clamp(increment, -NICE_MAX_INC, NICE_MAX_INC);

Apologies for the noise.
He did that in the next patch.




^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] kernel/sys: Replace hardcoding of 20 with MAX_NICE + 1
  2014-03-05 12:36 ` [PATCH 3/3] sched/prio: Replace hardcoding of 40 with NICE_WIDTH Dongsheng Yang
@ 2014-03-10 22:23   ` Joe Perches
  2014-03-11  1:17     ` Dongsheng Yang
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2014-03-10 22:23 UTC (permalink / raw)
  To: Dongsheng Yang; +Cc: linux-kernel, fweisbec, rostedt, peterz, mingo

Convert the use of 20 to NICE_MAX + 1.

Reduce the indent the switch case labels while there.

git diff -w shows 3 lines changed and a /* fall-through */ comment added

  $ git diff -w -U0 kernel/sys.c
  @@ -253 +253 @@ SYSCALL_DEFINE2(getpriority, int, which, int, who)
  -                               niceval = 20 - task_nice(p);
  +                       niceval = MAX_NICE - task_nice(p) + 1;
  @@ -264 +264 @@ SYSCALL_DEFINE2(getpriority, int, which, int, who)
  -                               niceval = 20 - task_nice(p);
  +                       niceval = MAX_NICE - task_nice(p) + 1;
  @@ -280 +280 @@ SYSCALL_DEFINE2(getpriority, int, which, int, who)
  -                                       niceval = 20 - task_nice(p);
  +                               niceval = MAX_NICE - task_nice(p) + 1;
  @@ -1558 +1558 @@ static void k_getrusage(struct task_struct *p, int who, struct rusage *r)
  -
  +               /* fall-through */

Signed-off-by: Joe Perches <joe@perches.com>
---
 kernel/sys.c | 206 +++++++++++++++++++++++++++++------------------------------
 1 file changed, 103 insertions(+), 103 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index fba0f29..0bd5fe9 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -182,39 +182,39 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
 	rcu_read_lock();
 	read_lock(&tasklist_lock);
 	switch (which) {
-		case PRIO_PROCESS:
-			if (who)
-				p = find_task_by_vpid(who);
-			else
-				p = current;
-			if (p)
-				error = set_one_prio(p, niceval, error);
-			break;
-		case PRIO_PGRP:
-			if (who)
-				pgrp = find_vpid(who);
-			else
-				pgrp = task_pgrp(current);
-			do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
+	case PRIO_PROCESS:
+		if (who)
+			p = find_task_by_vpid(who);
+		else
+			p = current;
+		if (p)
+			error = set_one_prio(p, niceval, error);
+		break;
+	case PRIO_PGRP:
+		if (who)
+			pgrp = find_vpid(who);
+		else
+			pgrp = task_pgrp(current);
+		do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
+			error = set_one_prio(p, niceval, error);
+		} while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
+		break;
+	case PRIO_USER:
+		uid = make_kuid(cred->user_ns, who);
+		user = cred->user;
+		if (!who)
+			uid = cred->uid;
+		else if (!uid_eq(uid, cred->uid) &&
+			 !(user = find_user(uid)))
+			goto out_unlock;	/* No processes for this user */
+
+		do_each_thread(g, p) {
+			if (uid_eq(task_uid(p), uid))
 				error = set_one_prio(p, niceval, error);
-			} while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
-			break;
-		case PRIO_USER:
-			uid = make_kuid(cred->user_ns, who);
-			user = cred->user;
-			if (!who)
-				uid = cred->uid;
-			else if (!uid_eq(uid, cred->uid) &&
-				 !(user = find_user(uid)))
-				goto out_unlock;	/* No processes for this user */
-
-			do_each_thread(g, p) {
-				if (uid_eq(task_uid(p), uid))
-					error = set_one_prio(p, niceval, error);
-			} while_each_thread(g, p);
-			if (!uid_eq(uid, cred->uid))
-				free_uid(user);		/* For find_user() */
-			break;
+		} while_each_thread(g, p);
+		if (!uid_eq(uid, cred->uid))
+			free_uid(user);		/* For find_user() */
+		break;
 	}
 out_unlock:
 	read_unlock(&tasklist_lock);
@@ -244,47 +244,47 @@ SYSCALL_DEFINE2(getpriority, int, which, int, who)
 	rcu_read_lock();
 	read_lock(&tasklist_lock);
 	switch (which) {
-		case PRIO_PROCESS:
-			if (who)
-				p = find_task_by_vpid(who);
-			else
-				p = current;
-			if (p) {
-				niceval = 20 - task_nice(p);
+	case PRIO_PROCESS:
+		if (who)
+			p = find_task_by_vpid(who);
+		else
+			p = current;
+		if (p) {
+			niceval = MAX_NICE - task_nice(p) + 1;
+			if (niceval > retval)
+				retval = niceval;
+		}
+		break;
+	case PRIO_PGRP:
+		if (who)
+			pgrp = find_vpid(who);
+		else
+			pgrp = task_pgrp(current);
+		do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
+			niceval = MAX_NICE - task_nice(p) + 1;
+			if (niceval > retval)
+				retval = niceval;
+		} while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
+		break;
+	case PRIO_USER:
+		uid = make_kuid(cred->user_ns, who);
+		user = cred->user;
+		if (!who)
+			uid = cred->uid;
+		else if (!uid_eq(uid, cred->uid) &&
+			 !(user = find_user(uid)))
+			goto out_unlock;	/* No processes for this user */
+
+		do_each_thread(g, p) {
+			if (uid_eq(task_uid(p), uid)) {
+				niceval = MAX_NICE - task_nice(p) + 1;
 				if (niceval > retval)
 					retval = niceval;
 			}
-			break;
-		case PRIO_PGRP:
-			if (who)
-				pgrp = find_vpid(who);
-			else
-				pgrp = task_pgrp(current);
-			do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
-				niceval = 20 - task_nice(p);
-				if (niceval > retval)
-					retval = niceval;
-			} while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
-			break;
-		case PRIO_USER:
-			uid = make_kuid(cred->user_ns, who);
-			user = cred->user;
-			if (!who)
-				uid = cred->uid;
-			else if (!uid_eq(uid, cred->uid) &&
-				 !(user = find_user(uid)))
-				goto out_unlock;	/* No processes for this user */
-
-			do_each_thread(g, p) {
-				if (uid_eq(task_uid(p), uid)) {
-					niceval = 20 - task_nice(p);
-					if (niceval > retval)
-						retval = niceval;
-				}
-			} while_each_thread(g, p);
-			if (!uid_eq(uid, cred->uid))
-				free_uid(user);		/* for find_user() */
-			break;
+		} while_each_thread(g, p);
+		if (!uid_eq(uid, cred->uid))
+			free_uid(user);		/* for find_user() */
+		break;
 	}
 out_unlock:
 	read_unlock(&tasklist_lock);
@@ -1541,41 +1541,41 @@ static void k_getrusage(struct task_struct *p, int who, struct rusage *r)
 		return;
 
 	switch (who) {
-		case RUSAGE_BOTH:
-		case RUSAGE_CHILDREN:
-			utime = p->signal->cutime;
-			stime = p->signal->cstime;
-			r->ru_nvcsw = p->signal->cnvcsw;
-			r->ru_nivcsw = p->signal->cnivcsw;
-			r->ru_minflt = p->signal->cmin_flt;
-			r->ru_majflt = p->signal->cmaj_flt;
-			r->ru_inblock = p->signal->cinblock;
-			r->ru_oublock = p->signal->coublock;
-			maxrss = p->signal->cmaxrss;
-
-			if (who == RUSAGE_CHILDREN)
-				break;
-
-		case RUSAGE_SELF:
-			thread_group_cputime_adjusted(p, &tgutime, &tgstime);
-			utime += tgutime;
-			stime += tgstime;
-			r->ru_nvcsw += p->signal->nvcsw;
-			r->ru_nivcsw += p->signal->nivcsw;
-			r->ru_minflt += p->signal->min_flt;
-			r->ru_majflt += p->signal->maj_flt;
-			r->ru_inblock += p->signal->inblock;
-			r->ru_oublock += p->signal->oublock;
-			if (maxrss < p->signal->maxrss)
-				maxrss = p->signal->maxrss;
-			t = p;
-			do {
-				accumulate_thread_rusage(t, r);
-			} while_each_thread(p, t);
+	case RUSAGE_BOTH:
+	case RUSAGE_CHILDREN:
+		utime = p->signal->cutime;
+		stime = p->signal->cstime;
+		r->ru_nvcsw = p->signal->cnvcsw;
+		r->ru_nivcsw = p->signal->cnivcsw;
+		r->ru_minflt = p->signal->cmin_flt;
+		r->ru_majflt = p->signal->cmaj_flt;
+		r->ru_inblock = p->signal->cinblock;
+		r->ru_oublock = p->signal->coublock;
+		maxrss = p->signal->cmaxrss;
+
+		if (who == RUSAGE_CHILDREN)
 			break;
+		/* fall-through */
+	case RUSAGE_SELF:
+		thread_group_cputime_adjusted(p, &tgutime, &tgstime);
+		utime += tgutime;
+		stime += tgstime;
+		r->ru_nvcsw += p->signal->nvcsw;
+		r->ru_nivcsw += p->signal->nivcsw;
+		r->ru_minflt += p->signal->min_flt;
+		r->ru_majflt += p->signal->maj_flt;
+		r->ru_inblock += p->signal->inblock;
+		r->ru_oublock += p->signal->oublock;
+		if (maxrss < p->signal->maxrss)
+			maxrss = p->signal->maxrss;
+		t = p;
+		do {
+			accumulate_thread_rusage(t, r);
+		} while_each_thread(p, t);
+		break;
 
-		default:
-			BUG();
+	default:
+		BUG();
 	}
 	unlock_task_sighand(p, &flags);
 



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] kernel/sys: Replace hardcoding of 20 with MAX_NICE + 1
  2014-03-10 22:23   ` [PATCH] kernel/sys: Replace hardcoding of 20 with MAX_NICE + 1 Joe Perches
@ 2014-03-11  1:17     ` Dongsheng Yang
  2014-03-11  2:21       ` Joe Perches
  0 siblings, 1 reply; 11+ messages in thread
From: Dongsheng Yang @ 2014-03-11  1:17 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, fweisbec, rostedt, peterz, mingo

Hi Joe,

On 03/11/2014 06:23 AM, Joe Perches wrote:
> Convert the use of 20 to NICE_MAX + 1.

What about adding a macro in prio.h to convert nice value [19,-20] to 
rlimit
style value [1,40]?

It seems that it will be used in several places.

- Dongsheng
>
> Reduce the indent the switch case labels while there.
>
> git diff -w shows 3 lines changed and a /* fall-through */ comment added
>
>    $ git diff -w -U0 kernel/sys.c
>    @@ -253 +253 @@ SYSCALL_DEFINE2(getpriority, int, which, int, who)
>    -                               niceval = 20 - task_nice(p);
>    +                       niceval = MAX_NICE - task_nice(p) + 1;
>    @@ -264 +264 @@ SYSCALL_DEFINE2(getpriority, int, which, int, who)
>    -                               niceval = 20 - task_nice(p);
>    +                       niceval = MAX_NICE - task_nice(p) + 1;
>    @@ -280 +280 @@ SYSCALL_DEFINE2(getpriority, int, which, int, who)
>    -                                       niceval = 20 - task_nice(p);
>    +                               niceval = MAX_NICE - task_nice(p) + 1;
>    @@ -1558 +1558 @@ static void k_getrusage(struct task_struct *p, int who, struct rusage *r)
>    -
>    +               /* fall-through */
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>   kernel/sys.c | 206 +++++++++++++++++++++++++++++------------------------------
>   1 file changed, 103 insertions(+), 103 deletions(-)
>
> diff --git a/kernel/sys.c b/kernel/sys.c
> index fba0f29..0bd5fe9 100644
> --- a/kernel/sys.c
> +++ b/kernel/sys.c
> @@ -182,39 +182,39 @@ SYSCALL_DEFINE3(setpriority, int, which, int, who, int, niceval)
>   	rcu_read_lock();
>   	read_lock(&tasklist_lock);
>   	switch (which) {
> -		case PRIO_PROCESS:
> -			if (who)
> -				p = find_task_by_vpid(who);
> -			else
> -				p = current;
> -			if (p)
> -				error = set_one_prio(p, niceval, error);
> -			break;
> -		case PRIO_PGRP:
> -			if (who)
> -				pgrp = find_vpid(who);
> -			else
> -				pgrp = task_pgrp(current);
> -			do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
> +	case PRIO_PROCESS:
> +		if (who)
> +			p = find_task_by_vpid(who);
> +		else
> +			p = current;
> +		if (p)
> +			error = set_one_prio(p, niceval, error);
> +		break;
> +	case PRIO_PGRP:
> +		if (who)
> +			pgrp = find_vpid(who);
> +		else
> +			pgrp = task_pgrp(current);
> +		do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
> +			error = set_one_prio(p, niceval, error);
> +		} while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
> +		break;
> +	case PRIO_USER:
> +		uid = make_kuid(cred->user_ns, who);
> +		user = cred->user;
> +		if (!who)
> +			uid = cred->uid;
> +		else if (!uid_eq(uid, cred->uid) &&
> +			 !(user = find_user(uid)))
> +			goto out_unlock;	/* No processes for this user */
> +
> +		do_each_thread(g, p) {
> +			if (uid_eq(task_uid(p), uid))
>   				error = set_one_prio(p, niceval, error);
> -			} while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
> -			break;
> -		case PRIO_USER:
> -			uid = make_kuid(cred->user_ns, who);
> -			user = cred->user;
> -			if (!who)
> -				uid = cred->uid;
> -			else if (!uid_eq(uid, cred->uid) &&
> -				 !(user = find_user(uid)))
> -				goto out_unlock;	/* No processes for this user */
> -
> -			do_each_thread(g, p) {
> -				if (uid_eq(task_uid(p), uid))
> -					error = set_one_prio(p, niceval, error);
> -			} while_each_thread(g, p);
> -			if (!uid_eq(uid, cred->uid))
> -				free_uid(user);		/* For find_user() */
> -			break;
> +		} while_each_thread(g, p);
> +		if (!uid_eq(uid, cred->uid))
> +			free_uid(user);		/* For find_user() */
> +		break;
>   	}
>   out_unlock:
>   	read_unlock(&tasklist_lock);
> @@ -244,47 +244,47 @@ SYSCALL_DEFINE2(getpriority, int, which, int, who)
>   	rcu_read_lock();
>   	read_lock(&tasklist_lock);
>   	switch (which) {
> -		case PRIO_PROCESS:
> -			if (who)
> -				p = find_task_by_vpid(who);
> -			else
> -				p = current;
> -			if (p) {
> -				niceval = 20 - task_nice(p);
> +	case PRIO_PROCESS:
> +		if (who)
> +			p = find_task_by_vpid(who);
> +		else
> +			p = current;
> +		if (p) {
> +			niceval = MAX_NICE - task_nice(p) + 1;
> +			if (niceval > retval)
> +				retval = niceval;
> +		}
> +		break;
> +	case PRIO_PGRP:
> +		if (who)
> +			pgrp = find_vpid(who);
> +		else
> +			pgrp = task_pgrp(current);
> +		do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
> +			niceval = MAX_NICE - task_nice(p) + 1;
> +			if (niceval > retval)
> +				retval = niceval;
> +		} while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
> +		break;
> +	case PRIO_USER:
> +		uid = make_kuid(cred->user_ns, who);
> +		user = cred->user;
> +		if (!who)
> +			uid = cred->uid;
> +		else if (!uid_eq(uid, cred->uid) &&
> +			 !(user = find_user(uid)))
> +			goto out_unlock;	/* No processes for this user */
> +
> +		do_each_thread(g, p) {
> +			if (uid_eq(task_uid(p), uid)) {
> +				niceval = MAX_NICE - task_nice(p) + 1;
>   				if (niceval > retval)
>   					retval = niceval;
>   			}
> -			break;
> -		case PRIO_PGRP:
> -			if (who)
> -				pgrp = find_vpid(who);
> -			else
> -				pgrp = task_pgrp(current);
> -			do_each_pid_thread(pgrp, PIDTYPE_PGID, p) {
> -				niceval = 20 - task_nice(p);
> -				if (niceval > retval)
> -					retval = niceval;
> -			} while_each_pid_thread(pgrp, PIDTYPE_PGID, p);
> -			break;
> -		case PRIO_USER:
> -			uid = make_kuid(cred->user_ns, who);
> -			user = cred->user;
> -			if (!who)
> -				uid = cred->uid;
> -			else if (!uid_eq(uid, cred->uid) &&
> -				 !(user = find_user(uid)))
> -				goto out_unlock;	/* No processes for this user */
> -
> -			do_each_thread(g, p) {
> -				if (uid_eq(task_uid(p), uid)) {
> -					niceval = 20 - task_nice(p);
> -					if (niceval > retval)
> -						retval = niceval;
> -				}
> -			} while_each_thread(g, p);
> -			if (!uid_eq(uid, cred->uid))
> -				free_uid(user);		/* for find_user() */
> -			break;
> +		} while_each_thread(g, p);
> +		if (!uid_eq(uid, cred->uid))
> +			free_uid(user);		/* for find_user() */
> +		break;
>   	}
>   out_unlock:
>   	read_unlock(&tasklist_lock);
> @@ -1541,41 +1541,41 @@ static void k_getrusage(struct task_struct *p, int who, struct rusage *r)
>   		return;
>   
>   	switch (who) {
> -		case RUSAGE_BOTH:
> -		case RUSAGE_CHILDREN:
> -			utime = p->signal->cutime;
> -			stime = p->signal->cstime;
> -			r->ru_nvcsw = p->signal->cnvcsw;
> -			r->ru_nivcsw = p->signal->cnivcsw;
> -			r->ru_minflt = p->signal->cmin_flt;
> -			r->ru_majflt = p->signal->cmaj_flt;
> -			r->ru_inblock = p->signal->cinblock;
> -			r->ru_oublock = p->signal->coublock;
> -			maxrss = p->signal->cmaxrss;
> -
> -			if (who == RUSAGE_CHILDREN)
> -				break;
> -
> -		case RUSAGE_SELF:
> -			thread_group_cputime_adjusted(p, &tgutime, &tgstime);
> -			utime += tgutime;
> -			stime += tgstime;
> -			r->ru_nvcsw += p->signal->nvcsw;
> -			r->ru_nivcsw += p->signal->nivcsw;
> -			r->ru_minflt += p->signal->min_flt;
> -			r->ru_majflt += p->signal->maj_flt;
> -			r->ru_inblock += p->signal->inblock;
> -			r->ru_oublock += p->signal->oublock;
> -			if (maxrss < p->signal->maxrss)
> -				maxrss = p->signal->maxrss;
> -			t = p;
> -			do {
> -				accumulate_thread_rusage(t, r);
> -			} while_each_thread(p, t);
> +	case RUSAGE_BOTH:
> +	case RUSAGE_CHILDREN:
> +		utime = p->signal->cutime;
> +		stime = p->signal->cstime;
> +		r->ru_nvcsw = p->signal->cnvcsw;
> +		r->ru_nivcsw = p->signal->cnivcsw;
> +		r->ru_minflt = p->signal->cmin_flt;
> +		r->ru_majflt = p->signal->cmaj_flt;
> +		r->ru_inblock = p->signal->cinblock;
> +		r->ru_oublock = p->signal->coublock;
> +		maxrss = p->signal->cmaxrss;
> +
> +		if (who == RUSAGE_CHILDREN)
>   			break;
> +		/* fall-through */
> +	case RUSAGE_SELF:
> +		thread_group_cputime_adjusted(p, &tgutime, &tgstime);
> +		utime += tgutime;
> +		stime += tgstime;
> +		r->ru_nvcsw += p->signal->nvcsw;
> +		r->ru_nivcsw += p->signal->nivcsw;
> +		r->ru_minflt += p->signal->min_flt;
> +		r->ru_majflt += p->signal->maj_flt;
> +		r->ru_inblock += p->signal->inblock;
> +		r->ru_oublock += p->signal->oublock;
> +		if (maxrss < p->signal->maxrss)
> +			maxrss = p->signal->maxrss;
> +		t = p;
> +		do {
> +			accumulate_thread_rusage(t, r);
> +		} while_each_thread(p, t);
> +		break;
>   
> -		default:
> -			BUG();
> +	default:
> +		BUG();
>   	}
>   	unlock_task_sighand(p, &flags);
>   
>
>
> --
> 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/
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kernel/sys: Replace hardcoding of 20 with MAX_NICE + 1
  2014-03-11  1:17     ` Dongsheng Yang
@ 2014-03-11  2:21       ` Joe Perches
  2014-03-11  2:35         ` Dongsheng Yang
  0 siblings, 1 reply; 11+ messages in thread
From: Joe Perches @ 2014-03-11  2:21 UTC (permalink / raw)
  To: Dongsheng Yang; +Cc: linux-kernel, fweisbec, rostedt, peterz, mingo

On Tue, 2014-03-11 at 09:17 +0800, Dongsheng Yang wrote:
> Hi Joe,
> 
> On 03/11/2014 06:23 AM, Joe Perches wrote:
> > Convert the use of 20 to NICE_MAX + 1.
> 
> What about adding a macro in prio.h to convert nice value [19,-20] to 
> rlimit
> style value [1,40]?
> 
> It seems that it will be used in several places.

Fine by me.  knock yourself out.

You might also consider changing these:

$ git grep -n -w -E "19|20"|grep -P "_nice" -i
drivers/block/loop.c:551:	set_user_nice(current, -20);
drivers/block/nbd.c:536:	set_user_nice(current, -20);
drivers/block/pktcdvd.c:1466:	set_user_nice(current, -20);
drivers/char/ipmi/ipmi_si_intf.c:1001:	set_user_nice(current, 19);
drivers/s390/crypto/ap_bus.c:1806:	set_user_nice(current, 19);
drivers/scsi/bnx2fc/bnx2fc_fcoe.c:467:	set_user_nice(current, -20);
drivers/scsi/bnx2fc/bnx2fc_fcoe.c:605:	set_user_nice(current, -20);
drivers/scsi/bnx2i/bnx2i_hwi.c:1873:	set_user_nice(current, -20);
drivers/scsi/fcoe/fcoe.c:1875:	set_user_nice(current, -20);
drivers/scsi/ibmvscsi/ibmvfc.c:4518:	set_user_nice(current, -20);
drivers/scsi/ibmvscsi/ibmvscsi.c:2216:	set_user_nice(current, -20);
drivers/scsi/lpfc/lpfc_hbadisc.c:736:	set_user_nice(current, -20);
drivers/scsi/qla2xxx/qla_os.c:4759:	set_user_nice(current, -20);
drivers/staging/android/binder.c:439:	min_nice = 20 - current->signal->rlim[RLIMIT_NICE].rlim_cur;
drivers/staging/android/binder.c:444:	if (min_nice < 20)
drivers/staging/lustre/lustre/llite/lloop.c:410:	set_user_nice(current, -20);
fs/ocfs2/cluster/heartbeat.c:1110:	set_user_nice(current, -20);
include/linux/ioprio.h:55:	return (task_nice(task) + 20) / 5;
include/linux/sched/prio.h:4:#define MAX_NICE	19
include/linux/sched/prio.h:5:#define MIN_NICE	-20
kernel/locking/locktorture.c:219:	set_user_nice(current, 19);
kernel/sys.c:253:				niceval = 20 - task_nice(p);
kernel/sys.c:264:				niceval = 20 - task_nice(p);
kernel/sys.c:280:					niceval = 20 - task_nice(p);
kernel/workqueue.c:105:	RESCUER_NICE_LEVEL	= -20,
kernel/workqueue.c:106:	HIGHPRI_NICE_LEVEL	= -20,
tools/testing/selftests/mqueue/mq_perf_tests.c:532:	cur_nice = -20;



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] kernel/sys: Replace hardcoding of 20 with MAX_NICE + 1
  2014-03-11  2:21       ` Joe Perches
@ 2014-03-11  2:35         ` Dongsheng Yang
  0 siblings, 0 replies; 11+ messages in thread
From: Dongsheng Yang @ 2014-03-11  2:35 UTC (permalink / raw)
  To: Joe Perches; +Cc: linux-kernel, fweisbec, rostedt, peterz, mingo

On 03/11/2014 10:21 AM, Joe Perches wrote:
> On Tue, 2014-03-11 at 09:17 +0800, Dongsheng Yang wrote:
>> Hi Joe,
>>
>> On 03/11/2014 06:23 AM, Joe Perches wrote:
>>> Convert the use of 20 to NICE_MAX + 1.
>> What about adding a macro in prio.h to convert nice value [19,-20] to
>> rlimit
>> style value [1,40]?
>>
>> It seems that it will be used in several places.
> Fine by me.  knock yourself out.
>
> You might also consider changing these:

Yeah, my original plan is changing them from kernel/* to include/* then
to other subsystem such as driver, as I think I need to send them to 
different
ML and different maintainers. But I think it is time to complete it now,
and we should make these changes go into tip tree.

So I will send a patch set for them.

- Dongsheng
>
> $ git grep -n -w -E "19|20"|grep -P "_nice" -i
> drivers/block/loop.c:551:	set_user_nice(current, -20);
> drivers/block/nbd.c:536:	set_user_nice(current, -20);
> drivers/block/pktcdvd.c:1466:	set_user_nice(current, -20);
> drivers/char/ipmi/ipmi_si_intf.c:1001:	set_user_nice(current, 19);
> drivers/s390/crypto/ap_bus.c:1806:	set_user_nice(current, 19);
> drivers/scsi/bnx2fc/bnx2fc_fcoe.c:467:	set_user_nice(current, -20);
> drivers/scsi/bnx2fc/bnx2fc_fcoe.c:605:	set_user_nice(current, -20);
> drivers/scsi/bnx2i/bnx2i_hwi.c:1873:	set_user_nice(current, -20);
> drivers/scsi/fcoe/fcoe.c:1875:	set_user_nice(current, -20);
> drivers/scsi/ibmvscsi/ibmvfc.c:4518:	set_user_nice(current, -20);
> drivers/scsi/ibmvscsi/ibmvscsi.c:2216:	set_user_nice(current, -20);
> drivers/scsi/lpfc/lpfc_hbadisc.c:736:	set_user_nice(current, -20);
> drivers/scsi/qla2xxx/qla_os.c:4759:	set_user_nice(current, -20);
> drivers/staging/android/binder.c:439:	min_nice = 20 - current->signal->rlim[RLIMIT_NICE].rlim_cur;
> drivers/staging/android/binder.c:444:	if (min_nice < 20)
> drivers/staging/lustre/lustre/llite/lloop.c:410:	set_user_nice(current, -20);
> fs/ocfs2/cluster/heartbeat.c:1110:	set_user_nice(current, -20);
> include/linux/ioprio.h:55:	return (task_nice(task) + 20) / 5;
> include/linux/sched/prio.h:4:#define MAX_NICE	19
> include/linux/sched/prio.h:5:#define MIN_NICE	-20
> kernel/locking/locktorture.c:219:	set_user_nice(current, 19);
> kernel/sys.c:253:				niceval = 20 - task_nice(p);
> kernel/sys.c:264:				niceval = 20 - task_nice(p);
> kernel/sys.c:280:					niceval = 20 - task_nice(p);
> kernel/workqueue.c:105:	RESCUER_NICE_LEVEL	= -20,
> kernel/workqueue.c:106:	HIGHPRI_NICE_LEVEL	= -20,
> tools/testing/selftests/mqueue/mq_perf_tests.c:532:	cur_nice = -20;
>
>
> --
> 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/
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] tracing: Use inline task_nice() to get rid of an open coded implementation of it.
  2014-03-05 12:36 [PATCH 1/3] tracing: Use inline task_nice() to get rid of an open coded implementation of it Dongsheng Yang
  2014-03-05 12:36 ` [PATCH 2/3] sched: Use clamp() and clamp_val() to make it more readable Dongsheng Yang
  2014-03-05 12:36 ` [PATCH 3/3] sched/prio: Replace hardcoding of 40 with NICE_WIDTH Dongsheng Yang
@ 2014-03-11  5:04 ` Dongsheng Yang
  2 siblings, 0 replies; 11+ messages in thread
From: Dongsheng Yang @ 2014-03-11  5:04 UTC (permalink / raw)
  To: Dongsheng Yang; +Cc: linux-kernel, fweisbec, Steven Rostedt, peterz, mingo

Hi steve,
     Could you take these three patches if they looks fine to you??

Thanx

On 03/05/2014 08:36 PM, Dongsheng Yang wrote:
> Function task_nice() was reimplemented as inline function, we can use it here
> to replace the open coded implementation.
>
> Signed-off-by: Dongsheng Yang <yangds.fnst@cn.fujitsu.com>
> cc: Steven Rostedt <rostedt@goodmis.org>
> ---
>   kernel/trace/trace.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 815c878..dba0e3d 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -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;
>   


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-03-11  5:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-05 12:36 [PATCH 1/3] tracing: Use inline task_nice() to get rid of an open coded implementation of it Dongsheng Yang
2014-03-05 12:36 ` [PATCH 2/3] sched: Use clamp() and clamp_val() to make it more readable Dongsheng Yang
2014-03-05 15:17   ` Joe Perches
2014-03-05 15:26     ` Steven Rostedt
2014-03-05 15:29       ` Joe Perches
2014-03-05 12:36 ` [PATCH 3/3] sched/prio: Replace hardcoding of 40 with NICE_WIDTH Dongsheng Yang
2014-03-10 22:23   ` [PATCH] kernel/sys: Replace hardcoding of 20 with MAX_NICE + 1 Joe Perches
2014-03-11  1:17     ` Dongsheng Yang
2014-03-11  2:21       ` Joe Perches
2014-03-11  2:35         ` Dongsheng Yang
2014-03-11  5:04 ` [PATCH 1/3] tracing: Use inline task_nice() to get rid of an open coded implementation of it Dongsheng Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox