From 621f6dd715209d3c3c27841943ae71fc2c75c9f5 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sun, 26 Oct 2008 11:04:20 +0100 Subject: firewire: fw-sbp2: remove unnecessary locking What was I thinking when I added sbp2_set_generation()? Its locking did nothing (except for implicitly providing the necessary barrier between node IDs update and generation update). Signed-off-by: Stefan Richter --- drivers/firewire/fw-sbp2.c | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c index e54403e..e88d506 100644 --- a/drivers/firewire/fw-sbp2.c +++ b/drivers/firewire/fw-sbp2.c @@ -670,17 +670,6 @@ static void sbp2_agent_reset_no_wait(struct sbp2_logical_unit *lu) &d, sizeof(d), complete_agent_reset_write_no_wait, t); } -static void sbp2_set_generation(struct sbp2_logical_unit *lu, int generation) -{ - struct fw_card *card = fw_device(lu->tgt->unit->device.parent)->card; - unsigned long flags; - - /* serialize with comparisons of lu->generation and card->generation */ - spin_lock_irqsave(&card->lock, flags); - lu->generation = generation; - spin_unlock_irqrestore(&card->lock, flags); -} - static inline void sbp2_allow_block(struct sbp2_logical_unit *lu) { /* @@ -884,7 +873,7 @@ static void sbp2_login(struct work_struct *work) goto out; generation = device->generation; - smp_rmb(); /* node_id must not be older than generation */ + smp_rmb(); /* node IDs must not be older than generation */ node_id = device->node_id; local_node_id = device->card->node_id; @@ -908,7 +897,8 @@ static void sbp2_login(struct work_struct *work) tgt->node_id = node_id; tgt->address_high = local_node_id << 16; - sbp2_set_generation(lu, generation); + smp_wmb(); /* node IDs must not be older than generation */ + lu->generation = generation; lu->command_block_agent_address = ((u64)(be32_to_cpu(response.command_block_agent.high) & 0xffff) @@ -1201,7 +1191,7 @@ static void sbp2_reconnect(struct work_struct *work) goto out; generation = device->generation; - smp_rmb(); /* node_id must not be older than generation */ + smp_rmb(); /* node IDs must not be older than generation */ node_id = device->node_id; local_node_id = device->card->node_id; @@ -1228,7 +1218,8 @@ static void sbp2_reconnect(struct work_struct *work) tgt->node_id = node_id; tgt->address_high = local_node_id << 16; - sbp2_set_generation(lu, generation); + smp_wmb(); /* node IDs must not be older than generation */ + lu->generation = generation; fw_notify("%s: reconnected to LUN %04x (%d retries)\n", tgt->bus_id, lu->lun, lu->retries); -- cgit v1.1 From d6053e08f5520dcb58c200d2e1861d9c505b72e8 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Mon, 24 Nov 2008 20:40:00 +0100 Subject: firewire: fix small memory leak at module removal Signed-off-by: Stefan Richter --- drivers/firewire/fw-device.c | 2 +- drivers/firewire/fw-device.h | 2 ++ drivers/firewire/fw-transaction.c | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index 6b9be42..31b6c74 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -617,7 +617,7 @@ static int shutdown_unit(struct device *device, void *data) */ DECLARE_RWSEM(fw_device_rwsem); -static DEFINE_IDR(fw_device_idr); +DEFINE_IDR(fw_device_idr); int fw_cdev_major; struct fw_device *fw_device_get_by_devt(dev_t devt) diff --git a/drivers/firewire/fw-device.h b/drivers/firewire/fw-device.h index 42305bb..df51732 100644 --- a/drivers/firewire/fw-device.h +++ b/drivers/firewire/fw-device.h @@ -21,6 +21,7 @@ #include #include +#include #include #include @@ -99,6 +100,7 @@ void fw_device_cdev_update(struct fw_device *device); void fw_device_cdev_remove(struct fw_device *device); extern struct rw_semaphore fw_device_rwsem; +extern struct idr fw_device_idr; extern int fw_cdev_major; /* diff --git a/drivers/firewire/fw-transaction.c b/drivers/firewire/fw-transaction.c index 2884f87..699ac04 100644 --- a/drivers/firewire/fw-transaction.c +++ b/drivers/firewire/fw-transaction.c @@ -19,6 +19,7 @@ */ #include +#include #include #include #include @@ -971,6 +972,7 @@ static void __exit fw_core_cleanup(void) { unregister_chrdev(fw_cdev_major, "firewire"); bus_unregister(&fw_bus_type); + idr_destroy(&fw_device_idr); } module_init(fw_core_init); -- cgit v1.1 From 2cc489c21338950c2b4097dec48864bdf7b30f1b Mon Sep 17 00:00:00 2001 From: Jay Fenlason Date: Wed, 22 Oct 2008 15:59:42 -0400 Subject: firewire: typo in comment Signed-off-by: Jay Fenlason Signed-off-by: Stefan Richter --- drivers/firewire/fw-card.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/fw-card.c b/drivers/firewire/fw-card.c index 418c18f..c776079 100644 --- a/drivers/firewire/fw-card.c +++ b/drivers/firewire/fw-card.c @@ -75,7 +75,7 @@ generate_config_rom(struct fw_card *card, size_t *config_rom_length) * controller, block reads to the config rom accesses the host * memory, but quadlet read access the hardware bus info block * registers. That's just crack, but it means we should make - * sure the contents of bus info block in host memory mathces + * sure the contents of bus info block in host memory matches * the version stored in the OHCI registers. */ -- cgit v1.1 From 0fa1986f3a6c385b3bca0b6a051c30e548bda30d Mon Sep 17 00:00:00 2001 From: Jay Fenlason Date: Sat, 29 Nov 2008 17:44:57 +0100 Subject: firewire: improve refcounting of fw_card Take a reference to the card whenever fw_card_bm_work() is scheduled on that card and release it when the work is done. This allows us to remove the cancel_delayed_work_sync() in fw_core_remove_card(). Signed-off-by: Jay Fenlason Signed-off-by: Stefan Richter (patch update) --- drivers/firewire/fw-card.c | 18 +++++++++++++++--- drivers/firewire/fw-device.c | 6 +++--- drivers/firewire/fw-topology.c | 2 +- drivers/firewire/fw-transaction.h | 2 ++ 4 files changed, 21 insertions(+), 7 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/fw-card.c b/drivers/firewire/fw-card.c index c776079..799f944 100644 --- a/drivers/firewire/fw-card.c +++ b/drivers/firewire/fw-card.c @@ -189,6 +189,17 @@ static const char gap_count_table[] = { 63, 5, 7, 8, 10, 13, 16, 18, 21, 24, 26, 29, 32, 35, 37, 40 }; +void +fw_schedule_bm_work(struct fw_card *card, unsigned long delay) +{ + int scheduled; + + fw_card_get(card); + scheduled = schedule_delayed_work(&card->work, delay); + if (!scheduled) + fw_card_put(card); +} + static void fw_card_bm_work(struct work_struct *work) { @@ -206,7 +217,7 @@ fw_card_bm_work(struct work_struct *work) if (local_node == NULL) { spin_unlock_irqrestore(&card->lock, flags); - return; + goto out_put_card; } fw_node_get(local_node); fw_node_get(root_node); @@ -280,7 +291,7 @@ fw_card_bm_work(struct work_struct *work) * this task 100ms from now. */ spin_unlock_irqrestore(&card->lock, flags); - schedule_delayed_work(&card->work, DIV_ROUND_UP(HZ, 10)); + fw_schedule_bm_work(card, DIV_ROUND_UP(HZ, 10)); goto out; } @@ -355,6 +366,8 @@ fw_card_bm_work(struct work_struct *work) fw_device_put(root_device); fw_node_put(root_node); fw_node_put(local_node); + out_put_card: + fw_card_put(card); } static void @@ -510,7 +523,6 @@ fw_core_remove_card(struct fw_card *card) fw_card_put(card); wait_for_completion(&card->done); - cancel_delayed_work_sync(&card->work); WARN_ON(!list_empty(&card->transaction_list)); del_timer_sync(&card->flush_timer); } diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index 31b6c74..c173be3 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -689,7 +689,7 @@ static void fw_device_init(struct work_struct *work) fw_notify("giving up on config rom for node id %x\n", device->node_id); if (device->node == device->card->root_node) - schedule_delayed_work(&device->card->work, 0); + fw_schedule_bm_work(device->card, 0); fw_device_release(&device->device); } return; @@ -758,7 +758,7 @@ static void fw_device_init(struct work_struct *work) * pretty harmless. */ if (device->node == device->card->root_node) - schedule_delayed_work(&device->card->work, 0); + fw_schedule_bm_work(device->card, 0); return; @@ -892,7 +892,7 @@ static void fw_device_refresh(struct work_struct *work) fw_device_shutdown(work); out: if (node_id == card->root_node->node_id) - schedule_delayed_work(&card->work, 0); + fw_schedule_bm_work(card, 0); } void fw_node_event(struct fw_card *card, struct fw_node *node, int event) diff --git a/drivers/firewire/fw-topology.c b/drivers/firewire/fw-topology.c index 5e20471..7687dca 100644 --- a/drivers/firewire/fw-topology.c +++ b/drivers/firewire/fw-topology.c @@ -530,7 +530,7 @@ fw_core_handle_bus_reset(struct fw_card *card, smp_wmb(); card->generation = generation; card->reset_jiffies = jiffies; - schedule_delayed_work(&card->work, 0); + fw_schedule_bm_work(card, 0); local_node = build_tree(card, self_ids, self_id_count); diff --git a/drivers/firewire/fw-transaction.h b/drivers/firewire/fw-transaction.h index 839466f0..0497a18 100644 --- a/drivers/firewire/fw-transaction.h +++ b/drivers/firewire/fw-transaction.h @@ -278,6 +278,8 @@ static inline void fw_card_put(struct fw_card *card) kref_put(&card->kref, fw_card_release); } +extern void fw_schedule_bm_work(struct fw_card *card, unsigned long delay); + /* * The iso packet format allows for an immediate header/payload part * stored in 'header' immediately after the packet info plus an -- cgit v1.1 From d6f95a3d14dc403881b23ad268ec1e3600c4e6b4 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 29 Nov 2008 18:56:47 +0100 Subject: firewire: fix resetting of bus manager retry counter An earlier change, maybe long ago, removed the copying of self_id_count into card->self_id_count. Since then each bus reset cleared card->bm_retries even when it shouldn't. Signed-off-by: Stefan Richter --- drivers/firewire/fw-topology.c | 14 ++++++-------- drivers/firewire/fw-transaction.h | 1 - 2 files changed, 6 insertions(+), 9 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/fw-topology.c b/drivers/firewire/fw-topology.c index 7687dca..c9be6e6 100644 --- a/drivers/firewire/fw-topology.c +++ b/drivers/firewire/fw-topology.c @@ -355,6 +355,9 @@ report_lost_node(struct fw_card *card, { fw_node_event(card, node, FW_NODE_DESTROYED); fw_node_put(node); + + /* Topology has changed - reset bus manager retry counter */ + card->bm_retries = 0; } static void @@ -374,6 +377,9 @@ report_found_node(struct fw_card *card, } fw_node_event(card, node, FW_NODE_CREATED); + + /* Topology has changed - reset bus manager retry counter */ + card->bm_retries = 0; } void fw_destroy_nodes(struct fw_card *card) @@ -514,14 +520,6 @@ fw_core_handle_bus_reset(struct fw_card *card, spin_lock_irqsave(&card->lock, flags); - /* - * If the new topology has a different self_id_count the topology - * changed, either nodes were added or removed. In that case we - * reset the IRM reset counter. - */ - if (card->self_id_count != self_id_count) - card->bm_retries = 0; - card->node_id = node_id; /* * Update node_id before generation to prevent anybody from using diff --git a/drivers/firewire/fw-transaction.h b/drivers/firewire/fw-transaction.h index 0497a18..5a57bb8 100644 --- a/drivers/firewire/fw-transaction.h +++ b/drivers/firewire/fw-transaction.h @@ -241,7 +241,6 @@ struct fw_card { * We need to store up to 4 self ID for a maximum of 63 * devices plus 3 words for the topology map header. */ - int self_id_count; u32 topology_map[252 + 3]; u32 broadcast_channel; -- cgit v1.1 From c8a12d45d543905a2718fccafd612edbd73a1341 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Sat, 29 Nov 2008 19:00:56 +0100 Subject: firewire: reorder struct fw_card for better cache efficiency topology_map is by far the largest member in struct fw_card. Move it to the very end of the struct so that card pointer dereferences have better chances to hit the CPU cache. This requires to increase the topology_map backing store to the size specified in IEEE 1394, i.e. 256 rather than 255 quadlets. Otherwise the topology_map response handler may access invalid memory. Signed-off-by: Stefan Richter --- drivers/firewire/fw-transaction.h | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/fw-transaction.h b/drivers/firewire/fw-transaction.h index 5a57bb8..c9ab12a 100644 --- a/drivers/firewire/fw-transaction.h +++ b/drivers/firewire/fw-transaction.h @@ -237,13 +237,6 @@ struct fw_card { int link_speed; int config_rom_generation; - /* - * We need to store up to 4 self ID for a maximum of 63 - * devices plus 3 words for the topology map header. - */ - u32 topology_map[252 + 3]; - u32 broadcast_channel; - spinlock_t lock; /* Take this lock when handling the lists in * this struct. */ struct fw_node *local_node; @@ -261,6 +254,9 @@ struct fw_card { struct delayed_work work; int bm_retries; int bm_generation; + + u32 broadcast_channel; + u32 topology_map[(CSR_TOPOLOGY_MAP_END - CSR_TOPOLOGY_MAP) / 4]; }; static inline struct fw_card *fw_card_get(struct fw_card *card) -- cgit v1.1 From 6230582320b721e6cf2581d048cb688dca97f504 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Fri, 9 Jan 2009 20:49:37 +0100 Subject: firewire: core: fix sleep in atomic context due to driver core change Due to commit 2831fe6f9cc4e16c103504ee09a47a084297c0f3, "driver core: create a private portion of struct device", device_initialize() can no longer be called from atomic contexts. We now defer it until after config ROM probing. This requires changes to the bus manager code because this may use a device before it was probed. Reported-by: Jay Fenlason Signed-off-by: Stefan Richter --- drivers/firewire/fw-card.c | 13 +++++++------ drivers/firewire/fw-device.c | 23 +++++++++++++---------- 2 files changed, 20 insertions(+), 16 deletions(-) (limited to 'drivers/firewire') diff --git a/drivers/firewire/fw-card.c b/drivers/firewire/fw-card.c index 799f944..6bd91a1 100644 --- a/drivers/firewire/fw-card.c +++ b/drivers/firewire/fw-card.c @@ -209,6 +209,8 @@ fw_card_bm_work(struct work_struct *work) unsigned long flags; int root_id, new_root_id, irm_id, gap_count, generation, grace, rcode; bool do_reset = false; + bool root_device_is_running; + bool root_device_is_cmc; __be32 lock_data[2]; spin_lock_irqsave(&card->lock, flags); @@ -224,8 +226,9 @@ fw_card_bm_work(struct work_struct *work) generation = card->generation; root_device = root_node->data; - if (root_device) - fw_device_get(root_device); + root_device_is_running = root_device && + atomic_read(&root_device->state) == FW_DEVICE_RUNNING; + root_device_is_cmc = root_device && root_device->cmc; root_id = root_node->node_id; grace = time_after(jiffies, card->reset_jiffies + DIV_ROUND_UP(HZ, 10)); @@ -308,14 +311,14 @@ fw_card_bm_work(struct work_struct *work) * config rom. In either case, pick another root. */ new_root_id = local_node->node_id; - } else if (atomic_read(&root_device->state) != FW_DEVICE_RUNNING) { + } else if (!root_device_is_running) { /* * If we haven't probed this device yet, bail out now * and let's try again once that's done. */ spin_unlock_irqrestore(&card->lock, flags); goto out; - } else if (root_device->cmc) { + } else if (root_device_is_cmc) { /* * FIXME: I suppose we should set the cmstr bit in the * STATE_CLEAR register of this node, as described in @@ -362,8 +365,6 @@ fw_card_bm_work(struct work_struct *work) fw_core_initiate_bus_reset(card, 1); } out: - if (root_device) - fw_device_put(root_device); fw_node_put(root_node); fw_node_put(local_node); out_put_card: diff --git a/drivers/firewire/fw-device.c b/drivers/firewire/fw-device.c index c173be3..2af5a8d 100644 --- a/drivers/firewire/fw-device.c +++ b/drivers/firewire/fw-device.c @@ -159,7 +159,8 @@ static void fw_device_release(struct device *dev) /* * Take the card lock so we don't set this to NULL while a - * FW_NODE_UPDATED callback is being handled. + * FW_NODE_UPDATED callback is being handled or while the + * bus manager work looks at this node. */ spin_lock_irqsave(&card->lock, flags); device->node->data = NULL; @@ -695,12 +696,13 @@ static void fw_device_init(struct work_struct *work) return; } - err = -ENOMEM; + device_initialize(&device->device); fw_device_get(device); down_write(&fw_device_rwsem); - if (idr_pre_get(&fw_device_idr, GFP_KERNEL)) - err = idr_get_new(&fw_device_idr, device, &minor); + err = idr_pre_get(&fw_device_idr, GFP_KERNEL) ? + idr_get_new(&fw_device_idr, device, &minor) : + -ENOMEM; up_write(&fw_device_rwsem); if (err < 0) @@ -911,13 +913,14 @@ void fw_node_event(struct fw_card *card, struct fw_node *node, int event) /* * Do minimal intialization of the device here, the - * rest will happen in fw_device_init(). We need the - * card and node so we can read the config rom and we - * need to do device_initialize() now so - * device_for_each_child() in FW_NODE_UPDATED is - * doesn't freak out. + * rest will happen in fw_device_init(). + * + * Attention: A lot of things, even fw_device_get(), + * cannot be done before fw_device_init() finished! + * You can basically just check device->state and + * schedule work until then, but only while holding + * card->lock. */ - device_initialize(&device->device); atomic_set(&device->state, FW_DEVICE_INITIALIZING); device->card = fw_card_get(card); device->node = fw_node_get(node); -- cgit v1.1