linux-kernel - Re: [PATCH] [SCSI] osst: remove double conversion of timeout

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160108073155.GA16584@osadl.at>
Date:	Fri, 8 Jan 2016 07:31:55 +0000
From:	Nicholas Mc Guire <der.herr@...r.at>
To:	"Seymour, Shane M" <shane.seymour@....com>
Cc:	Nicholas Mc Guire <hofrat@...dl.org>,
	Willem Riede <osst@...de.org>,
	"James E.J. Bottomley" <JBottomley@...n.com>,
	"Martin K. Petersen" <martin.petersen@...cle.com>,
	"osst-users@...ts.sourceforge.net" <osst-users@...ts.sourceforge.net>,
	"linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [SCSI] osst: remove double conversion of timeout

On Fri, Jan 08, 2016 at 12:02:18AM +0000, Seymour, Shane M wrote:
> Hi Nicholas,
> 
> > -		msleep(jiffies_to_msecs(initial_delay));
> > +		schedule_timeout_uninterruptible(initial_delay);
>  
> The code to msleep looks like this:
> 
> /**
>  * msleep - sleep safely even with waitqueue interruptions
>  * @msecs: Time in milliseconds to sleep for
>  */
> void msleep(unsigned int msecs)
> {
> 	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
> 
> 	while (timeout)
> 		timeout = schedule_timeout_uninterruptible(timeout);
> }
> 
> So msleep will wait for the requested amount of time to pass even if woken early but your change may not. To make it equivalent you would need to borrow code from msleep and remove the "if (initial_delay > 0)" and just have:

If I understand the documentation to 
schedule_timeout() correctly then this is not the 
case:
<snip>
/**
 * schedule_timeout - sleep until timeout
 * @timeout: timeout value in jiffies
 *
 * Make the current task sleep until @timeout jiffies have
 * elapsed. The routine will return immediately unless
 * the current task state has been set (see set_current_state()).
 *
 * You can set the task state as follows -
 *
 * %TASK_UNINTERRUPTIBLE - at least @timeout jiffies are guaranteed to
 * pass before the routine returns. The routine will return 0
...
<snip>

 and schedule_timeout_uninterruptiple does that

signed long __sched schedule_timeout_uninterruptible(signed long timeout)
{
        __set_current_state(TASK_UNINTERRUPTIBLE);
        return schedule_timeout(timeout);
}

so my understanding is that this will not return early and should thus
be equivalent to the behavior of msleep.

did I overlook something here ?

thx!
hofrat