From: Matthew Wilcox <matthew@wil.cx>
To: Stephen Hemminger <shemminger@vyatta.com>
Cc: Jesse Barnes <jbarnes@virtuousgeek.org>,
Ben Hutchings <bhutchings@solarflare.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] PCI: vpd handle longer delays in access
Date: Fri, 5 Sep 2008 06:40:46 -0600 [thread overview]
Message-ID: <20080905124046.GP2772@parisc-linux.org> (raw)
In-Reply-To: <20080904205718.543005986@vyatta.com>
On Thu, Sep 04, 2008 at 01:56:37PM -0700, Stephen Hemminger wrote:
> - udelay(10);
> + if (signal_pending(current))
> + return -EINTR;
If you're going to use _killable instead of _interruptible, this needs
to be fatal_signal_pending(). Otherwise the one who owns the lock can
be interrupted by _any_ signal while those waiting for the lock can only
be interrupted by fatal signals. Which seems daft to me.
> - spin_lock_irq(&vpd->lock);
> + ret = mutex_lock_killable(&vpd->lock);
> + if (ret)
> + return ret;
What's wrong with the shorter:
if (mutex_lock_killable(&vpd->lock))
return -EINTR;
?
The actual error is irrelevant here since userspace will never consume it.
(I agree with Peter about use of yield())
--
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-05 12:41 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-04 20:56 [PATCH 0/3] PCI VPD changes Stephen Hemminger
2008-09-04 20:56 ` [PATCH 0/4] " Stephen Hemminger
2008-09-04 20:56 ` [PATCH 1/4] sky2: set VPD size Stephen Hemminger
2008-09-04 20:56 ` [PATCH 2/4] sky2: add more generic ethtool hooks Stephen Hemminger
2008-09-04 20:56 ` [PATCH 3/4] sky2: move VPD display into debug interface Stephen Hemminger
2008-09-04 20:56 ` [PATCH 4/4] sky2: remove VPD eeprom Stephen Hemminger
2008-09-04 20:56 ` [PATCH 1/3] PCI: vpd handle longer delays in access Stephen Hemminger
2008-09-05 9:11 ` Peter Zijlstra
2008-09-05 12:40 ` Matthew Wilcox [this message]
2008-09-08 20:40 ` Andrew Morton
2008-09-08 21:08 ` Stephen Hemminger
2008-09-08 21:26 ` Arjan van de Ven
2008-09-09 16:55 ` Stephen Hemminger
2008-09-09 17:01 ` Arjan van de Ven
2008-09-04 20:56 ` [PATCH 2/3] PCI: revise VPD access interface Stephen Hemminger
2008-09-05 11:02 ` Ben Hutchings
2008-09-08 20:46 ` Andrew Morton
2008-09-04 20:56 ` [PATCH 3/3] PCI: add interface to set visible size of VPD Stephen Hemminger
2008-09-05 11:07 ` Ben Hutchings
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=20080905124046.GP2772@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=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.