Re: IDE locking problem

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

And there's more to it... ide_unregister() doesn't copy hwif->hold from
old to new hwif causing my hotswap bay to lose it's iops on next plug,
it doesn't call unregister_device() for neither hwif->gendev nor
drive[n]->gendev, causing the device model list to be corrupted after
an unregister, ...

Here's a patch that makes it work (media bay hotswap works and doesn't
break the device model device list), however, locking is probably broken
as hell (I just release the spinlock here or there).

I'm not sure what is the best way to fix that locking issue. Regarding
userland access, I beleive the settings semaphore is the way to go.
Regarding the blk queue, I don't know what's the "official" way to
atomically tear down a queue, making sure we properly complete (fail ?)
requests that may have slipped in, and make sure that queue won't be
called again, Jens ? In most cases, once that's done, the rest of
the locking in ide_unregister() is mostly to guard against userland
access (settings) and other ide_register() trying to take the slot
while we are still cleaning up, which can be dealt either with a 
"I'm busy housekeeping" flag in hwif, or a semaphore.

What do you think ?

In the meantime, here's the patch that at least makes it work, though
it's probably still racy: (Against 2.6-test2)

(Note to PPC users: there are other bugs in current
drivers/macintosh/mediabay.c in there that need fixes I have here, those
will be pushed separately).

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1008  -> 1.1009 
#	   drivers/ide/ide.c	1.58    -> 1.59   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/08/03	benh@kernel.crashing.org	1.1009
# Fixes to ide_unregister() to avoid device list corruption and
# scheduling at interrupt time. Also make sure "hold" flag is
# properly transferred from old to new interface
# --------------------------------------------
#
diff -Nru a/drivers/ide/ide.c b/drivers/ide/ide.c
--- a/drivers/ide/ide.c	Sun Aug  3 11:57:56 2003
+++ b/drivers/ide/ide.c	Sun Aug  3 11:57:56 2003
@@ -689,8 +689,8 @@
 #ifdef CONFIG_PROC_FS
 	destroy_proc_ide_drives(hwif);
 #endif
-	hwgroup = hwif->hwgroup;
 
+	hwgroup = hwif->hwgroup;
 	/*
 	 * free the irq if we were the only hwif using it
 	 */
@@ -745,7 +745,11 @@
 			drive->id = NULL;
 		}
 		drive->present = 0;
+		/* Fucked up locking ... */
+		spin_unlock_irq(&ide_lock);
 		blk_cleanup_queue(&drive->queue);
+		device_unregister(&drive->gendev);
+		spin_lock_irq(&ide_lock);
 	}
 	if (hwif->next == hwif) {
 		BUG_ON(hwgroup->hwif != hwif);
@@ -770,6 +774,11 @@
 		BUG_ON(hwgroup->hwif == hwif);
 	}
 
+	/* More fucked up locking ... */
+	spin_unlock_irq(&ide_lock);
+	device_unregister(&hwif->gendev);
+	spin_lock_irq(&ide_lock);
+
 #if !defined(CONFIG_DMA_NONPCI)
 	if (hwif->dma_base) {
 		(void) ide_release_dma(hwif);
@@ -794,6 +803,7 @@
 		put_disk(disk);
 	}
 	unregister_blkdev(hwif->major, hwif->name);
+
 	old_hwif			= *hwif;
 	init_hwif_data(index);	/* restore hwif data to pristine status */
 	hwif->hwgroup			= old_hwif.hwgroup;
@@ -812,6 +822,7 @@
 	hwif->swdma_mask		= old_hwif.swdma_mask;
 
 	hwif->chipset			= old_hwif.chipset;
+	hwif->hold			= old_hwif.hold;
 
 #ifdef CONFIG_BLK_DEV_IDEPCI
 	hwif->pci_dev			= old_hwif.pci_dev;
@@ -864,6 +875,13 @@
 	hwif->ide_dma_retune		= old_hwif.ide_dma_retune;
 	hwif->ide_dma_lostirq		= old_hwif.ide_dma_lostirq;
 	hwif->ide_dma_timeout		= old_hwif.ide_dma_timeout;
+	hwif->ide_dma_queued_on		= old_hwif.ide_dma_queued_on;
+	hwif->ide_dma_queued_off	= old_hwif.ide_dma_queued_off;
+#ifdef CONFIG_BLK_DEV_IDE_TCQ
+	hwif->ide_dma_queued_read	= old_hwif.ide_dma_queued_read;
+	hwif->ide_dma_queued_write	= old_hwif.ide_dma_queued_write;
+	hwif->ide_dma_queued_start	= old_hwif.ide_dma_queued_start;
+#endif
 #endif
 
 #if 0