Re: [patch] Real-Time Preemption, deactivate() scheduling issue

Eugeny S. Mints
2005-03-04T12:20:03Z

Esben Nielsen wrote:
> As I read the code the driver task (A) should _not_ be removed from the
> runqueue. It has to be waken up to call schedule_timeout() such it gets
> back on the runqueue after 10 ms. If it is taken out of the runqueue at
> line 76 it will stay off the runqueue forever in the TASK_UNINTERRUBTIBLE
> state!
Exactly. This is definilty the bug in the driver code - a developer just
didn;t care about proper utilization of set_current_state(). The driver
works
just because as you have described - his fortune
that scheduler doesn't remove task in not TASK_RUNNING state from a run
queue.
And my main question was - does everybody think it's ok have task in not
TASK_RUNNING state in run queue. My current feeling is that this should
not be allowed.
> As I read the use PREEMPT_ACTIVE, it is there to test on wether this
> rescheduling is volentery or forced (a preemption). If it is forced the
> task shall ofcourse not go off the runqueue but stay there to run again
> when it gets the highest priority. That is why PREEMPT_ACTIVE is set in
> preempt_schedule() and preempt_schedule_irq(). On the other hand if the
> task itself has called schedule() or schedule_timeout() it has to go out
> of the runqueue and wait for some event to wake it up.
You right - it works perfectly - but not for my test case - I believe
task in not TASK_RUNNING state should be removed from a run queue by the
first (any - volontery or forced) execution of the schedule() which
detects the task state is not TASK_RUNNIG.
>
> Yes there will be tasks in state other that TASK_RUNNING on the runqueue.
> The "bug" as I see it is in the scheduler interface: There is no way to
> set the task state and call schedule() or schedule_timeout() atomicly.
> Therefore you can be preempted while the state is not TASK_RUNNING.
Exactly. IMO this interface is weird and needs rework. I don;t undestand
what the reason to set task state before schedule_timeout() call but not
inside, right before the schedule(). The actual task state may be passed
as a parameter.

As to tasks in not TASK_RUNNING state into a run queue - I always
believe the definition of a run queue is - queue of tasks ready to run,
i.e. in TASK_RUNNING state.

Eugeny
>
> Esben
>
>
> On Thu, 3 Mar 2005, Eugeny S. Mints wrote:
>
>
>>please consider the following scenario for full RT kernel.
>>
>>Task A is running then an irq is occured which in turn wakes up irq
>>related thread (B) of a higher priority than A.
>>
>>my current understanding that actual context switch between A and B will
>>occure at preempt_schedule_irq() on the "return form irq " path.
>>
>>in this case the following "if" statement in __schedule() always returns
>>false since preempt_schedule_irq() always sets up PREEMPT_ACTIVE
>>before __schedule() call.
>>
>> if ((prev->state & ~TASK_RUNNING_MUTEX) &&
>> !(preempt_count() & PREEMPT_ACTIVE)) {
>>
>>as result the deactivate() is never called for preempted task A in this
>>scenario. BUt if the task A is preempted while not in TASK_RUNNING state
>>such behaviour seems incorrect since we get a task in not TASK_RUNNING
>>state linked into a run queue.
>>
>>An example:
>>
>>drivers/net/irda/sir_dev.c: 76 (2.6.10 kernel)
>>
>> spin_lock_irqsave(&dev->tx_lock, flags); /* serialize th other
>>tx operations */
>> while (dev->tx_buff.len > 0) { /* wait until tx idle */
>> spin_unlock_irqrestore(&dev->tx_lock, flags);
>>76: set_current_state(TASK_UNINTERRUPTIBLE);
>> schedule_timeout(msecs_to_jiffies(10));
>> spin_lock_irqsave(&dev->tx_lock, flags);
>> }
>>
>>At line 76 irqs are enabled, preemption is enabled.
>>Let assume the task A executes this code and gets preempted right after
>>line 76. Task state is TASK_UNINTERRUPTIBLE but it will not be
>>deactevated. Of cource this is the bug in set_current_state()
>>utilization in this particular driver but schedule stuff should be
>>robust to such bugs I believe. There are a lot such bugs in the kernel I
>>believe.
>>
>>Not sure what the actual reason for !(preempt_count() & PREEMPT_ACTIVE))
>> condition is but if it's just a sort of optimization (not remove a
>>task from run queue if it was preemped in TASK_RUNNING state) then
>>probably it should be removed in order to save correctness. Patch attached.
>>
>> Eugeny
>>
>>
>
>
>
>