Skip to content

Commit

Permalink
firewire: remove global lock around address handlers, convert to RCU
Browse files Browse the repository at this point in the history
Upper-layer handlers for inbound requests were called with a spinlock
held by firewire-core.  Calling into upper layers with a lower layer
lock held is generally a bad idea.

What's more, since commit ea102d0 "firewire: core: convert AR-req
handler lock from _irqsave to _bh", a caller of fw_send_request() i.e.
initiator of outbound request could no longer do that while having
interrupts disabled, if the local node was addressed by that request.

In order to make all this more flexible, convert the management of
address ranges and handlers from a global lock around readers and
writers to RCU (and a remaining global lock for writers).  As a minor
side effect, handling of inbound requests at different cards and of
local requests is now no longer serialized.  (There is still per-card
serialization of remote requests since firewire-ohci uses a single DMA
tasklet for inbound request events.)

In other words, address handlers are now called in an RCU read-side
critical section instead of from within a spin_lock_bh serialized
section.

(Changelog rewritten by Stefan R.)

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
  • Loading branch information
Peter Hurley authored and Stefan Richter committed Sep 28, 2012
1 parent 52439d6 commit 35202f7
Showing 1 changed file with 11 additions and 9 deletions.
20 changes: 11 additions & 9 deletions drivers/firewire/core-transaction.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/module.h>
#include <linux/rculist.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
#include <linux/string.h>
Expand Down Expand Up @@ -489,7 +490,7 @@ static struct fw_address_handler *lookup_overlapping_address_handler(
{
struct fw_address_handler *handler;

list_for_each_entry(handler, list, link) {
list_for_each_entry_rcu(handler, list, link) {
if (handler->offset < offset + length &&
offset < handler->offset + handler->length)
return handler;
Expand All @@ -510,7 +511,7 @@ static struct fw_address_handler *lookup_enclosing_address_handler(
{
struct fw_address_handler *handler;

list_for_each_entry(handler, list, link) {
list_for_each_entry_rcu(handler, list, link) {
if (is_enclosing_handler(handler, offset, length))
return handler;
}
Expand Down Expand Up @@ -588,7 +589,7 @@ int fw_core_add_address_handler(struct fw_address_handler *handler,
if (other != NULL) {
handler->offset += other->length;
} else {
list_add_tail(&handler->link, &address_handler_list);
list_add_tail_rcu(&handler->link, &address_handler_list);
ret = 0;
break;
}
Expand All @@ -609,8 +610,9 @@ EXPORT_SYMBOL(fw_core_add_address_handler);
void fw_core_remove_address_handler(struct fw_address_handler *handler)
{
spin_lock_bh(&address_handler_lock);
list_del(&handler->link);
list_del_rcu(&handler->link);
spin_unlock_bh(&address_handler_lock);
synchronize_rcu();
}
EXPORT_SYMBOL(fw_core_remove_address_handler);

Expand Down Expand Up @@ -844,7 +846,7 @@ static void handle_exclusive_region_request(struct fw_card *card,
if (tcode == TCODE_LOCK_REQUEST)
tcode = 0x10 + HEADER_GET_EXTENDED_TCODE(p->header[3]);

spin_lock_bh(&address_handler_lock);
rcu_read_lock();
handler = lookup_enclosing_address_handler(&address_handler_list,
offset, request->length);
if (handler)
Expand All @@ -853,7 +855,7 @@ static void handle_exclusive_region_request(struct fw_card *card,
p->generation, offset,
request->data, request->length,
handler->callback_data);
spin_unlock_bh(&address_handler_lock);
rcu_read_unlock();

if (!handler)
fw_send_response(card, request, RCODE_ADDRESS_ERROR);
Expand Down Expand Up @@ -886,8 +888,8 @@ static void handle_fcp_region_request(struct fw_card *card,
return;
}

spin_lock_bh(&address_handler_lock);
list_for_each_entry(handler, &address_handler_list, link) {
rcu_read_lock();
list_for_each_entry_rcu(handler, &address_handler_list, link) {
if (is_enclosing_handler(handler, offset, request->length))
handler->address_callback(card, NULL, tcode,
destination, source,
Expand All @@ -896,7 +898,7 @@ static void handle_fcp_region_request(struct fw_card *card,
request->length,
handler->callback_data);
}
spin_unlock_bh(&address_handler_lock);
rcu_read_unlock();

fw_send_response(card, request, RCODE_COMPLETE);
}
Expand Down

0 comments on commit 35202f7

Please sign in to comment.