[PATCH] firewire: core: update fw_device outside of device_find_child()

[Takashi Sakamoto]

When detecting updates of bus topology, the data of fw_device is newly
allocated and caches the content of configuration ROM from the
corresponding node. Then, the tree of device is sought to find the
previous data of fw_device corresponding to the node, since in IEEE 1394
specification numeric node identifier could be changed dynamically every
generation of bus topology. If it is found, the previous data is updated
and reused, then the newly allocated data is going to be released.

The above procedure is done in the call of device_find_child(), however it
is a bit abusing against the intention of the helper function, since the
call would not only find but also update.

This commit splits the update outside of the call.
---
 drivers/firewire/core-device.c | 109 ++++++++++++++++-----------------
 1 file changed, 54 insertions(+), 55 deletions(-)

diff --git a/drivers/firewire/core-device.c b/drivers/firewire/core-device.c
index bc4c9e5a..62e8d839 100644
--- a/drivers/firewire/core-device.c
+++ b/drivers/firewire/core-device.c
@@ -928,56 +928,6 @@ static void fw_device_update(struct work_struct *work)
 	device_for_each_child(&device->device, NULL, update_unit);
 }
 
-/*
- * If a device was pending for deletion because its node went away but its
- * bus info block and root directory header matches that of a newly discovered
- * device, revive the existing fw_device.
- * The newly allocated fw_device becomes obsolete instead.
- */
-static int lookup_existing_device(struct device *dev, void *data)
-{
-	struct fw_device *old = fw_device(dev);
-	struct fw_device *new = data;
-	struct fw_card *card = new->card;
-	int match = 0;
-
-	if (!is_fw_device(dev))
-		return 0;
-
-	guard(rwsem_read)(&fw_device_rwsem); // serialize config_rom access
-	guard(spinlock_irq)(&card->lock); // serialize node access
-
-	if (memcmp(old->config_rom, new->config_rom, 6 * 4) == 0 &&
-	    atomic_cmpxchg(&old->state,
-			   FW_DEVICE_GONE,
-			   FW_DEVICE_RUNNING) == FW_DEVICE_GONE) {
-		struct fw_node *current_node = new->node;
-		struct fw_node *obsolete_node = old->node;
-
-		new->node = obsolete_node;
-		new->node->data = new;
-		old->node = current_node;
-		old->node->data = old;
-
-		old->max_speed = new->max_speed;
-		old->node_id = current_node->node_id;
-		smp_wmb();  /* update node_id before generation */
-		old->generation = card->generation;
-		old->config_rom_retries = 0;
-		fw_notice(card, "rediscovered device %s\n", dev_name(dev));
-
-		old->workfn = fw_device_update;
-		fw_schedule_device_work(old, 0);
-
-		if (current_node == card->root_node)
-			fw_schedule_bm_work(card, 0);
-
-		match = 1;
-	}
-
-	return match;
-}
-
 enum { BC_UNKNOWN = 0, BC_UNIMPLEMENTED, BC_IMPLEMENTED, };
 
 static void set_broadcast_channel(struct fw_device *device, int generation)
@@ -1038,6 +988,17 @@ int fw_device_set_broadcast_channel(struct device *dev, void *gen)
 	return 0;
 }
 
+static int compare_configuration_rom(struct device *dev, void *data)
+{
+	const struct fw_device *old = fw_device(dev);
+	const u32 *config_rom = data;
+
+	if (!is_fw_device(dev))
+		return 0;
+
+	return !!memcmp(old->config_rom, config_rom, 6 * 4);
+}
+
 static void fw_device_init(struct work_struct *work)
 {
 	struct fw_device *device =
@@ -1071,13 +1032,51 @@ static void fw_device_init(struct work_struct *work)
 		return;
 	}
 
-	revived_dev = device_find_child(card->device,
-					device, lookup_existing_device);
+	// If a device was pending for deletion because its node went away but its bus info block
+	// and root directory header matches that of a newly discovered device, revive the
+	// existing fw_device. The newly allocated fw_device becomes obsolete instead.
+	//
+	// serialize config_rom access.
+	scoped_guard(rwsem_read, &fw_device_rwsem) {
+		// TODO: The cast to 'void *' could be removed if Zijun Hu's work goes well.
+		revived_dev = device_find_child(card->device, (void *)device->config_rom,
+						compare_configuration_rom);
+	}
 	if (revived_dev) {
-		put_device(revived_dev);
-		fw_device_release(&device->device);
+		struct fw_device *found = fw_device(revived_dev);
 
-		return;
+		// serialize node access
+		guard(spinlock_irq)(&card->lock);
+
+		if (atomic_cmpxchg(&found->state,
+				   FW_DEVICE_GONE,
+				   FW_DEVICE_RUNNING) == FW_DEVICE_GONE) {
+			struct fw_node *current_node = device->node;
+			struct fw_node *obsolete_node = found->node;
+
+			device->node = obsolete_node;
+			device->node->data = device;
+			found->node = current_node;
+			found->node->data = found;
+
+			found->max_speed = device->max_speed;
+			found->node_id = current_node->node_id;
+			smp_wmb();  /* update node_id before generation */
+			found->generation = card->generation;
+			found->config_rom_retries = 0;
+			fw_notice(card, "rediscovered device %s\n", dev_name(revived_dev));
+
+			found->workfn = fw_device_update;
+			fw_schedule_device_work(found, 0);
+
+			if (current_node == card->root_node)
+				fw_schedule_bm_work(card, 0);
+
+			put_device(revived_dev);
+			fw_device_release(&device->device);
+
+			return;
+		}
 	}
 
 	device_initialize(&device->device);
-- 
2.43.0



Regards

Takashi Sakamoto