Re: [UPDATE PATCH] Add schedule_timeout_{intr,unintr}{,_msecs}() interfaces

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Mon, 1 Aug 2005, Nishanth Aravamudan wrote:

> +unsigned int __sched schedule_timeout_msecs(unsigned int timeout_msecs)
> +{
> +	unsigned long expire_jifs;
> +
> +	if (timeout_msecs == MAX_SCHEDULE_TIMEOUT_MSECS) {
> +		expire_jifs = MAX_SCHEDULE_TIMEOUT;
> +	} else {
> +		/*
> +		 * msecs_to_jiffies() is a unit conversion, which truncates
> +		 * (rounds down), so we need to add 1.
> +		 */
> +		expire_jifs = msecs_to_jiffies(timeout_msecs) + 1;
> +	}
> +
> +	expire_jifs = schedule_timeout(expire_jifs);
> +
> +	/*
> +	 * don't need to add 1 here, even though there is truncation,
> +	 * because we will add 1 if/when the value is sent back in
> +	 */
> +	return jiffies_to_msecs(expire_jifs);
> +}

As I already mentioned for msleep_interruptible this is a really terrible 
interface.
The "jiffies_to_msecs(msecs_to_jiffies(timeout_msecs) + 1)" case (when the 
process is immediately woken up again) makes the caller suspectible to 
timeout manipulations and requires constant reauditing, that no caller 
gets it wrong, so it's better to avoid this error source completely.
Constant conversion between different time units is a really bad idea. If 
the user needs the remaining time, he is really better off to do it 
himself by checking jiffies and only does an initial conversion from 
relative to absolute (kernel) time.
This wrapper function really should be an inline function and should look 
more like this:

static inline int schedule_timeout_msecs(unsigned int timeout_msecs)
{
	return schedule_timeout(msecs_to_jiffies(timeout_msecs) + 1) != 0;
}

bye, Roman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]
  Powered by Linux