* Re: [PATCH] pmac_cpufreq msleep cleanup/fixes [not found] <200410221906.i9MJ63Ai022889@hera.kernel.org> @ 2004-10-22 22:36 ` Benjamin Herrenschmidt 2004-10-22 22:53 ` Linus Torvalds 0 siblings, 1 reply; 7+ messages in thread From: Benjamin Herrenschmidt @ 2004-10-22 22:36 UTC (permalink / raw) To: Linux Kernel Mailing List; +Cc: Linus Torvalds, Paul Mackerras On Sat, 2004-10-23 at 04:11, Linux Kernel Mailing List wrote: > ChangeSet 1.2015, 2004/10/22 11:11:36-07:00, nacc@us.ibm.com > > [PATCH] pmac_cpufreq msleep cleanup/fixes > > Uses msleep() instead of schedule_timeout() to guarantee the task delays > as expected. Two of the changes are reworks of previous msleep() calls > which unnecessarily added a jiffy to the parameter. > > Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com> > Signed-off-by: Linus Torvalds <torvalds@osdl.org> Please revert that change until we have made absolutely sure that msleep(1) on a HZ=1000 machine will actually sleep at least 1ms, this is really not clear since it will end up doing schedule_timeout(1) which, afaik, will only guarantee to sleep up to the next jiffie, which can be a lot shorter than the actual duration of a jiffie. Ben. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pmac_cpufreq msleep cleanup/fixes 2004-10-22 22:36 ` [PATCH] pmac_cpufreq msleep cleanup/fixes Benjamin Herrenschmidt @ 2004-10-22 22:53 ` Linus Torvalds 2004-10-22 23:17 ` Benjamin Herrenschmidt 2004-10-25 0:19 ` [PATCH][RESEND] Fix msleep to sleep _at_least_ the requested amount Benjamin Herrenschmidt 0 siblings, 2 replies; 7+ messages in thread From: Linus Torvalds @ 2004-10-22 22:53 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Linux Kernel Mailing List, Paul Mackerras On Sat, 23 Oct 2004, Benjamin Herrenschmidt wrote: > > Please revert that change until we have made absolutely sure that msleep(1) > on a HZ=1000 machine will actually sleep at least 1ms, this is really not > clear since it will end up doing schedule_timeout(1) which, afaik, will > only guarantee to sleep up to the next jiffie, which can be a lot shorter > than the actual duration of a jiffie. In that case I'd much prefer to revert the whole previous "cleanup" as well, since it obviously isn't really. Having msleep(1 + jiffy_to_ms(1)); is just not a cleanup to me. Linus ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pmac_cpufreq msleep cleanup/fixes 2004-10-22 22:53 ` Linus Torvalds @ 2004-10-22 23:17 ` Benjamin Herrenschmidt 2004-10-22 23:43 ` Nishanth Aravamudan 2004-10-22 23:51 ` Linus Torvalds 2004-10-25 0:19 ` [PATCH][RESEND] Fix msleep to sleep _at_least_ the requested amount Benjamin Herrenschmidt 1 sibling, 2 replies; 7+ messages in thread From: Benjamin Herrenschmidt @ 2004-10-22 23:17 UTC (permalink / raw) To: Linus Torvalds; +Cc: Linux Kernel Mailing List, Paul Mackerras On Sat, 2004-10-23 at 08:53, Linus Torvalds wrote: > On Sat, 23 Oct 2004, Benjamin Herrenschmidt wrote: > > > > Please revert that change until we have made absolutely sure that msleep(1) > > on a HZ=1000 machine will actually sleep at least 1ms, this is really not > > clear since it will end up doing schedule_timeout(1) which, afaik, will > > only guarantee to sleep up to the next jiffie, which can be a lot shorter > > than the actual duration of a jiffie. > > In that case I'd much prefer to revert the whole previous "cleanup" as > well, since it obviously isn't really. Having > > msleep(1 + jiffy_to_ms(1)); > > is just not a cleanup to me. This wasn't a cleanup but a bug fix actually ... Oh well, I think we need to fix msleep() instead, what do you think ? If we keep Nishanth's latest cleanup and fix msleep to add +1 to the delay, that would work and potentially fix other users as well ... provided my theory is right in the first place and that schedule_timeout(1) will indeed only sleep until the next jiffy and not for at least one jiffy... What about something like this ? --- Makes sure msleep() sleeps at least the amount provided, since schedule_timeout() doesn't guarantee a full jiffy. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> ===== kernel/timer.c 1.100 vs edited ===== --- 1.100/kernel/timer.c 2004-10-19 19:40:28 +10:00 +++ edited/kernel/timer.c 2004-10-23 09:16:10 +10:00 @@ -1605,7 +1605,7 @@ */ void msleep(unsigned int msecs) { - unsigned long timeout = msecs_to_jiffies(msecs); + unsigned long timeout = msecs_to_jiffies(msecs) + 1; while (timeout) { set_current_state(TASK_UNINTERRUPTIBLE); @@ -1621,7 +1621,7 @@ */ unsigned long msleep_interruptible(unsigned int msecs) { - unsigned long timeout = msecs_to_jiffies(msecs); + unsigned long timeout = msecs_to_jiffies(msecs) + 1; while (timeout && !signal_pending(current)) { set_current_state(TASK_INTERRUPTIBLE); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pmac_cpufreq msleep cleanup/fixes 2004-10-22 23:17 ` Benjamin Herrenschmidt @ 2004-10-22 23:43 ` Nishanth Aravamudan 2004-10-23 0:33 ` Paul Mackerras 2004-10-22 23:51 ` Linus Torvalds 1 sibling, 1 reply; 7+ messages in thread From: Nishanth Aravamudan @ 2004-10-22 23:43 UTC (permalink / raw) To: Benjamin Herrenschmidt Cc: Linus Torvalds, Linux Kernel Mailing List, Paul Mackerras On Sat, Oct 23, 2004 at 09:17:33AM +1000, Benjamin Herrenschmidt wrote: > On Sat, 2004-10-23 at 08:53, Linus Torvalds wrote: > > On Sat, 23 Oct 2004, Benjamin Herrenschmidt wrote: > > > > > > Please revert that change until we have made absolutely sure that msleep(1) > > > on a HZ=1000 machine will actually sleep at least 1ms, this is really not > > > clear since it will end up doing schedule_timeout(1) which, afaik, will > > > only guarantee to sleep up to the next jiffie, which can be a lot shorter > > > than the actual duration of a jiffie. > > > > In that case I'd much prefer to revert the whole previous "cleanup" as > > well, since it obviously isn't really. Having > > > > msleep(1 + jiffy_to_ms(1)); > > > > is just not a cleanup to me. > > This wasn't a cleanup but a bug fix actually ... Oh well, I think we need > to fix msleep() instead, what do you think ? If we keep Nishanth's latest > cleanup and fix msleep to add +1 to the delay, that would work and potentially > fix other users as well ... provided my theory is right in the first place > and that schedule_timeout(1) will indeed only sleep until the next jiffy and > not for at least one jiffy... > > What about something like this ? > Looks good to me... And makes quite a bit of sense, after I thought about it. Does feel a little hacky, but I don't see a slicker way around the problem... Makes sure msleep() sleeps at least the amount provided, since schedule_timeout() doesn't guarantee a full jiffy. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Acked-by: Nishanth Aravamudan <nacc@us.ibm.com> ===== kernel/timer.c 1.100 vs edited ===== --- 1.100/kernel/timer.c 2004-10-19 19:40:28 +10:00 +++ edited/kernel/timer.c 2004-10-23 09:16:10 +10:00 @@ -1605,7 +1605,7 @@ */ void msleep(unsigned int msecs) { - unsigned long timeout = msecs_to_jiffies(msecs); + unsigned long timeout = msecs_to_jiffies(msecs) + 1; while (timeout) { set_current_state(TASK_UNINTERRUPTIBLE); @@ -1621,7 +1621,7 @@ */ unsigned long msleep_interruptible(unsigned int msecs) { - unsigned long timeout = msecs_to_jiffies(msecs); + unsigned long timeout = msecs_to_jiffies(msecs) + 1; while (timeout && !signal_pending(current)) { set_current_state(TASK_INTERRUPTIBLE); ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pmac_cpufreq msleep cleanup/fixes 2004-10-22 23:43 ` Nishanth Aravamudan @ 2004-10-23 0:33 ` Paul Mackerras 0 siblings, 0 replies; 7+ messages in thread From: Paul Mackerras @ 2004-10-23 0:33 UTC (permalink / raw) To: Nishanth Aravamudan Cc: Benjamin Herrenschmidt, Linus Torvalds, Linux Kernel Mailing List Nishanth Aravamudan writes: > Looks good to me... And makes quite a bit of sense, after I thought > about it. Does feel a little hacky, but I don't see a slicker way around > the problem... We would need a platform-specific function to tell us how long until the next jiffy to do any better, I think, and even then it would only make much of a difference if HZ was significantly smaller than 1000. We could do the how-long-until-next-jiffy function quite easily and quickly on ppc and ppc64, just by reading the decrementer register and scaling it. I don't know about other architectures. But if we are mostly using HZ=1000 there doesn't seem to be much point. Paul. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pmac_cpufreq msleep cleanup/fixes 2004-10-22 23:17 ` Benjamin Herrenschmidt 2004-10-22 23:43 ` Nishanth Aravamudan @ 2004-10-22 23:51 ` Linus Torvalds 1 sibling, 0 replies; 7+ messages in thread From: Linus Torvalds @ 2004-10-22 23:51 UTC (permalink / raw) To: Benjamin Herrenschmidt; +Cc: Linux Kernel Mailing List, Paul Mackerras On Sat, 23 Oct 2004, Benjamin Herrenschmidt wrote: > > This wasn't a cleanup but a bug fix actually ... Oh well, I think we need > to fix msleep() instead, what do you think ? I agree. Linus ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH][RESEND] Fix msleep to sleep _at_least_ the requested amount 2004-10-22 22:53 ` Linus Torvalds 2004-10-22 23:17 ` Benjamin Herrenschmidt @ 2004-10-25 0:19 ` Benjamin Herrenschmidt 1 sibling, 0 replies; 7+ messages in thread From: Benjamin Herrenschmidt @ 2004-10-25 0:19 UTC (permalink / raw) To: Linus Torvalds, Andrew Morton; +Cc: Linux Kernel Mailing List, Paul Mackerras Hi Linus ! You "agreed" but didn't apply the patch on the last message ... so here it is again. Makes sure msleep() sleeps at least the amount provided, since schedule_timeout() doesn't guarantee a full jiffy. Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> ===== kernel/timer.c 1.100 vs edited ===== --- 1.100/kernel/timer.c 2004-10-19 19:40:28 +10:00 +++ edited/kernel/timer.c 2004-10-23 09:16:10 +10:00 @@ -1605,7 +1605,7 @@ */ void msleep(unsigned int msecs) { - unsigned long timeout = msecs_to_jiffies(msecs); + unsigned long timeout = msecs_to_jiffies(msecs) + 1; while (timeout) { set_current_state(TASK_UNINTERRUPTIBLE); @@ -1621,7 +1621,7 @@ */ unsigned long msleep_interruptible(unsigned int msecs) { - unsigned long timeout = msecs_to_jiffies(msecs); + unsigned long timeout = msecs_to_jiffies(msecs) + 1; while (timeout && !signal_pending(current)) { set_current_state(TASK_INTERRUPTIBLE); ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2004-10-25 0:21 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <200410221906.i9MJ63Ai022889@hera.kernel.org>
2004-10-22 22:36 ` [PATCH] pmac_cpufreq msleep cleanup/fixes Benjamin Herrenschmidt
2004-10-22 22:53 ` Linus Torvalds
2004-10-22 23:17 ` Benjamin Herrenschmidt
2004-10-22 23:43 ` Nishanth Aravamudan
2004-10-23 0:33 ` Paul Mackerras
2004-10-22 23:51 ` Linus Torvalds
2004-10-25 0:19 ` [PATCH][RESEND] Fix msleep to sleep _at_least_ the requested amount Benjamin Herrenschmidt
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.