Re: [PATCH][2.5] convert atm_dev_lock from spinlock to semaphore

[Matthew Wilcox <willy@debian.org>]

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