lkml.org 
[lkml]   [2003]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [ATM] second pass at fixing atm spinlock
Date
Hi Chas, good work.  A few comments below

On Thursday 27 March 2003 23:28, chas williams wrote:
> ok, here is a further attempt at fixing the smp problems in the linux-atm
> stack. things have been moved around in net/atm:
>
> . atm sk's are reference counted. this makes the vcc_release (renamed
> from atm_release_vcc for consistency) and svc_release easier to write
> since you dont need to explicity free the sk
> . vcc's are stored as a list of sockets, like other protocols
> . atm drivers should use vcc_hold/vcc_put to modify reference counts. the
> driver should read_lock(vcc_sklist_lock) before doing a vcc_hold() if
> they are in their 'bottom half'. i converted eni, idt77252 (untested --
> no hardware), fore200e (untest -- lazy) and nicstar. i also included the
> he driver. the eni driver is special, since the bottom half is a tasklet
> you can simply stop the bottom half and be sure the bottom half
> isnt holding a reference. the he just grabs the vcc_sklist_lock and
> processes all the vcc's needing serviced. a vcc cant disappear while
> the lock is held (since the vcc_release needs to do a write_lock to
> unhash the socket/vcc before dropping the refcnt to 0).

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 do it if you are just taking a reference. After all, you should only take a
reference to a vcc (ok, to the socket) if the vcc you are going to use/copy
already has a reference taken [Example: if the ATM layer calls you and
passes you a vcc, it must have taken a reference to the vcc before calling
you, otherwise the vcc might vanish under you; then you take a reference
to this vcc -> ref count = 2]. So no-one is going to delete the vcc under
you, so why take a lock? As I said, I'm kind of confused here.

> . the nodev list has been removed and bind_vcc/unlink have been changed to
> just insert/remove vcc list operations. this makes things a bit clearer.
> . driver's check_ci() routine have been removed in favor of the one in
> atm_misc.c. very few were actually any faster, and this functionality is
> rarely used.

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
more than one vcc for the same vpi/vci pair? Also, calling check_ci then
claiming the vpi/vci pair is racy. If you want to have something like this,
shouldn't there be a routine "claim_ci", which either returns an error if that
vpi/vci pair was already in use, or marks it as in use if not (done atomically
of course).

> . there is a new function, vcc_lookup(), used mostly by the fore200e and he
> driver. vcc's returned by this function will need a vcc_put() or the
> ref count will be wrong. in the future, most drivers should probably
> use this function to find the vcc for their newly arrived data. there
> possibly could be a routine atmif_rx(atmdev, skb, vpi, vci) which
> just does the right thing. ideally, atm would use netif_rx() but one
> would need to dummy up a ethernet header on each of the pdu's. comments
> on that idea?
> . all the stuff from the previous version: atmdev ref counting, atm and
> clip can now build as modules.
>
> ftp://ftp.cmf.nrl.navy.mil/pub/chas/linux-atm/2_5_64_atm_dev_lock.patch
>
> [i will try to get a 2.5.65 version available this weekend]

Good stuff!

Ciao,

Duncan.
-
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:0.143 / U:0.896 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site