Re: [PATCH] 2.5.53 : net/atm/lec.c

romieu@fr.zoreil.com
Tue, 31 Dec 2002 00:07:56 +0100


Greetings,

Frank Davis <fdavis@si.rr.com> :
[locking in net/atm/lec.c]

net/atm/lec.c::lec_arp_destroy()
-> spin_lock_irqsave(&lec_lock, flags);
-> lec_arp_remove(priv->lec_arp_tables, entry);
-> spin_lock_irqsave(&lec_lock, flags);
-> good bye Charlie !

Both lec_arp_check_expire() (timer function) and lec_arp_destroy() are
walking the lec_arp_table from the 'lec_priv' they were attached to and if
the intent of lec_arp_{lock/unlock} is to protect the timer function
lec_arp_check_expire() by virtue of refcounting, it seems slightly racy.
If you are wondering who the callers of lec_arp_remove() are:
lec_arp_remove
<- lec_atm_send
<- lecdev_ops.send = lec_atm_send
<- lec_arp_destroy
<- lec_atm_close
<- lecdev_ops.close = lec_atm_close
<- lec_arp_check_expire
<- priv->lec_arp_timer.function = lec_arp_check_expire;
<- lec_addr_delete
<- lec_atm_send
<- lec_vcc_close
<- lec_push
<- vcc->push = lec_push;
<- lec_arp_check_empties
<- lec_push

Removing the lock from lec_arp_remove() and asking callers to do themselves
the locking (liek lec_arp_destroy) doesn't seem too unsane.

Btw, del_timer_sync() in lec_arp_destroy() shouldn't hurt imho.

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