Re: sleep

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

 



On Mon, Oct 17, 2005 at 06:36:53 -0700, Nish Aravamudan wrote:
> On 10/16/05, Jan Hudec <bulb@xxxxxx> wrote:
> > On Mon, Sep 19, 2005 at 15:13:10 +0900, linux lover wrote:
> > > You can use schedule_timeout like this
> > > current->state = TASK_UNINTERRUPTIBLE;
> > > schedule_timeout(X*HZ); /* Sleep for X seconds */
> > > current->state = TASK_INTERRUPTIBLE;
> >
> > Um, no. Since preempt went in, this is exactly the one you _can't_ use.
> > Because a schedule can occur from preempt just between setting the state
> > to TASK_UNINTERRUPTIBLE and calling schedule_timeout. I don't know how
> > to operate the necessary preempt stuff though.
> 
> Well, the code of:
> 
> set_current_state(TASK_{,UN}INTERRUPTIBLE);
> schedule_timeout(x_secs*HZ);
> 
> does work in the kernel, under preempt. But you would not manually

The code *exists* in kernel. I am however not sure about works:

set_current_state(TASK_UNINTERRUPTIBLE);
<- preempt comes in, calling schedule() .. but there is NO way to wake
   that process up again, since schedule_timeout did not set up the
   timer schedule_timeout would...
schedule_timeout(x_secs*HZ); // Well, we never ever get here.

> keep the task in a sleeping state, that doesn't make sense (i.e. the
> current->state = TASK_INTERRUPTIBLE assignment after
> schedule_timeout), as schedule_timeout() guarantees to return in
> TASK_RUNNING and you're not sleeping again immediately. I guess your
> concern might be that we might sleep much longer than requested, since
> we could preempt then sleep the x seconds? Not much you can do there.

Unfortunately it's not /much longer/, but /indefinitely/.

There *IS* something you can do there though:

preempt_disable()
set_current_state(TASK_UNINTERRUPTIBLE)
schedule_timeout(x_sec*HZ)
preempt_enable()

preempt_enable/disable /are/ recursive, so they should work.

> Code should probably check that it should sleep before calling
> schedule_timeout, generally. Not to say it can't happen, but I haven't
> seen anyone report anything like that.

Does not help. You must set the means of waking up and the state
atomically. Wrt. SMP ordering is enough, as noone else should modify
your state, but for preempt you must disable preempt. That's why
wait_event_timeout (which you should be using anyway) now declares a
spinlock.

> But we've changed that anyways now, and you have
> 
> schedule_timeout_{,un}interruptible(x_secs*HZ);
> 
> that sets the state for you.

Now means since which version? 2.6.11 (latest on LXR; yes I know latest
is 2.6.13) does not seem to have that.

> [...]
>
> > Um, and current->state = TASK_<whatever> is not correct either -- there
> > is a set_current_state macro, that must be used, because unlike the
> > assignment it's quaranteed to be atomic and block undesired
> > optimizations.
> 
> We also have
> 
> __set_current_state(), which is also atomic (I believe) but does not
> have an mb, e.g. it should be identical to current->state =
> TASK_{,UN}INTERRUPTIBLE.

Yes, I see.

> You are right, though, if you are calling schedule_timeout()
> immediately after, then you need the mb-version.

Yes, yes. The __set_current_state variant is only usable when you are
going to use something else that contains a barrier, like spin_unlock(),
right afterwards.

> > >  On 9/19/05, raja <vnagaraju@xxxxxxxxxxxx> wrote:
> > > >
> > > > Hi,
> > > > Would you please tell me how can i sleep in kernel space.
> >
> > Um, the above, with schedule_timeout, does not seem to be correct
> > responese... In fact the correct response would be: In many ways (though
> > all of them end up calling schedule() underneath). What do you want it
> > for?
> >
> > By the way, TASK_INTERRUUPTIBLE is also sleep. When you
> > set_current_state(TASK_INTERRUPTIBLE), preempt may call schedule on you
> > and you won't wake up unless you get a signal.
> 
> Not entirely true, see above.

I beg your pardon, but I did not fund an explanation for this above, nor
do I see it in the code (I am looking at 2.6.11 on LXR).

--
						 Jan 'Bulb' Hudec <bulb@xxxxxx>

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]
  Powered by Linux