ata: libata-core: Fix port and device removal

Whenever an ATA adapter driver is removed (e.g. rmmod),
ata_port_detach() is called repeatedly for all the adapter ports to
remove (unload) the devices attached to the port and delete the port
device itself. Removing of devices is done using libata EH with the
ATA_PFLAG_UNLOADING port flag set. This causes libata EH to execute
ata_eh_unload() which disables all devices attached to the port.

ata_port_detach() finishes by calling scsi_remove_host() to remove the
scsi host associated with the port. This function will trigger the
removal of all scsi devices attached to the host and in the case of
disks, calls to sd_shutdown() which will flush the device write cache
and stop the device. However, given that the devices were already
disabled by ata_eh_unload(), the synchronize write cache command and
start stop unit commands fail. E.g. running "rmmod ahci" with first
removing sd_mod results in error messages like:

ata13.00: disable device
sd 0:0:0:0: [sda] Synchronizing SCSI cache
sd 0:0:0:0: [sda] Synchronize Cache(10) failed: Result: hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK
sd 0:0:0:0: [sda] Stopping disk
sd 0:0:0:0: [sda] Start/Stop Unit failed: Result: hostbyte=DID_BAD_TARGET driverbyte=DRIVER_OK

Fix this by removing all scsi devices of the ata devices connected to
the port before scheduling libata EH to disable the ATA devices.

Fixes: 720ba12620ee ("[PATCH] libata-hp: update unload-unplug")
Cc: stable@vger.kernel.org
Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Niklas Cassel <niklas.cassel@wdc.com>
Tested-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 732f3d0b4fd9441f0725bf2c84550888601a159f..8e35afe5e560dac2ef37a0e00508ec7cb6f6d2f4 100644 (file)
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5948,11 +5948,30 @@ static void ata_port_detach(struct ata_port *ap)
        struct ata_link *link;
        struct ata_device *dev;
 
-       /* tell EH we're leaving & flush EH */
+       /* Wait for any ongoing EH */
+       ata_port_wait_eh(ap);
+
+       mutex_lock(&ap->scsi_scan_mutex);
        spin_lock_irqsave(ap->lock, flags);
+
+       /* Remove scsi devices */
+       ata_for_each_link(link, ap, HOST_FIRST) {
+               ata_for_each_dev(dev, link, ALL) {
+                       if (dev->sdev) {
+                               spin_unlock_irqrestore(ap->lock, flags);
+                               scsi_remove_device(dev->sdev);
+                               spin_lock_irqsave(ap->lock, flags);
+                               dev->sdev = NULL;
+                       }
+               }
+       }
+
+       /* Tell EH to disable all devices */
        ap->pflags |= ATA_PFLAG_UNLOADING;
        ata_port_schedule_eh(ap);
+
        spin_unlock_irqrestore(ap->lock, flags);
+       mutex_unlock(&ap->scsi_scan_mutex);
 
        /* wait till EH commits suicide */
        ata_port_wait_eh(ap);