All of lore.kernel.org
 help / color / mirror / Atom feed
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.

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.