2005-09-23 17:34:51

by Davide Libenzi

[permalink] [raw]
Subject: [patch] Make epoll_wait() handle negative timeouts as MAX_SCHEDULE_TIMEOUT ...


As reported by Vadim Lobanov, epoll_wait() did not handle correctly
timeouts <0 (only the -1 case was MAX_SCHEDULE_TIMEOUT'd).


Signed-off-by: Davide Libenzi <[email protected]>


- Davide


Attachments:
epoll-timeofix.diff (472.00 B)

2005-09-23 17:42:25

by Nish Aravamudan

[permalink] [raw]
Subject: Re: [patch] Make epoll_wait() handle negative timeouts as MAX_SCHEDULE_TIMEOUT ...

On 9/23/05, Davide Libenzi <[email protected]> wrote:
>
> As reported by Vadim Lobanov, epoll_wait() did not handle correctly
> timeouts <0 (only the -1 case was MAX_SCHEDULE_TIMEOUT'd).
>
>
> Signed-off-by: Davide Libenzi <[email protected]>

Arrgggh, this is as wrong as sys_poll() was before! :)

--- a/fs/eventpoll.c 2005-09-23 10:06:45.000000000 -0700
+++ b/fs/eventpoll.c 2005-09-23 10:09:35.000000000 -0700
@@ -1507,7 +1507,7 @@
* and the overflow condition. The passed timeout is in milliseconds,
* that why (t * HZ) / 1000.
*/
- jtimeout = timeout == -1 || timeout > (MAX_SCHEDULE_TIMEOUT - 1000) / HZ ?
+ jtimeout = timeout < 0 || timeout > (MAX_SCHEDULE_TIMEOUT - 1000) / HZ ?

@timeout is in miliseconds, per the comment, yes? If so, then

timeout [msecs] > MAX_SCHEDULE_TIMEOUT [jiffies] - 1000 [jiffies] / HZ
[jiffies / sec]

compares milliseconds to seconds! (Don't worry, sys_poll() had the
same error for a long time). There is a patch in 2.6.14-rc2-mm1 for
sys_poll() to fix the handling of long timeouts, please take a look
and maybe apply the same ideas to epoll(). Alexey Dobriyan has filed a
regression against the patch, but I'm unable to reproduce it (and it
could be an app depending on the old broken behavior).

Thanks,
Nish

2005-09-23 18:01:22

by Davide Libenzi

[permalink] [raw]
Subject: Re: [patch] Make epoll_wait() handle negative timeouts as MAX_SCHEDULE_TIMEOUT ...

On Fri, 23 Sep 2005, Nish Aravamudan wrote:

> On 9/23/05, Davide Libenzi <[email protected]> wrote:
>>
>> As reported by Vadim Lobanov, epoll_wait() did not handle correctly
>> timeouts <0 (only the -1 case was MAX_SCHEDULE_TIMEOUT'd).
>>
>>
>> Signed-off-by: Davide Libenzi <[email protected]>
>
> Arrgggh, this is as wrong as sys_poll() was before! :)
>
> --- a/fs/eventpoll.c 2005-09-23 10:06:45.000000000 -0700
> +++ b/fs/eventpoll.c 2005-09-23 10:09:35.000000000 -0700
> @@ -1507,7 +1507,7 @@
> * and the overflow condition. The passed timeout is in milliseconds,
> * that why (t * HZ) / 1000.
> */
> - jtimeout = timeout == -1 || timeout > (MAX_SCHEDULE_TIMEOUT - 1000) / HZ ?
> + jtimeout = timeout < 0 || timeout > (MAX_SCHEDULE_TIMEOUT - 1000) / HZ ?
>
> @timeout is in miliseconds, per the comment, yes? If so, then
>
> timeout [msecs] > MAX_SCHEDULE_TIMEOUT [jiffies] - 1000 [jiffies] / HZ
> [jiffies / sec]

Sh*t, you're right! Reposting soon.


- Davide