Re: [PATCH] sys_prctl(): simplify arg2 judgment when calling PR_SET_TIMERSLACK

From: Yang Xu
Date: Tue Jul 23 2019 - 04:11:36 EST

on 2019/07/23 15:23, Cyrill Gorcunov wrote:

On Tue, Jul 23, 2019 at 11:30:53AM +0800, Yang Xu wrote:
arg2 will never< 0, for its type is 'unsigned long'. So negative
judgment is meaningless.

Signed-off-by: Yang Xu<xuyang2018.jy@xxxxxxxxxxxxxx>
kernel/sys.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index 2969304c29fe..399457d26bef 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2372,11 +2372,11 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
error = current->timer_slack_ns;
- if (arg2<= 0)
+ if (arg2)
+ current->timer_slack_ns = arg2;
+ else
current->timer_slack_ns =
- else
- current->timer_slack_ns = arg2;
if (arg4 | arg5)
> From a glance it looks correct to me, but then...

1) you might simply compare with zero, iow if (arg2 == 0)
instead of changing 7 lines
Hi Cyril

Indeed. simply compare with zero might be better.

2) according to man page passing negative value should be acceptable,
though it never worked as expected. I've been grepping "git log"
for this file and the former API is coming from

commit 6976675d94042fbd446231d1bd8b7de71a980ada
Author: Arjan van de Ven<arjan@xxxxxxxxxxxxxxx>
Date: Mon Sep 1 15:52:40 2008 -0700

hrtimer: create a "timer_slack" field in the task struct

which is 11 years old by now. Nobody complained so far even when man
page is saying pretty obviously

PR_SET_TIMERSLACK (since Linux 2.6.28)
Each thread has two associated timer slack values: a "default"
value, and a "current" value. This operation sets the "current"
timer slack value for the calling thread. If the nanosecond
value supplied in arg2 is greater than zero, then the "current"
value is set to this value. If arg2 is less than or equal to
zero, the "current" timer slack is reset to the thread's
"default" timer slack value.

So i think to match the man page (and assuming that accepting negative value
has been supposed) we should rather do

if ((long)arg2< 0)
Looks correct. But if we set a ULONG_MAX(PR_GET_TIMERSLACK also limits ULONG_MAX)
value(about 4s) on 32bit machine, this code will think this value is a negative value and use default value.

I guess man page was written as "less than or equal to zero" because of this confusing code(arg2<=0, but arg2
is an unsinged long value).
I think we can change this man page and also add bounds value description.

Also, I found a patch about arg2 is an unsigned long value

commit 7fe5e04292e71af34ae171b88caa2a139e0b6125
Author: Chen Gang<gang.chen@xxxxxxxxxxx>
Date: Thu Feb 21 16:43:06 2013 -0800

sys_prctl(): arg2 is unsigned long which is never< 0

arg2 will never< 0, for its type is 'unsigned long'

Also, use the provided macros.

What do you think about it ?