From: Matthew Wilcox <matthew@wil.cx>
To: Ben Hutchings <bhutchings@solarflare.com>
Cc: Stephen Hemminger <shemminger@vyatta.com>,
Jesse Barnes <jbarnes@virtuousgeek.org>,
linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-pci@vger.kernel.org
Subject: Re: [PATCH 1/3] pci: VPD access timeout increase
Date: Thu, 4 Sep 2008 10:10:51 -0600 [thread overview]
Message-ID: <20080904161051.GL2772@parisc-linux.org> (raw)
In-Reply-To: <20080904141944.GE7908@solarflare.com>
On Thu, Sep 04, 2008 at 03:19:46PM +0100, Ben Hutchings wrote:
> Matthew Wilcox wrote:
> > On Wed, Sep 03, 2008 at 03:57:13PM -0700, Stephen Hemminger wrote:
> > > Accessing the VPD area can take a long time. There are comments in the
> > > SysKonnect vendor driver that it can take up to 25ms. The existing vpd
> > > access code fails consistently on my hardware.
> >
> > Wow, that's slow. If you were to try to read all 32k, it'd take more
> > than three minutes! (I presume it doesn't actually have as much as 32k).
> >
> > > Change the access routines to:
> > > * use a mutex rather than spinning with IRQ's disabled and lock held
> > > * have a longer timeout
> > > * call schedule while spinning to provide some responsivness
> >
> > I agree with your approach, but have one minor comment:
> >
> > > - spin_lock_irq(&vpd->lock);
> > > + mutex_lock(&vpd->lock);
> >
> > This should be:
> >
> > + if (mutex_lock_interruptible(&vpd->lock))
> > + return -EINTR;
> [...]
>
> This is fine for the sysfs case, but not if this is called during device
> probe - we don't want signals to modprobe to break device initialisation,
> do we?
Probably only fatal signals -- in which case the if (signal_pending)
check should be a fatal_signal_pending() and mutex_lock_interruptible()
should be mutex_lock_killable().
OTOH, who's signalling modprobe to do anything other than die?
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
next prev parent reply other threads:[~2008-09-04 16:11 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-28 3:46 [PATCH 1/2] sky2: EEPROM read/write bug fixes Stephen Hemminger
2008-08-28 3:48 ` [PATCH 2/2] sky2: display product info on boot Stephen Hemminger
2008-08-28 11:13 ` [PATCH 1/2] sky2: EEPROM read/write bug fixes Ben Hutchings
2008-08-28 15:30 ` Stephen Hemminger
2008-08-30 15:03 ` Stephen Hemminger
2008-08-31 20:35 ` Ben Hutchings
2008-08-31 23:24 ` Stephen Hemminger
[not found] ` <20080903155316.1a0a5698@extreme>
2008-09-03 22:57 ` [PATCH 2/3] pci: revise VPD access interface Stephen Hemminger
2008-09-03 23:00 ` [PATCH 3/3] sky2: use pci_read_vpd to read info during boot Stephen Hemminger
2008-09-04 7:36 ` Jeff Garzik
2008-09-09 4:36 ` Jesse Barnes
2008-09-03 22:57 ` [PATCH 1/3] pci: VPD access timeout increase Stephen Hemminger
2008-09-04 12:52 ` Matthew Wilcox
2008-09-04 14:19 ` Ben Hutchings
2008-09-04 16:10 ` Matthew Wilcox [this message]
2008-09-04 16:32 ` Stephen Hemminger
2008-09-04 16:07 ` [PATCH] Return value from schedule() Matthew Wilcox
2008-09-04 16:14 ` Ingo Molnar
2008-09-04 16:21 ` Matthew Wilcox
2008-09-04 17:30 ` Arjan van de Ven
2008-09-04 17:48 ` Matthew Wilcox
2008-09-04 19:05 ` Stephen Hemminger
2008-09-05 7:40 ` Peter Zijlstra
2008-09-03 14:25 ` [PATCH 1/2] sky2: EEPROM read/write bug fixes Jeff Garzik
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=20080904161051.GL2772@parisc-linux.org \
--to=matthew@wil.cx \
--cc=bhutchings@solarflare.com \
--cc=jbarnes@virtuousgeek.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=shemminger@vyatta.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.