David S. Miller wrote:
> Although, unless VCC connect can potentially sleep, it might
> be better to keep exporting the rwlock and take it as a reader
> instead of grabbing the rtnl semaphore.
VCC connect can definately sleep - a good example is
drirvers/usb/misc/speedtch.c but there are probably other ones.
My personal recommendations:
* There should be a per-atm-device semaphore held across calls into the
driver's ->open, ->close, ->change_qos and maybe a couple other things
to serialize those operations (for the sake of keeping the drivers
sane - there's no reason there should be multiple operations pending)
* The ATM layer shouldn't need to keep any other sorts of locks across
calls into atm_dev->open or such. To avoid race conditions the ATM
code needs to:
1. Take whatever internal spinlock it needs
2. Add the itf/vpi/vci tuple to whatever datastructure we're holding
the vcc list in - but mark it as "inactive" so we don't find it
while others are searching the list. Obviously the open will
fail if we find a matching tuple (whether or not it was active)
3. Drop the spinlock
4. Take the per-device semaphore
5. Call atmdev->open
6. Drop the per-device semaphore
7. Re-take the internal spinlock
8. If open succeeded now mark the vcc as "active". If the open failed
just delete it
9. Drop the internal spinlock
To do a close the method is similar:
1. Take the internal spinlock
2. Find the tuple, mark it as inactive
3. Drop the spinlock
4. Take the per-device semaphore
5. Call atmdev->close
6. Drop the per-device semaphore
7. Re-take the internal spinlock
8. Delete vcc from the list
9. Drop the internal spinlock
Then ->open and ->close can sleep like they need to but won't block
other accesses to the list in the mean time.
* If ATM_ITF_ANY is a pain to fix it should just be removed - it's
basically a misfeature and is probably never used by anybody. I've
already posted a patch to linux-atm-general that basically does this
(it changes "ATM_ITF_ANY" to mean "first interface we find")
-Mitch
|