From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, linux1394-devel@lists.sourceforge.net
Subject: [git pull] FireWire regression fix
Date: Wed, 4 May 2011 13:45:20 +0200 [thread overview]
Message-ID: <20110504134520.0a0aa35e@stein> (raw)
Linus, please pull from the fixes branch at
git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394-2.6.git fixes
to receive the following update for the IEEE 1394 (FireWire) subsystem.
Thanks.
B.J. Buchalter (1):
firewire: Fix for broken configrom updates in quick succession
drivers/firewire/ohci.c | 39 +++++++++++++++++++++++++--------------
1 files changed, 25 insertions(+), 14 deletions(-)
commit 2e053a27d9d5ad5e0831e002cbf8043836fb2060
Author: B.J. Buchalter <bj@mhlabs.com>
Date: Mon May 2 13:33:42 2011 -0400
firewire: Fix for broken configrom updates in quick succession
Current implementation of ohci_set_config_rom() uses a deferred
bus reset via fw_schedule_bus_reset(). If clients add multiple
unit descriptors to the config_rom in quick succession, the
deferred bus reset may not have fired before succeeding update
requests have come in. This can lead to an incorrect partial
update of the config_rom for both addition and removal of
config_rom descriptors, as the ohci_set_config_rom() routine
will return -EBUSY if a previous pending update has not been
completed yet; the requested update just gets dropped on the floor.
This patch recognizes that the "in-flight" update can be modified
until it has been processed by the bus-reset, and the locking
in the bus_reset_tasklet ensures that the update is done atomically
with respect to modifications made by ohci_set_config_rom(). The
-EBUSY error case is simply removed.
[Stefan R: The bug always existed at least theoretically. But it
became easy to trigger since 2.6.36 commit 02d37bed188c "firewire: core:
integrate software-forced bus resets with bus management" which
introduced long mandatory delays between janitorial bus resets.]
Signed-off-by: Benjamin Buchalter <bj@mhlabs.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de> (trivial style changes)
Cc: <stable@kernel.org> # 2.6.36.y and newer
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index f903d7b6..23d1468 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -2199,7 +2199,6 @@ static int ohci_set_config_rom(struct fw_card *card,
{
struct fw_ohci *ohci;
unsigned long flags;
- int ret = -EBUSY;
__be32 *next_config_rom;
dma_addr_t uninitialized_var(next_config_rom_bus);
@@ -2240,22 +2239,37 @@ static int ohci_set_config_rom(struct fw_card *card,
spin_lock_irqsave(&ohci->lock, flags);
+ /*
+ * If there is not an already pending config_rom update,
+ * push our new allocation into the ohci->next_config_rom
+ * and then mark the local variable as null so that we
+ * won't deallocate the new buffer.
+ *
+ * OTOH, if there is a pending config_rom update, just
+ * use that buffer with the new config_rom data, and
+ * let this routine free the unused DMA allocation.
+ */
+
if (ohci->next_config_rom == NULL) {
ohci->next_config_rom = next_config_rom;
ohci->next_config_rom_bus = next_config_rom_bus;
+ next_config_rom = NULL;
+ }
- copy_config_rom(ohci->next_config_rom, config_rom, length);
+ copy_config_rom(ohci->next_config_rom, config_rom, length);
- ohci->next_header = config_rom[0];
- ohci->next_config_rom[0] = 0;
+ ohci->next_header = config_rom[0];
+ ohci->next_config_rom[0] = 0;
- reg_write(ohci, OHCI1394_ConfigROMmap,
- ohci->next_config_rom_bus);
- ret = 0;
- }
+ reg_write(ohci, OHCI1394_ConfigROMmap, ohci->next_config_rom_bus);
spin_unlock_irqrestore(&ohci->lock, flags);
+ /* If we didn't use the DMA allocation, delete it. */
+ if (next_config_rom != NULL)
+ dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE,
+ next_config_rom, next_config_rom_bus);
+
/*
* Now initiate a bus reset to have the changes take
* effect. We clean up the old config rom memory and DMA
@@ -2263,13 +2277,10 @@ static int ohci_set_config_rom(struct fw_card *card,
* controller could need to access it before the bus reset
* takes effect.
*/
- if (ret == 0)
- fw_schedule_bus_reset(&ohci->card, true, true);
- else
- dma_free_coherent(ohci->card.device, CONFIG_ROM_SIZE,
- next_config_rom, next_config_rom_bus);
- return ret;
+ fw_schedule_bus_reset(&ohci->card, true, true);
+
+ return 0;
}
static void ohci_send_request(struct fw_card *card, struct fw_packet *packet)
--
Stefan Richter
-=====-==-== -=-= --=--
http://arcgraph.de/sr/
next reply other threads:[~2011-05-04 11:45 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-04 11:45 Stefan Richter [this message]
-- strict thread matches above, loose matches on Subject: below --
2014-07-26 8:48 [git pull] FireWire regression fix Stefan Richter
2013-07-29 20:33 Stefan Richter
2008-08-06 18:47 Stefan Richter
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=20110504134520.0a0aa35e@stein \
--to=stefanr@s5r6.in-berlin.de \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux1394-devel@lists.sourceforge.net \
--cc=torvalds@linux-foundation.org \
/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.