Nishanth Aravamudan wrote:

>On 09.09.2005 [19:36:21 -0700], Andrew Morton wrote:
>  
>
>>Nishanth Aravamudan <nacc@us.ibm.com> wrote:
>>    
>>
>>>+	/*
>>> +	 * We compare HZ with 1000 to work out which side of the
>>> +	 * expression needs conversion.  Because we want to avoid
>>> +	 * converting any value to a numerically higher value, which
>>> +	 * could overflow.
>>> +	 */
>>> +#if HZ > 1000
>>> +	overflow = timeout_msecs >= jiffies_to_msecs(MAX_SCHEDULE_TIMEOUT);
>>> +#else
>>> +	overflow = msecs_to_jiffies(timeout_msecs) >= MAX_SCHEDULE_TIMEOUT;
>>> +#endif
>>> +
>>> +	/*
>>> +	 * If we would overflow in the conversion or a negative timeout
>>> +	 * is requested, sleep indefinitely.
>>> +	 */
>>> +	if (overflow || timeout_msecs < 0)
>>> +		timeout_jiffies = MAX_SCHEDULE_TIMEOUT;
>>>      
>>>
>>Do we need to test (timeout_msecs < 0) here?  If we make timeout_msecs
>>unsigned long then I think `overflow' will always be correct.
>>    
>>
>
>Even though poll is explicitly allowed to take negative values, as per
>my man-page:
>
>"#include <sys/poll.h>
>
>int poll(struct pollfd *ufds, unsigned int nfds, int timeout);
>
>...
>
>A negative value means infinite timeout."
>
>Would we have a local variable to store timeout_msecs as well? Or do we
>want to make a userspace-visible change like this? I don't have a
>preference, I just want to make sure I understand.
>

Actually, given this, isn't the interface for sys_poll() incorrectly 
defined?
Shouldn't the timeout argument be an int, instead of a long?

And, if we make it an int, then can't we do the math correctly for all
possible values of the timeout?  The patch could look like:

Signed-off-by: Peter Staubach <staubach@redhat.com>