On Mon, Sep 12, 2016 at 01:54:34PM +0200, Martin Pieuchot wrote:
> I extended the G/C timeout to 1sec, because there's no reference count
> for 'struct tcpcb'.  And the only race possible is described in the
> comment below:
> 
>       /*
>        * If one of the timer tasks is already running, it must
>        * be spinning on the KERNEL_LOCK().  So schedule a G/C
>        * in one second, when we known the task will have released
>        * its reference.
>        */
> 
> Since the first thing task are doing is checking for the ``TF_DEAD''
> flag this should be enough.

So we rely on the assumption that grabbing the kernel lock will not
take longer than one second.  As reference counting would be much
more work, this seems fine for now.

> +void
> +tcp_destroytimers(struct tcpcb *tp)
> +{
> +     int i;
> +
> +     for (i = 0; i < TCPT_NTIMERS; i++)
> +             TCP_TIMER_DESTROY(tp, i);
> +}

Why do you introduce a special tcp_destroytimers() and TCP_TIMER_DESTROY()
interface?

>  #define      TCP_TIMER_DISARM(tp, timer)                                     
> \
> -     timeout_del(&(tp)->t_timer[(timer)])
> +     timeout_del(&(tp)->t_timer[(timer)].tcpt_timeout)
>  
> +#define      TCP_TIMER_DESTROY(tp, timer)                                    
> \
> +     task_del(softnettq, &(tp)->t_timer[(timer)].tcpt_task)

Could you just move the task_del() to the TCP_TIMER_DISARM()?  Then
it would be totally opaque to the user wehter it is timeout or task.

bluhm

Reply via email to