2006-03-16 22:31:13

by Yitzchak Eidus

Subject: puting task to TASK_INTERRUPTIBLE before adding it to an wait queue

the function worker_thread in kernel 2.6.15.6 first put the task to
TASK_INTERRUPTIBLE and only then add itself to an wait queue:
set_current_state(TASK_INTERRUPTIBLE);
while (!kthread_should_stop()) {
add_wait_queue(&cwq->more_work, &wait);
....
my question is, what will happen if the timeslice for the
worker_thread will finished just before it add itself to the wait
queue?
wont it call schedule() that will find the task is in
TASK_INTERRUPTIBLE state and remove it from the runqueue? ( that what
schedule() should do no? )
and then how will the kernel be able to call to worker_thread ever if
it isnt in any list???
thanks for the comments!

2006-03-16 22:52:12

by Yitzchak Eidus

Subject: Re: puting task to TASK_INTERRUPTIBLE before adding it to an wait queue

On 3/17/06, Yitzchak Eidus <[email protected]> wrote:
> the function worker_thread in kernel 2.6.15.6 first put the task to
> TASK_INTERRUPTIBLE and only then add itself to an wait queue:
> set_current_state(TASK_INTERRUPTIBLE);
> while (!kthread_should_stop()) {
> add_wait_queue(&cwq->more_work, &wait);
> ....
> my question is, what will happen if the timeslice for the
> worker_thread will finished just before it add itself to the wait
> queue?
> wont it call schedule() that will find the task is in
> TASK_INTERRUPTIBLE state and remove it from the runqueue? ( that what
> schedule() should do no? )
> and then how will the kernel be able to call to worker_thread ever if
> it isnt in any list???
> thanks for the comments!
>

more over the whole loop look like that:
set_current_state(TASK_INTERRUPTIBLE);
while (!kthread_should_stop()) {
add_wait_queue(&cwq->more_work, &wait);
if (list_empty(&cwq->worklist))
schedule();
else
__set_current_state(TASK_RUNNING);
remove_wait_queue(&cwq->more_work, &wait);

if (!list_empty(&cwq->worklist))
run_workqueue(cwq);
set_current_state(TASK_INTERRUPTIBLE);
}

what was the logic of putting the
set_current_state(TASK_INTERRUPTIBLE); before the loop and in the last
statement of the loop?

why not use something like this:
while (!kthread_should_stop()) {
add_wait_queue(&cwq->more_work, &wait);
set_current_state(TASK_INTERRUPTIBLE);
if (list_empty(&cwq->worklist))
schedule();
else
__set_current_state(TASK_RUNNING);
remove_wait_queue(&cwq->more_work, &wait);

if (!list_empty(&cwq->worklist))
run_workqueue(cwq);
}
that do the same thing without putting the task before the loop and in
the loop...?
( unless i am missing something? )

2006-03-17 08:00:11

by Mike Galbraith

Subject: Re: puting task to TASK_INTERRUPTIBLE before adding it to an wait queue

On Fri, 2006-03-17 at 00:31 +0200, Yitzchak Eidus wrote:
> the function worker_thread in kernel 2.6.15.6 first put the task to
> TASK_INTERRUPTIBLE and only then add itself to an wait queue:
> set_current_state(TASK_INTERRUPTIBLE);
> while (!kthread_should_stop()) {
> add_wait_queue(&cwq->more_work, &wait);

See dusty old archives...

http://www.ussg.iu.edu/hypermail/linux/kernel/9712.2/0545.html

<quote>
Anyway, the basic race-free wait loop looks like this (there are
variations, but this is one of the basic versions that you find in
various places):


if (should_wait_condition) {
add_wait_queue(..);
repeat:
current->state = TASK_UNINTERRUPTIBLE;
if (should_wait_condition) {
schedule();
goto repeat;
}
remove_wait_queue(..);
current->state = TASK_RUNNING;
}


There are only two important rules:
- you have to add yourself to the wait queue _before_ testing for the
condition.
- you have to mark yourself asleep _before_ testing for the condition.

</quote>

2006-03-18 06:25:08

by Andy Lutomirski

Subject: Re: puting task to TASK_INTERRUPTIBLE before adding it to an wait queue

Mike Galbraith wrote:
> On Fri, 2006-03-17 at 00:31 +0200, Yitzchak Eidus wrote:
>
>>the function worker_thread in kernel 2.6.15.6 first put the task to
>>TASK_INTERRUPTIBLE and only then add itself to an wait queue:
>> set_current_state(TASK_INTERRUPTIBLE);
>> while (!kthread_should_stop()) {
>> add_wait_queue(&cwq->more_work, &wait);
>
>
> See dusty old archives...

Also, preempted tasks get rescheduled regardless of state:

http://www.cs.helsinki.fi/linux/linux-kernel/2003-15/0402.html

--Andy