Message-ID: <393478D7.7D131923@uow.edu.au>
Date:   Wed, 31 May 2000 02:28:39 +0000
From:   Andrew Morton <andrewm@uow.edu.au>
MIME-Version: 1.0
To:     kuznet@ms2.inr.ac.ru
CC:     netdev@oss.sgi.com
Subject: Re: [timers] net/ipv4
References: <3933B0B2.50AB5EA1@uow.edu.au> from "Andrew Morton" at May 30,
            0 10:14:42 pm <200005301659.UAA12445@ms2.inr.ac.ru>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Sender: owner-netdev@oss.sgi.com
Precedence: bulk

Alexey, I'm obviously missing something.

If you want to say "Andrew, TCP is safe" then that's cool.  I'll just
buzz off and stare unhappily at the SCSI code.  But I just need one more
shot at it. :)

kuznet@ms2.inr.ac.ru wrote:
> 
> Even not counting that del_timer_sync is deadlocky,
> it should not be ever used for static timers,

static struct timer_list timer;
int foo;

handler()
{
	foo = 1;
}

mainline()
{
	del_timer_async(&timer);
	foo = 2;
}

That is a race.  So why do you say del_timer_sync() is not needed for
static timers?

>
> Of course. I repeat again and again, TCP use _reference_ _counting_.

Bear with me Alexey, I'm Australian.  Where are the refcounts held?


Let's look at igmp.c:

static __inline__ void igmp_stop_timer(struct ip_mc_list *im)
{
// igmp_timer_expire() could be running right now
    spin_lock_bh(&im->lock);
// and now
    if (del_timer(&im->timer))
        atomic_dec(&im->refcnt);
    im->tm_running=0;
    im->reporter = 0;
// and now.
    im->unsolicit_count = 0;
    spin_unlock_bh(&im->lock);
}

static void igmp_timer_expire(unsigned long data)
{
    ...
// Race here.
    im->reporter = 1;
    ip_ma_put(im);
}

I assume there is SOMETHING at a higher layer which prevents this
occurring.  But what?