From: Jean Delvare <khali@linux-fr.org>
To: Imre Deak <imre.deak@intel.com>
Cc: linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Daniel Vetter <daniel.vetter@ffwll.ch>,
John Stultz <john.stultz@linaro.org>,
Ingo Molnar <mingo@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
"David S. Miller" <davem@davemloft.net>
Subject: Re: [PATCH 01/11] time: add *_to_jiffies_min helpers to guarantee a minimum duration
Date: Mon, 13 May 2013 09:29:04 +0200 [thread overview]
Message-ID: <20130513092904.3ba12f9d@endymion.delvare> (raw)
In-Reply-To: <1368188011-23661-1-git-send-email-imre.deak@intel.com>
Hi Imre,
On Fri, 10 May 2013 15:13:19 +0300, Imre Deak wrote:
> The *_to_jiffies(x) macros return a jiffy value, which if used as a
> delta to wait for a specific amount of time, may result in a wait-time
> that is less than x.
Are you sure? I have always considered that *_to_jiffies(x) macros
rounded up, and reading the code seems to confirm that:
/*
* Generic case - multiply, round and divide. (...)
*/
(...)
return (MSEC_TO_HZ_MUL32 * m + MSEC_TO_HZ_ADJ32)
>> MSEC_TO_HZ_SHR32;
What makes you think the resulting wait time can be less that requested?
If this really is the case then the proper way to address the issue is
to fix the original macros, not introducing new ones.
> Many callers already compensate for this by adding
> one to the returned value.
This is an assumption from you, and I am afraid it is wrong in many
cases. I see that Michal Kubecek already pointed out one case where it
was indeed wrong, and I am about to make a similar reply to another
post of yours.
> To document why we need to add one and to get
> rid of some code duplication add a helper that does the same.
I'm sorry but you can't call "+ 1" code duplication.
> Later patches will convert the currently open-coded call sites to use
> the new helpers.
>
> Also this can serve as a basis for auditing those users of *_to_jiffies
> that most likely do the wrong thing - for example set a timeout value of
> msecs_to_jiffies(1) - and converting them to use the new helpers.
You should be very, very careful before claiming that the code is wrong
and you're fixing it. It might as well be that the code is right but
you did not understand it, and you're actually breaking it. Or the code
was already wrong and you're making it worse ;)
As a summary, I don't like the idea, sorry.
--
Jean Delvare
next prev parent reply other threads:[~2013-05-13 7:29 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-10 12:13 Imre Deak
2013-05-10 12:13 ` [PATCH 02/11] sched: msleep: take msecs_to_jiffies_min into use Imre Deak
2013-05-10 12:13 ` [PATCH 03/11] drm/i915: " Imre Deak
2013-05-10 12:13 ` [PATCH 04/11] hwmon/lm63,lm90: " Imre Deak
2013-05-12 23:55 ` Guenter Roeck
2013-05-13 7:47 ` Jean Delvare
2013-05-13 11:56 ` Imre Deak
2013-05-13 12:23 ` Jean Delvare
2013-05-10 12:13 ` [PATCH 05/11] media/si4713-i2c: take usecs_to_jiffies_min " Imre Deak
2013-05-10 12:13 ` [PATCH 06/11] net/bonding: take msecs_to_jiffies_min " Imre Deak
2013-05-10 13:58 ` Michal Kubecek
2013-05-10 21:19 ` Imre Deak
2013-05-10 12:13 ` [PATCH 07/11] net/peak_pcmcia: " Imre Deak
2013-05-15 9:12 ` Marc Kleine-Budde
2013-05-15 11:45 ` Marc Kleine-Budde
2013-05-10 12:13 ` [PATCH 08/11] usb/isp116x-hcd: " Imre Deak
2013-05-10 12:13 ` [PATCH 09/11] net/sunrpc: " Imre Deak
2013-05-10 12:13 ` [PATCH 10/11] net/tipc: " Imre Deak
2013-05-10 12:13 ` [PATCH 11/11] sound/oxygen_io: " Imre Deak
2013-05-13 14:00 ` Takashi Iwai
2013-05-13 14:24 ` Imre Deak
2013-05-13 14:35 ` Takashi Iwai
2013-05-13 14:30 ` Clemens Ladisch
2013-05-10 12:24 ` [PATCH 01/11] time: add *_to_jiffies_min helpers to guarantee a minimum duration Daniel Vetter
2013-05-10 13:49 ` Imre Deak
2013-05-13 7:29 ` Jean Delvare [this message]
2013-05-13 11:27 ` Imre Deak
2013-05-13 12:28 ` Jean Delvare
2013-05-13 13:07 ` Imre Deak
2013-05-13 8:17 ` Jean Delvare
2013-05-13 12:01 ` Imre Deak
2013-05-14 14:48 ` [PATCH v2 0/8] add *_to_jiffies_timeout " Imre Deak
2013-05-14 14:48 ` [PATCH v2 1/8] time: " Imre Deak
2013-05-15 15:26 ` Arnd Bergmann
2013-05-15 17:56 ` Imre Deak
2013-05-17 13:35 ` Arnd Bergmann
2013-05-17 15:14 ` Imre Deak
2013-05-14 14:48 ` [PATCH v2 2/8] sched: msleep: take msecs_to_jiffies_timeout into use Imre Deak
2013-05-14 14:48 ` [PATCH v2 3/8] drm/i915: " Imre Deak
2013-05-14 14:48 ` [PATCH v2 4/8] media/si4713-i2c: take usecs_to_jiffies_timeout " Imre Deak
2013-05-14 17:45 ` edubezval
2013-05-14 14:48 ` [PATCH v2 5/8] usb/isp116x-hcd: take msecs_to_jiffies_timeout " Imre Deak
2013-05-14 14:48 ` [PATCH v2 6/8] net/sunrpc: " Imre Deak
2013-05-14 14:48 ` [PATCH v2 7/8] net/tipc: " Imre Deak
2013-05-14 14:48 ` [PATCH v2 8/8] sound/oxygen_io: " Imre Deak
2013-05-14 14:50 ` Takashi Iwai
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=20130513092904.3ba12f9d@endymion.delvare \
--to=khali@linux-fr.org \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=daniel.vetter@ffwll.ch \
--cc=davem@davemloft.net \
--cc=imre.deak@intel.com \
--cc=john.stultz@linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
/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
all inboxes | Powered by JetHome®