All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nishanth Aravamudan <nacc@us.ibm.com>
To: janitor@sternwelten.at
Cc: netdev@oss.sgi.com, jgarzik@pobox.com,
	linux-kernel@vger.kernel.org, kernel-janitors@lists.osdl.org
Subject: [KJ] Re: [PATCH] Add ssleep_interruptible()
Date: Wed, 17 Nov 2004 01:30:59 +0000	[thread overview]
Message-ID: <20041117013059.GA4218@us.ibm.com> (raw)
In-Reply-To: <20041101200749.GF1730@us.ibm.com>

[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]

On Mon, Nov 01, 2004 at 12:07:49PM -0800, Nishanth Aravamudan wrote:
> Description: Adds ssleep_interruptible() to allow longer delays to occur
> in TASK_INTERRUPTIBLE, similarly to ssleep(). To be consistent with
> msleep_interruptible(), ssleep_interruptible() returns the remaining time
> left in the delay in terms of seconds. This required dividing the return
> value of msleep_interruptible() by 1000, thus a cast to (unsigned long)
> to prevent any floating point issues.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> --- 2.6.10-rc1-vanilla/include/linux/delay.h	2004-10-30 
> 15:34:03.000000000 -0700
> +++ 2.6.10-rc1/include/linux/delay.h	2004-11-01 12:06:11.000000000 -0800
> @@ -46,4 +46,9 @@ static inline void ssleep(unsigned int s
> 	msleep(seconds * 1000);
> }
> 
> +static inline unsigned long ssleep_interruptible(unsigned int seconds)
> +{
> +	return (unsigned long)(msleep_interruptible(seconds * 1000) / 1000);
> +}
> +
> #endif /* defined(_LINUX_DELAY_H) */

After a discussion on IRC, I believe it is pretty clear that this
function has serious issues. Mainly, that if I request a delay of 1
second, but msleep_interruptible() returns after 1 millisecond, then
ssleep_interruptible() will return 0, claiming the entire delay was
used (due to rounding).

Perhaps we should just be satisfied with milliseconds being the grossest
(in contrast to fine) measure of time, at least in terms of
interruptible delays. ssleep() is unaffected by this problem, of course.

Please revert this patch, if applied, as well as any of the other
patches I sent using ssleep_interruptible() [only a handful].

Thanks,
Nish

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
http://lists.osdl.org/mailman/listinfo/kernel-janitors

WARNING: multiple messages have this Message-ID (diff)
From: Nishanth Aravamudan <nacc@us.ibm.com>
To: janitor@sternwelten.at
Cc: netdev@oss.sgi.com, jgarzik@pobox.com,
	linux-kernel@vger.kernel.org, kernel-janitors@lists.osdl.org
Subject: Re: [PATCH] Add ssleep_interruptible()
Date: Tue, 16 Nov 2004 17:30:59 -0800	[thread overview]
Message-ID: <20041117013059.GA4218@us.ibm.com> (raw)
In-Reply-To: <20041101200749.GF1730@us.ibm.com>

On Mon, Nov 01, 2004 at 12:07:49PM -0800, Nishanth Aravamudan wrote:
> Description: Adds ssleep_interruptible() to allow longer delays to occur
> in TASK_INTERRUPTIBLE, similarly to ssleep(). To be consistent with
> msleep_interruptible(), ssleep_interruptible() returns the remaining time
> left in the delay in terms of seconds. This required dividing the return
> value of msleep_interruptible() by 1000, thus a cast to (unsigned long)
> to prevent any floating point issues.
> 
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
> 
> --- 2.6.10-rc1-vanilla/include/linux/delay.h	2004-10-30 
> 15:34:03.000000000 -0700
> +++ 2.6.10-rc1/include/linux/delay.h	2004-11-01 12:06:11.000000000 -0800
> @@ -46,4 +46,9 @@ static inline void ssleep(unsigned int s
> 	msleep(seconds * 1000);
> }
> 
> +static inline unsigned long ssleep_interruptible(unsigned int seconds)
> +{
> +	return (unsigned long)(msleep_interruptible(seconds * 1000) / 1000);
> +}
> +
> #endif /* defined(_LINUX_DELAY_H) */

After a discussion on IRC, I believe it is pretty clear that this
function has serious issues. Mainly, that if I request a delay of 1
second, but msleep_interruptible() returns after 1 millisecond, then
ssleep_interruptible() will return 0, claiming the entire delay was
used (due to rounding).

Perhaps we should just be satisfied with milliseconds being the grossest
(in contrast to fine) measure of time, at least in terms of
interruptible delays. ssleep() is unaffected by this problem, of course.

Please revert this patch, if applied, as well as any of the other
patches I sent using ssleep_interruptible() [only a handful].

Thanks,
Nish

  reply	other threads:[~2004-11-17  1:30 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-10-30 22:42 [patch 05/18] net/s2io: replace schedule_timeout() with msleep() janitor
2004-10-31 11:09 ` Jeff Garzik
2004-11-01 18:31   ` Nishanth Aravamudan
2004-11-01 18:03     ` Jeff Garzik
2004-11-01 19:14       ` Nishanth Aravamudan
2004-11-01 19:23   ` [KJ] [PATCH] net/s2io: replace schedule_timeout() with Nishanth Aravamudan
2004-11-01 20:20     ` [PATCH] net/s2io: replace schedule_timeout() with msleep()/ssleep_interruptible() Nishanth Aravamudan
2004-11-01 19:18 ` [KJ] [PATCH] Add ssleep_interruptible() Nishanth Aravamudan
2004-11-01 20:07   ` Nishanth Aravamudan
2004-11-17  1:30   ` Nishanth Aravamudan [this message]
2004-11-17  1:30     ` Nishanth Aravamudan
2004-11-22  2:48     ` [KJ] " Horms
2004-11-22  2:48       ` Horms
2004-11-22 17:19       ` [KJ] " Nishanth Aravamudan
2004-11-22 17:19         ` Nishanth Aravamudan
2004-11-24  2:01         ` [KJ] " Horms
2004-11-24  2:01           ` Horms
2004-11-24 18:16           ` [KJ] " Nishanth Aravamudan
2004-11-24 18:16             ` Nishanth Aravamudan

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=20041117013059.GA4218@us.ibm.com \
    --to=nacc@us.ibm.com \
    --cc=janitor@sternwelten.at \
    --cc=jgarzik@pobox.com \
    --cc=kernel-janitors@lists.osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@oss.sgi.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.