Killing tasklet from interrupt

Killing tasklet from interrupt

[Jean Tourrilhes]

	Hi,

	I'm trying to use tasklets and I've come across one problem. I
need to kill a tasklet from a timer, and I wonder if it's legal.

	Code :
	-> User close IrDA TSAP and goes away
		-> LSAP not clean, more work to do
			-> Schedule timer in one second
	-> Timer
		-> If LSAP clean and nothing to do
			-> Kill tasklet
			-> Destroy LSAP
		-> Else re-shedule timer

	The tasklet is used in the Rx path, so may be scheduled after
the user close the TSAP. The TSAP may interface to the socket code, to
the TTY code, to the Ethernet code or the PPP code, so we are not even
guaranteed that the TSAP closure is done from a user context (fun,
fun, fun).
	To be fair, the timer API is much more versatile in that
respect. What I think I need is a tasklet_try_kill()...

	Regards,

	Jean

Re: Killing tasklet from interrupt

[Richard B. Johnson]

On Mon, 18 Mar 2002, Jean Tourrilhes wrote:

> 	Hi,
> 
> 	I'm trying to use tasklets and I've come across one problem. I
> need to kill a tasklet from a timer, and I wonder if it's legal.
> 
> 	Code :
> 	-> User close IrDA TSAP and goes away
> 		-> LSAP not clean, more work to do
> 			-> Schedule timer in one second
> 	-> Timer
> 		-> If LSAP clean and nothing to do
> 			-> Kill tasklet
> 			-> Destroy LSAP
> 		-> Else re-shedule timer
> 
> 	The tasklet is used in the Rx path, so may be scheduled after
> the user close the TSAP. The TSAP may interface to the socket code, to
> the TTY code, to the Ethernet code or the PPP code, so we are not even
> guaranteed that the TSAP closure is done from a user context (fun,
> fun, fun).
> 	To be fair, the timer API is much more versatile in that
> respect. What I think I need is a tasklet_try_kill()...
> 
> 	Regards,
> 
> 	Jean

You have the tasklet kill itself the next time it executes. Set some
flag so it knows it should give up its timer-slot and expire. The
interrupt sets the flag. It doesn't do anything else.


Cheers,
Dick Johnson

Penguin : Linux version 2.4.18 on an i686 machine (797.90 BogoMips).

                 Windows-2000/Professional isn't.

Re: Killing tasklet from interrupt

[Jean Tourrilhes]

On Mon, Mar 18, 2002 at 03:38:29PM -0500, Richard B. Johnson wrote:
> 
> You have the tasklet kill itself the next time it executes. Set some
> flag so it knows it should give up its timer-slot and expire. The
> interrupt sets the flag. It doesn't do anything else.

	I already have this flag and my code mostly work like this, so
that would be trivial to do.
	I looked at the code, and you are right, killing the tasklet
within itself is by far the safest way to do it. It's a shame that the
code doesn't explitely allow for it (i.e. you will deadlock every time
in tasklet_unlock_wait(t);).

	Thanks, and have fun...

	Jean

Re: Killing tasklet from interrupt

[Maksim Krasnyanskiy]

> > You have the tasklet kill itself the next time it executes. Set some
> > flag so it knows it should give up its timer-slot and expire. The
> > interrupt sets the flag. It doesn't do anything else.
>
>         I already have this flag and my code mostly work like this, so
>that would be trivial to do.
>         I looked at the code, and you are right, killing the tasklet
>within itself is by far the safest way to do it.
Sounds like what you need is tasklet_disable.
tasklet_kill needs process context so you can't use it in timer.

>It's a shame that the code doesn't explitely allow for it (i.e. you will 
>deadlock every time
>in tasklet_unlock_wait(t);).
Use tasklet_disable_nosync within the tasklet itself.

Max

Re: Killing tasklet from interrupt

[Jean Tourrilhes]

On Mon, Mar 18, 2002 at 03:20:50PM -0800, Maksim Krasnyanskiy wrote:
> 
> > > You have the tasklet kill itself the next time it executes. Set some
> > > flag so it knows it should give up its timer-slot and expire. The
> > > interrupt sets the flag. It doesn't do anything else.
> >
> >         I already have this flag and my code mostly work like this, so
> >that would be trivial to do.
> >         I looked at the code, and you are right, killing the tasklet
> >within itself is by far the safest way to do it.
> Sounds like what you need is tasklet_disable.
> tasklet_kill needs process context so you can't use it in timer.
> 
> >It's a shame that the code doesn't explitely allow for it (i.e. you will 
> >deadlock every time
> >in tasklet_unlock_wait(t);).
> Use tasklet_disable_nosync within the tasklet itself.
> 
> Max

	Well. I thought about that. Not possible.
	tasklet_disable is not the answer, because if the tasklet was
scheduled, it will stay forever in the tasklet queue. Also, I need to
forget forever about getting rid of the tasklet within the tasklet
itself, because it will just crash.
	Look below, comments by me (you've got to love uncommented
code). So, it's not today that I will use tasklets.

	Regards,

	Jean

P.S. : By the way, regarding flow control between TCP and netdevice
(our previous e-mail exchange with Paul), have you investigated the
effect of skb->destructor; (for example sock_wfree()).

-------------------------------------------------------------

static void tasklet_action(struct softirq_action *a)
{
	int cpu = smp_processor_id();
	struct tasklet_struct *list;

	local_irq_disable();
	list = tasklet_vec[cpu].list;
	tasklet_vec[cpu].list = NULL;
	local_irq_enable();

	while (list) {
		struct tasklet_struct *t = list;

		list = list->next;

		if (tasklet_trylock(t)) {
			if (!atomic_read(&t->count)) {
				if (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state))
					BUG();
	// Call tasklet handler
				t->func(t->data);
	// If tasklet was killed/destroyed/kfree above, we will die
				tasklet_unlock(t);
				continue;
			}
			tasklet_unlock(t);
		}

	// Tasklet is disabled, put back in tasklet queue
		local_irq_disable();
		t->next = tasklet_vec[cpu].list;
		tasklet_vec[cpu].list = t;
		__cpu_raise_softirq(cpu, TASKLET_SOFTIRQ);
		local_irq_enable();
	}
}

Re: Killing tasklet from interrupt

[Maksim Krasnyanskiy]

> > Sounds like what you need is tasklet_disable.
> > tasklet_kill needs process context so you can't use it in timer.
> >
> > >It's a shame that the code doesn't explitely allow for it (i.e. you will
> > >deadlock every time
> > >in tasklet_unlock_wait(t);).
> > Use tasklet_disable_nosync within the tasklet itself.
>
>         Well. I thought about that. Not possible.
>         tasklet_disable is not the answer, because if the tasklet was
>scheduled, it will stay forever in the tasklet queue. Also, I need to
>forget forever about getting rid of the tasklet within the tasklet
>itself, because it will just crash.
How about something like this ?

void tasklet_kill_from_interrupt(struct tasklet_struct *t)
{
         while (test_and_set_bit(TASKLET_STATE_SCHED, &t->state));
         tasklet_unlock_wait(t);
}

So, in your timer you would do:
         set_bit(CLOSING_PLEASE_DONT_SCHEDULE_ANYTHING, something->state);
         tasklet_kill_from_interrupt(something->tasklet);
         /* cleanup/kfree/etc */

>         Look below, comments by me (you've got to love uncommented
>code). So, it's not today that I will use tasklets.
Well, I use them without any problems in Bluetooth code. May be you should
redesign your code a bit. For example don't kill tasklets from the timer.

>P.S. : By the way, regarding flow control between TCP and netdevice
>(our previous e-mail exchange with Paul), have you investigated the
>effect of skb->destructor; (for example sock_wfree()).
I'm sorry I must have missed skb->destructor part. How sock_wfree could 
affect flow ctl between TCP and netdev ?
sock_wfree just wakes up process sleeping in sock_alloc_send_skb or alike.

>-------------------------------------------------------------
>
>static void tasklet_action(struct softirq_action *a)
>{
>         int cpu = smp_processor_id();
>         struct tasklet_struct *list;
>
>         local_irq_disable();
>         list = tasklet_vec[cpu].list;
>         tasklet_vec[cpu].list = NULL;
>         local_irq_enable();
>
>         while (list) {
>                 struct tasklet_struct *t = list;
>
>                 list = list->next;
>
>                 if (tasklet_trylock(t)) {
>                         if (!atomic_read(&t->count)) {
>                                 if 
> (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state))
>                                         BUG();
>         // Call tasklet handler
>                                 t->func(t->data);
>         // If tasklet was killed/destroyed/kfree above, we will die
>                                 tasklet_unlock(t);
>                                 continue;
>                         }
>                         tasklet_unlock(t);
>                 }
"kill" means "wait until tasklet terminates and is not in the queue". So 
it's not a problem
And you would not want to destroy _locked_ tasklet. You'd wait until it's 
unlocked.

Max

Re: Killing tasklet from interrupt

[Maksim Krasnyanskiy]

Did it help ?

>> > Sounds like what you need is tasklet_disable.
>> > tasklet_kill needs process context so you can't use it in timer.
>> >
>> > >It's a shame that the code doesn't explitely allow for it (i.e. you will
>> > >deadlock every time
>> > >in tasklet_unlock_wait(t);).
>> > Use tasklet_disable_nosync within the tasklet itself.
>>
>>         Well. I thought about that. Not possible.
>>         tasklet_disable is not the answer, because if the tasklet was
>>scheduled, it will stay forever in the tasklet queue. Also, I need to
>>forget forever about getting rid of the tasklet within the tasklet
>>itself, because it will just crash.
>How about something like this ?
>
>void tasklet_kill_from_interrupt(struct tasklet_struct *t)
>{
>         while (test_and_set_bit(TASKLET_STATE_SCHED, &t->state));
>         tasklet_unlock_wait(t);
>}
>
>So, in your timer you would do:
>         set_bit(CLOSING_PLEASE_DONT_SCHEDULE_ANYTHING, something->state);
>         tasklet_kill_from_interrupt(something->tasklet);
>         /* cleanup/kfree/etc */
>
>>         Look below, comments by me (you've got to love uncommented
>>code). So, it's not today that I will use tasklets.
>Well, I use them without any problems in Bluetooth code. May be you should
>redesign your code a bit. For example don't kill tasklets from the timer.
>
>>P.S. : By the way, regarding flow control between TCP and netdevice
>>(our previous e-mail exchange with Paul), have you investigated the
>>effect of skb->destructor; (for example sock_wfree()).
>I'm sorry I must have missed skb->destructor part. How sock_wfree could 
>affect flow ctl between TCP and netdev ?
>sock_wfree just wakes up process sleeping in sock_alloc_send_skb or alike.
>
>>-------------------------------------------------------------
>>
>>static void tasklet_action(struct softirq_action *a)
>>{
>>         int cpu = smp_processor_id();
>>         struct tasklet_struct *list;
>>
>>         local_irq_disable();
>>         list = tasklet_vec[cpu].list;
>>         tasklet_vec[cpu].list = NULL;
>>         local_irq_enable();
>>
>>         while (list) {
>>                 struct tasklet_struct *t = list;
>>
>>                 list = list->next;
>>
>>                 if (tasklet_trylock(t)) {
>>                         if (!atomic_read(&t->count)) {
>>                                 if 
>> (!test_and_clear_bit(TASKLET_STATE_SCHED, &t->state))
>>                                         BUG();
>>         // Call tasklet handler
>>                                 t->func(t->data);
>>         // If tasklet was killed/destroyed/kfree above, we will die
>>                                 tasklet_unlock(t);
>>                                 continue;
>>                         }
>>                         tasklet_unlock(t);
>>                 }
>"kill" means "wait until tasklet terminates and is not in the queue". So 
>it's not a problem
>And you would not want to destroy _locked_ tasklet. You'd wait until it's 
>unlocked.
>
>Max