* Re: [KJ] do all schedule_timeout() calls need to set state first?
2007-03-19 18:00 [KJ] do all schedule_timeout() calls need to set state first? Robert P. J. Day
2007-03-19 18:44 ` Nishanth Aravamudan
@ 2007-03-19 19:47 ` Robert P. J. Day
2007-03-19 20:25 ` Nishanth Aravamudan
2 siblings, 0 replies; 4+ messages in thread
From: Robert P. J. Day @ 2007-03-19 19:47 UTC (permalink / raw)
To: kernel-janitors
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.
> 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.)
> 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?
rday
--
====================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA
http://fsdev.net/wiki/index.php?title=Main_Page
====================================
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [KJ] do all schedule_timeout() calls need to set state first?
2007-03-19 18:00 [KJ] do all schedule_timeout() calls need to set state first? Robert P. J. Day
2007-03-19 18:44 ` Nishanth Aravamudan
2007-03-19 19:47 ` Robert P. J. Day
@ 2007-03-19 20:25 ` Nishanth Aravamudan
2 siblings, 0 replies; 4+ messages in thread
From: Nishanth Aravamudan @ 2007-03-19 20:25 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 4+ messages in thread