[prev in list] [next in list] [prev in thread] [next in thread] 

List:       quagga-dev
Subject:    [quagga-dev 599] Re: [PATH] isisd threads problems
From:       Hasso Tepper <hasso () estpak ! ee>
Date:       2003-12-29 13:32:07
[Download RAW message or body]

Vincent Jardin wrote:
> > Hasso Tepper <hasso@estpak.ee>
> >Attached small patch fixes problems with canceling threads in
> > isisd described in [quagga-dev 575]. First three parts of patch
> > are really fixes. About last two ones I don't know.
>
> Which ones do you mean ?

Fixes? They are fixes to the problem I described that there were 
attempts to cancel already unused threads. First two parts are fixes 
to the isis_run_dr_l[1|2]() functions. During thread run pointer to 
the thread should be set to NULL.

After fixing this I hit similar bug in isis_dr_resign(). This was just 
copy/paste error (I think). There was attempt to cancel thread of 
wrong level.

I will commit these parts if noone objects.

> Moreover, there is an "unused" flag - THREAD_UNUSED - that is set
> when a thread is canceled. Don't you think that it could be used or
> is it a private information of thread.c !?

This is how threading works in quagga. After canceling thread it's not 
freed but flaged as unused and moved to the list of unused threads. 
If there is need for new thread thread_get() looks at list of unused 
threads. If there is at least one, it will use that. If there is 
none, new is created.

> >Before thread=thread_add_timer(); there should be check if(thread
> > == NULL). And if it it isn't and there is need for rechedule
> > thread, old thread should be canceled before.
>
> THREAD_OFF(x) already check if x is not NULL.
>
> Then, for example,
> + if (circuit->u.bc.t_run_dr[1])
> +      THREAD_OFF(circuit->u.bc.t_run_dr[1]);
>
> could be replaced by
>
>   THREAD_OFF(circuit->u.bc.t_run_dr[1]);
> that is enough.

Ah, yes.

> Moreover, I think that THREAD_READ|WRITE|TIMER_OFF should be used
> instead of THREAD_OFF. It helps to understand the code ;-D

Agree :).

I will go through isisd code and try to fix these. Will post patch to 
the list.

-- 
Hasso Tepper
Elion Enterprises Ltd.
WAN administrator

_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-dev
[prev in list] [next in list] [prev in thread] [next in thread]