lkml.org 
[lkml]   [2003]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [ATM] second pass at fixing atm spinlock
Date
From
In message <200303281107.07586.baldrick@wanadoo.fr>,Duncan Sands writes:
>I'm kind of confused about this. It seems to me that you should only
>need to read_lock(vcc_sklist_lock) if you are going to traverse (or
>otherwise examine the structure of) the list. There should be no need

to prevent trouble with walking a list that might be changing while
you are walking it and to synchronize the release and "bottom half"
operation of the atm drivers. i came up with this as the worst
possible case (of course its one of those lookup via big index table
drivers):

driver->open() BH() vcc_release() sk.refcnt

ENTER
...
vcc_hold(vcc) 2
rx_vcc->vcc = vcc
...
EXIT
ENTER
read_lock(sklist)
vcc = rx_vcc->vcc
...
ENTER
driver->close()
...
rx_vcc->vcc = NULL
barrier()
vcc_put(vcc) 1
...
write_lock(sklist) [MUST WAIT]

vcc_hold(vcc) 2
read_unlock(sklist)
...
...
vcc_remove_socket()
write_unlock(sklist)
sock_put(vcc); 1
EXIT

...
vcc->push(skb)
vcc_put(vcc) 0
EXIT

if you didnt read_lock(sklist) then vcc_release() could (it is somewhat
unlikely) unhash the socket and vcc_put() (its really just sock_put) before
the BH has a chance to vcc_hold(vcc). the { vcc = rx_vcc->vcc; vcc_hold(vcc); }
isnt an atomic operation and that is were you run into trouble. you
could remove the hold/put for for the table references and this still
works but putting the vcc in the table is a reference. you should count it.
however, the nicstar/idt77252 probably still have a race since
the vc && vc->vcc in the BH isnt protected.
>Why does this exist at all? I mean, if someone has already opened a vcc for
>a given vpi/vci pair, the ATM layer could detect this itself and return an error,
>without ever calling the driver's open method. Is it sometimes useful to open

mitch and i have had this discussion. it seems like its probably a good
idea to move this to the upper layer and possibly remove the ATM_ANY_VPI/VCI
functionality. it probably should be claim_ci, and insert the entry into
the list during operation. as it stands there is still a race beween
find_ci() and vcc_insert_socket().
-
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/

\
 
 \ /
  Last update: 2005-03-22 13:34    [W:1.897 / U:0.360 seconds]
©2003-2011 Jasper Spaans. Advertise on this site