Re: [KJ] do all schedule_timeout() calls need to set state first?

[Nishanth Aravamudan <nacc@us.ibm.com>]

On 19.03.2007 [15:47:06 -0400], Robert P. J. Day wrote:
> On Mon, 19 Mar 2007, Nishanth Aravamudan wrote:
> 
> > On 19.03.2007 [14:00:53 -0400], Robert P. J. Day wrote:
> > >
> > >   based on what i've been reading, all calls to schedule_timeout()
> > > *must* first set the current process state to either
> > > TASK_INTERRUPTIBLE or TASK_UNINTERRUPTIBLE, is that correct?
> >
> > Yes, but note that setting it directly before is not always
> > necessary, in particular when used with wait-queues.
> 
> right, i realized that.  but it does have to be set at *some* point
> before invoking that routine.

Yes. Presuming the intent is to actually schedule a timeout. I could
conceive of some (brain-dead, perhaps) code that would either sleep or
not, depending on some condition, and thus may not change its state
before calling schedule_timeout. Last time I looked, though, there was
no such code in the tree (and I would discourage it if I saw it).

> > But, take a look at the comment heading schedule_timeout():
> >
> >  * 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()).
> >
> > >   since i was once again perusing the source tree and noticed the
> > > following in drivers/hwmon/ams/ams-i2c.c (line 93):
> > >
> > >         while (remaining) {
> > >                 result = ams_i2c_read(AMS_COMMAND);
> > >                 if (result = 0 || result & 0x80)
> > >                         return 0;
> > >
> > >                 remaining = schedule_timeout(remaining);
> > >         }
> > >
> > > it certainly *seems* that the state isn't being set here.  is that an
> > > error?  just curious.
> >
> > Yes, it is.
> 
> as in, yes, it is an error?  (i'm *assuming* that's what you meant
> here.)

Yes.

> > That is why we have schedule_timeout_interruptible() and
> > schedule_timeout_uninterruptible().
> 
> i'd noticed those -- does that mean that there is no need to ever call
> schedule_timeout(), and that one should always use one of those two
> routines above?  or am i misreading this?

Well, there are cases where you don't want to call the
{un,}interruptible versions. wait-queues, I mentioned earlier. And
sometimes, the task is put in a particular state, does some work and
then sleeps. It's not always clear that the state is important during
that work but rather than risk breaking something, I usually left those
alone.

In general, though, you are right, you shouldn't need to call
schedule_timeout(). In the wait-queue case, for instance, you could just
use wait_event_timeout().

Thanks,
Nish

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors