Thread (18 messages) 18 messages, 3 authors, 2004-08-17

Re: PATCH: straighten out the IDE layer locking and add hotplug

From: Bartlomiej Zolnierkiewicz <hidden>
Date: 2004-08-17 14:14:31
Also in: lkml

one more comment
quoted
@@ -931,15 +1138,25 @@
  */
 }

-/*
- * Register an IDE interface, specifying exactly the registers etc
- * Set init=1 iff calling before probes have taken place.
+/**
+ *	ide_register_hw		-	register IDE interface
+ *	@hw: hardware registers
+ *	@hwifp: pointer to returned hwif
+ *
+ *	Register an IDE interface, specifying exactly the registers etc
+ *	Set init=1 iff calling before probes have taken place. The
+ *	ide_cfg_sem protects this against races.
+ *
+ *	Returns -1 on error.
  */
+
 int ide_register_hw (hw_regs_t *hw, ide_hwif_t **hwifp)
 {
 	int index, retry = 1;
 	ide_hwif_t *hwif;

+	down(&ide_cfg_sem);
+
 	do {
 		for (index = 0; index < MAX_HWIFS; ++index) {
 			hwif = &ide_hwifs[index];
@@ -950,28 +1167,37 @@
 			hwif = &ide_hwifs[index];
 			if (hwif->hold)
 				continue;
-			if ((!hwif->present && !hwif->mate && !initializing) ||
+			if ((!hwif->configured && !hwif->mate && !initializing) ||
 			    (!hwif->hw.io_ports[IDE_DATA_OFFSET] && initializing))
 				goto found;
 		}
+		/* FIXME- this check should die as should the retry loop */
 		for (index = 0; index < MAX_HWIFS; index++)
-			ide_unregister(index);
+		{
+			hwif = &ide_hwifs[index];
+			__ide_unregister_hwif(hwif);
+		}
 	} while (retry--);
+
+	up(&ide_cfg_sem);
 	return -1;
 found:
-	if (hwif->present)
-		ide_unregister(index);
+	/* FIXME: do we really need this case */
+	if (hwif->configured)
+		__ide_unregister_hwif(hwif);
 	else if (!hwif->hold) {
 		init_hwif_data(hwif, index);
 		init_hwif_default(hwif, index);
 	}
-	if (hwif->present)
+	if (hwif->configured)
 		return -1;
+	hwif->configured = 1;
this is dubious for many non PCI drivers which use ide_register_hw() to only
claim/fill ide_hwifs[] entry but actual probing is done later by ide-generic 
driver - we end up with hwif->present == 0 and hwif->configured == 1
and if ide_register_hw() will try to unregister such hwif it will possibly 
crash (because we now check for ->configured not ->present in 
ide_unregister_hwif) - you've correctly noticed in the FIXMEs that we 
shouldn't be unregistering hwifs in ide_register_hw() but this has some 
side-effects (breaks HDIO_SCAN_HWIF for all non default/generic hwifs
and can change ordering in some rare situations) - we should go that way but 
we need to be fully aware of results of this change
quoted
 	memcpy(&hwif->hw, hw, sizeof(*hw));
 	memcpy(hwif->io_ports, hwif->hw.io_ports, sizeof(hwif->hw.io_ports));
 	hwif->irq = hw->irq;
 	hwif->noprobe = 0;
 	hwif->chipset = hw->chipset;
+	up(&ide_cfg_sem);

 	if (!initializing) {
 		probe_hwif_init(hwif);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help