Re: tasklet_kill will always hang for recursive tasklets on a UP

From: Nagendra Singh Tomar
Date: Thu Aug 28 2003 - 00:55:38 EST


Juergen,
The whole tasklet_kill function is a big confusion. It is a big
misnomer as Werner rightly said. For non-recursive tasklets this
function does not do anything. Its just an expensive "nop". If you simply
call tasklet_schedule after tasklet_kill, it will execute as nothing had
happened.
If we remove the line

clear_bit(TASKLET_STATE_SCHED, &t->state);

from tasklet_kill then tasklet_kill will have the desired effect of
"killing" the tasklet, tasklet_schedule() after tasklet_kill in that case,
will not call __tasklet_kill and hence it will not be queued to the CPU
queue and hence it will not run (desired effect).

tasklet_kill I believe was written to kill recursive tasklets only
(tasklets that schedule themseves from inside their handler), but as we
have seen for that particular case it hangs.
I feel either we remove tasklet_kill or fix it to do what it should.

Alexey, can u please comment on my observations.

Thanx,
tomar



On Wed, 27 Aug 2003, Juergen Quade wrote:

> > Thanx for ur inputs. I think that I am missing something in
> ur
> > explanation. Can u please elaborate. In the meantime, the approach
> that I
>
> Maybe it is easier we make it the other way round.
> If you look at the code of tasklet_kill:
>
> void tasklet_kill(struct tasklet_struct *t)
> {
> if (in_interrupt())
> printk("Attempt to kill tasklet from
> interrupt\n");
>
> while (test_and_set_bit(TASKLET_STATE_SCHED, &t->state))
> {
> do
> yield();
> while (test_bit(TASKLET_STATE_SCHED,
> &t->state));
> }
> tasklet_unlock_wait(t);
> clear_bit(TASKLET_STATE_SCHED, &t->state);
> }
>
> Can you explain me, what the last statement
> clear_bit(TASKLET_STATE_SCHED, &t->state);
> is for?
>
> > will like is to have another state TASKLET_STATE_KILLED so the code
> > changes that need to be done are
> >
> > void tasklet_kill(struct tasklet_struct *t)
> > {
> >
> > ...
> > ...
> > /*
> > * Mark the tasklet as killed, so the next time around
> > * tasklet_action does not call the handler for this tasklet
> > */
> > set_bit(TASKLET_STATE_KILLED, &t->state); <-- ADDED
> > ...
> >
>
> What I don't like on this approach is, to add another flag (=state) to
> the tasklet, which might make the world more complicated as necessary.
> I will take some time to think about it, but can't do that today :-(
>
> Beside this, if you can't use a function without looking
> at the code and without experimenting with it, that
> must lead to bugs! IMHO, here is a call for action.
>
> Juergen.
>

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/