linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Duncan Sands <duncan.sands@wanadoo.fr>
To: chas williams <chas@locutus.cmf.nrl.navy.mil>,
	Mitchell Blank Jr <mitch@sfgoth.com>
Cc: linux-atm-general@lists.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [Linux-ATM-General] Re: [ATM] first pass at fixing atm spinlock
Date: Wed, 19 Mar 2003 18:11:00 +0100	[thread overview]
Message-ID: <200303191811.00337.duncan.sands@wanadoo.fr> (raw)
In-Reply-To: <200303172325.h2HNPpGi015350@locutus.cmf.nrl.navy.mil>

>> atm_dev_lock is now a
>> read/write lock that now protects the list of atm devices.
>
>Are you sure you're never sleeping while holding this lock while iterating
>device lists?  I haven't checked but we do a fair number of things there
>(like the proc stuff) so we really need to track those down.

[I hope the following comments are not misplaced: I picked up this
thread half way through, and I didn't read the code...]

Presumably the only reason you want a spin lock is because drivers
may be walking the list in interrupt context when the time comes
to add/remove a member.  My first comment is: what are drivers
doing walking this list anyway?  Shouldn't it be private to the ATM
layer?  Drivers can maintain their own list if they need one.  My
second comment is that the ATM layer should never need to take
the spinlock itself when walking the list (I'm guessing it always
walks the list in process context).  You could have the following
setup: list protected by a semaphore and a spinlock.

Deleting a member does the following:
	acquire the semaphore
	...
	take the spinlock
	delete the member
	release the spinlock
	...
	release the semaphore

Adding a member does the following:
	acquire the semaphore
	...
	take the spinlock
	add the member
	release the spinlock
	...
	release the semaphore

Walking the list in process context does:
	acquire the semaphore
	walk the list doing stuff
	release the semaphore

Walking the list in interrupt context does:
	acquire the spinlock
	walk the list doing stuff that can't sleep
	release the spinlock

Of course the semaphore could be used as a "big device lock",
not just a list lock.

Duncan.

      parent reply	other threads:[~2003-03-19 17:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <OFAFD4106C.6053931B-ON85256CE0.004F1DD6@gdeb.com>
2003-03-17 22:24 ` [ATM] first pass at fixing atm spinlock chas williams
2003-03-17 23:01   ` Mitchell Blank Jr
2003-03-17 23:25     ` chas williams
2003-03-18  1:13       ` Till Immanuel Patzschke
2003-03-18 16:11         ` chas williams
2003-03-19 10:57       ` Mitchell Blank Jr
2003-03-19 15:13         ` chas williams
2003-03-19 17:11       ` Duncan Sands [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200303191811.00337.duncan.sands@wanadoo.fr \
    --to=duncan.sands@wanadoo.fr \
    --cc=chas@locutus.cmf.nrl.navy.mil \
    --cc=linux-atm-general@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mitch@sfgoth.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).