On Tue, Feb 18, 2003 at 11:52:50PM -0500, chas williams wrote:
> yes, atm_dev_lock is wrapped around large chunks of the code. however,
> it should probably never have been a spinlock--it certainly doesnt need
> to be one. as a mutex its far less offensive. i am willing to take
> suggestions as to what would need to be done to fix atm properly?
well, I'm no networking guru, but here are some of the things that seem
useful to me:
Try to use `struct sock' where you can. For example, using sk->stamp
would allow the SIOCGSTAMP ioctl to be dealt with at a higher level.
Right now, atm uses a timestamp embedded in vcc. (Actually, I think
I see how to do this one. I'll send you a patch for your review and
testing, OK?)
Figure out what actually needs to be locked and lock only that.
spinlocks are, by and large, the right solution -- having a Big ATM Lock
isn't a good thing (though I do recognise that you may want to do this
temporarily while you work on fixing it properly).
You should probably do away with SOCKOPS_WRAP / SOCKOPS_WRAPPED. The
BKL doesn't really protect you from much any more.
Might want to consider converting the proc files to the seqfile interface.
There seems to be some custom doubly-linked-list handling going on in
atm/resources.c. Should probably be switched to use <linux/list.h>.
I don't like the look of the locking in ipcommon.c:skb_migrate().
If there's really no better way of doing it, I'd recommend something like:
spinlock_t *first, *second;
if ((unsigned long)from < (unsigned long) to)) {
first = &from->lock;
second = &to->lock;
} else {
first = &to->lock;
second = &from->lock;
}
local_irq_save(flags);
spin_lock(&first);
spin_lock(&second);
(note that you can leave the spin_unlock and spin_unlock_irqrestore
calls as they are).
That's just a quick survey... I'm sure someone who actually uses ATM
could find more things to look at ;-)
-- "It's not Hollywood. War is real, war is primarily not about defeat or victory, it is about death. I've seen thousands and thousands of dead bodies. Do you think I want to have an academic debate on this subject?" -- Robert Fisk - 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/
This archive was generated by hypermail 2b29 : Sun Feb 23 2003 - 22:00:25 EST