From: Cyrill Gorcunov <gorcunov@gmail.com>
To: Yang Xu <xuyang2018.jy@cn.fujitsu.com>
Cc: akpm@linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sys_prctl(): simplify arg2 judgment when calling PR_SET_TIMERSLACK
Date: Tue, 23 Jul 2019 12:48:09 +0300 [thread overview]
Message-ID: <20190723094809.GE4832@uranus.lan> (raw)
In-Reply-To: <5D36C11D.1070804@cn.fujitsu.com>
On Tue, Jul 23, 2019 at 04:11:09PM +0800, Yang Xu wrote:
> > 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@linux.intel.com>
> > 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.
OK, seems reasonable. I think we should use comparision with zero
and simply update a man page.
next prev parent reply other threads:[~2019-07-23 9:48 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-23 3:30 [PATCH] sys_prctl(): simplify arg2 judgment when calling PR_SET_TIMERSLACK Yang Xu
2019-07-23 7:23 ` Cyrill Gorcunov
2019-07-23 8:11 ` Yang Xu
2019-07-23 9:48 ` Cyrill Gorcunov [this message]
2019-07-24 2:11 ` [PATCH v2] sys_prctl(): remove unsigned comparision with less than zero Yang Xu
2019-07-24 6:56 ` Cyrill Gorcunov
2019-07-25 2:14 ` Andrew Morton
2019-07-25 3:10 ` Yang Xu
2019-07-30 8:48 ` Yang Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190723094809.GE4832@uranus.lan \
--to=gorcunov@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=xuyang2018.jy@cn.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox