Hey Jeff, all, Patch 2/3 for libata hotswapping, the 'general purpose glitch-free incredibly awesome' hotswapping API. Lots of comments available in patch and header, please review, apply, and convert other drivers to it if you like it. Refer to patch 3 for a reference implementation. Luke Kosewski
26.09.05 Luke Kosewski <lkosewsk@xxxxxx> * A patch to add a general-purpose hotswap framework to libata. This is resend #4 wherein we maintain send #3's API! Driver writers call ata_hotplug_plug or ata_hotplug_unplug when a plug/unplug event occurs - it takes care of the rest. The idea is to make the sequence of events as straightforward as possible and not clutter the code with exceptions; as a result, a series of 'hooks' can be provided at points in the code for drivers that need it (such as the Silicon Image drivers which need a reset performed on device removal). See 'hotplug_(un)plug_janitor' for more info. * This edition is very streamlined. I like it. An area for improvement might facilitate the removal of the ata_scsi_prepare_qc_abort function in favour of changes to ata_qc_complete, but that can be up to debate. Basically this could probably be slightly more readable if more EH code were in place. * Small change here to ata_scsi_handle_unplug to set SDEV_CANCEL on a drive before killing outstanding qc's, or else we might see a new qc be queued before the CANCEL is set, which leads to timeouts. diff -rpuN linux-2.6.14-rc1/drivers/scsi/libata-core.c linux-2.6.14-rc1-new/drivers/scsi/libata-core.c --- linux-2.6.14-rc1/drivers/scsi/libata-core.c 2005-09-12 23:12:09.000000000 -0400 +++ linux-2.6.14-rc1-new/drivers/scsi/libata-core.c 2005-09-14 22:13:30.000000000 -0400 @@ -74,6 +74,7 @@ static void __ata_qc_complete(struct ata static unsigned int ata_unique_id = 1; static struct workqueue_struct *ata_wq; +static struct workqueue_struct *ata_hotplug_wq; int atapi_enabled = 0; module_param(atapi_enabled, int, 0444); @@ -1147,6 +1148,11 @@ static void ata_dev_identify(struct ata_ return; } + /* Necessary if we had an LBA48 drive in, we pulled it out, and put a + * non-LBA48 drive on the same ap. + */ + dev->flags &= ~ATA_DFLAG_LBA48; + if (ap->flags & (ATA_FLAG_SRST | ATA_FLAG_SATA_RESET)) using_edd = 0; else @@ -3692,6 +3698,102 @@ idle_irq: return 0; /* irq not handled */ } +void ata_check_kill_qc(struct ata_port *ap) +{ + struct ata_queued_cmd *qc = ata_qc_from_tag(ap, ap->active_tag); + + if (unlikely(qc)) { + /* This is SO bad. But we can't just run + * ata_qc_complete without doing this, because + * ata_scsi_qc_complete wants to talk to the device, + * and we can't let it do that since it doesn't exist + * anymore. + */ + ata_scsi_prepare_qc_abort(qc); + ata_qc_complete(qc, ATA_ERR); + } +} + +static void ata_hotplug_task(void *_data) +{ + struct ata_port *ap = (struct ata_port *)_data; + enum hotplug_states hotplug_todo; + + spin_lock(&ap->host_set->lock); + hotplug_todo = ap->plug; // Make sure we don't modify while reading! + spin_unlock(&ap->host_set->lock); + + if (hotplug_todo == ATA_HOTPLUG_PLUG) { + DPRINTK("Got a plug request on port %d\n", ap->id); + + /* This might be necessary if we unplug and plug in a drive + * within ATA_TMOUT_HOTSWAP / HZ seconds... due to the debounce + * timer, one event is generated. Since the last event was a + * plug, the unplug routine never gets called, so we need to + * clean up the mess first. If there was never a drive here in + * the first place, this will just do nothing. Otherwise, it + * basically prevents 'plug' from being called twice with no + * unplug in between. + */ + ata_scsi_handle_unplug(ap); + + // The following flag is necessary on some Seagate drives. + ap->flags |= ATA_FLAG_SATA_RESET; + ap->udma_mask = ap->orig_udma_mask; + + if (!ata_bus_probe(ap)) //Drive found! Tell the SCSI layer! + ata_scsi_handle_plug(ap); + } else if (hotplug_todo == ATA_HOTPLUG_UNPLUG) { + DPRINTK("Got an unplug request on port %d\n", ap->id); + + ata_scsi_handle_unplug(ap); + } else /* FIXME: Some kind of error condition? */ + BUG(); +} + +static void ata_hotplug_timer_func(unsigned long _data) +{ + struct ata_port *ap = (struct ata_port *)_data; + + queue_work(ata_hotplug_wq, &ap->hotplug_task); +} + +void ata_hotplug_plug(struct ata_port *ap) +{ + del_timer(&ap->hotplug_timer); + + if (ap->ops->hotplug_plug_janitor) + ap->ops->hotplug_plug_janitor(ap); + + /* This line should be protected by a host_set->lock. If we follow the + * call chain up from this, it should be called from ata_hotplug_unplug + * or ata_hotplug_plug, which should be called from an interrupt + * handler. Make sure the call to either of those functions is + * protected. + */ + ap->plug = ATA_HOTPLUG_PLUG; + + ap->hotplug_timer.expires = jiffies + ATA_TMOUT_HOTSWAP; + add_timer(&ap->hotplug_timer); +} + +void ata_hotplug_unplug(struct ata_port *ap) +{ + ata_port_disable(ap); //Gone gone gone! + + del_timer(&ap->hotplug_timer); + + if (ap->ops->hotplug_unplug_janitor) + ap->ops->hotplug_unplug_janitor(ap); + + /* See comment near similar line in ata_hotplug_plug function. + */ + ap->plug = ATA_HOTPLUG_UNPLUG; + + ap->hotplug_timer.expires = jiffies + ATA_TMOUT_HOTSWAP; + add_timer(&ap->hotplug_timer); +} + /** * ata_interrupt - Default ATA host interrupt handler * @irq: irq line (unused) @@ -3925,7 +4027,12 @@ static void ata_host_init(struct ata_por ap->cbl = ATA_CBL_NONE; ap->active_tag = ATA_TAG_POISON; ap->last_ctl = 0xFF; + ap->orig_udma_mask = ent->udma_mask; + ap->hotplug_timer.function = ata_hotplug_timer_func; + ap->hotplug_timer.data = (unsigned int)ap; + init_timer(&ap->hotplug_timer); + INIT_WORK(&ap->hotplug_task, ata_hotplug_task, ap); INIT_WORK(&ap->packet_task, atapi_packet_task, ap); INIT_WORK(&ap->pio_task, ata_pio_task, ap); @@ -4541,6 +4648,11 @@ static int __init ata_init(void) ata_wq = create_workqueue("ata"); if (!ata_wq) return -ENOMEM; + ata_hotplug_wq = create_workqueue("ata_hotplug"); + if (!ata_hotplug_wq) { + destroy_workqueue(ata_wq); + return -ENOMEM; + } printk(KERN_DEBUG "libata version " DRV_VERSION " loaded.\n"); return 0; @@ -4549,6 +4661,7 @@ static int __init ata_init(void) static void __exit ata_exit(void) { destroy_workqueue(ata_wq); + destroy_workqueue(ata_hotplug_wq); } module_init(ata_init); @@ -4604,6 +4717,8 @@ EXPORT_SYMBOL_GPL(ata_dev_classify); EXPORT_SYMBOL_GPL(ata_dev_id_string); EXPORT_SYMBOL_GPL(ata_dev_config); EXPORT_SYMBOL_GPL(ata_scsi_simulate); +EXPORT_SYMBOL_GPL(ata_hotplug_plug); +EXPORT_SYMBOL_GPL(ata_hotplug_unplug); #ifdef CONFIG_PCI EXPORT_SYMBOL_GPL(pci_test_config_bits); diff -rpuN linux-2.6.14-rc1/drivers/scsi/libata.h linux-2.6.14-rc1-new/drivers/scsi/libata.h --- linux-2.6.14-rc1/drivers/scsi/libata.h 2005-09-12 23:12:09.000000000 -0400 +++ linux-2.6.14-rc1-new/drivers/scsi/libata.h 2005-09-14 19:57:53.000000000 -0400 @@ -44,6 +44,7 @@ extern struct ata_queued_cmd *ata_qc_new extern void ata_qc_free(struct ata_queued_cmd *qc); extern int ata_qc_issue(struct ata_queued_cmd *qc); extern int ata_check_atapi_dma(struct ata_queued_cmd *qc); +extern void ata_check_kill_qc(struct ata_port *ap); extern void ata_dev_select(struct ata_port *ap, unsigned int device, unsigned int wait, unsigned int can_sleep); extern void ata_tf_to_host_nolock(struct ata_port *ap, struct ata_taskfile *tf); @@ -79,6 +80,9 @@ extern void ata_scsi_badcmd(struct scsi_ extern void ata_scsi_rbuf_fill(struct ata_scsi_args *args, unsigned int (*actor) (struct ata_scsi_args *args, u8 *rbuf, unsigned int buflen)); +extern void ata_scsi_prepare_qc_abort(struct ata_queued_cmd *qc); +extern void ata_scsi_handle_plug(struct ata_port *ap); +extern void ata_scsi_handle_unplug(struct ata_port *ap); static inline void ata_bad_scsiop(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *)) { diff -rpuN linux-2.6.14-rc1/drivers/scsi/libata-scsi.c linux-2.6.14-rc1-new/drivers/scsi/libata-scsi.c --- linux-2.6.14-rc1/drivers/scsi/libata-scsi.c 2005-09-12 23:12:09.000000000 -0400 +++ linux-2.6.14-rc1-new/drivers/scsi/libata-scsi.c 2005-09-14 19:57:53.000000000 -0400 @@ -716,6 +716,53 @@ static int ata_scsi_qc_complete(struct a return 0; } +static int ata_scsi_qc_abort(struct ata_queued_cmd *qc, u8 drv_stat) +{ + struct scsi_cmnd *cmd = qc->scsicmd; + + cmd->result = SAM_STAT_TASK_ABORTED; //FIXME: Is this what we want? + + qc->scsidone(cmd); + + return 0; +} + +void ata_scsi_prepare_qc_abort(struct ata_queued_cmd *qc) +{ + /* For some reason or another, we can't allow a normal scsi_qc_complete. + * Note that at this point, we must be SURE the command is going to time + * out, or else we might be changing this as it's being called. Never a + * good thing. + */ + if (qc->complete_fn == ata_scsi_qc_complete); + qc->complete_fn = ata_scsi_qc_abort; +} + +void ata_scsi_handle_plug(struct ata_port *ap) +{ + //Currently SATA only + scsi_add_device(ap->host, 0, 0, 0); +} + +void ata_scsi_handle_unplug(struct ata_port *ap) +{ + //SATA only, no PATA + struct scsi_device *scd = scsi_device_lookup(ap->host, 0, 0, 0); + + if (scd) // Set to cancel state to block further I/O + scsi_device_set_state(scd, SDEV_CANCEL); + // We might have a pending qc on I/O to a removed device. + ata_check_kill_qc(ap); + /* scd might not exist; someone did 'echo "scsi remove-single-device + * ... " > /proc/scsi/scsi' or somebody was turning the key in the + * hotswap bay between on and off really really fast. + */ + if (scd) { + scsi_remove_device(scd); + scsi_device_put(scd); + } +} + /** * ata_scsi_translate - Translate then issue SCSI command to ATA device * @ap: ATA port to which the command is addressed diff -rpuN linux-2.6.14-rc1/include/linux/libata.h linux-2.6.14-rc1-new/include/linux/libata.h --- linux-2.6.14-rc1/include/linux/libata.h 2005-09-12 23:12:09.000000000 -0400 +++ linux-2.6.14-rc1-new/include/linux/libata.h 2005-09-14 22:12:20.000000000 -0400 @@ -32,6 +32,7 @@ #include <asm/io.h> #include <linux/ata.h> #include <linux/workqueue.h> +#include <linux/timer.h> /* * compile-time options @@ -130,6 +131,7 @@ enum { ATA_TMOUT_BOOT_QUICK = 7 * HZ, /* hueristic */ ATA_TMOUT_CDB = 30 * HZ, ATA_TMOUT_CDB_QUICK = 5 * HZ, + ATA_TMOUT_HOTSWAP = HZ / 2, /* FIXME: Guess value? */ /* ATA bus states */ BUS_UNKNOWN = 0, @@ -167,6 +169,11 @@ enum pio_task_states { PIO_ST_ERR, }; +enum hotplug_states { + ATA_HOTPLUG_PLUG, + ATA_HOTPLUG_UNPLUG, +}; + /* forward declarations */ struct scsi_device; struct ata_port_operations; @@ -316,6 +323,8 @@ struct ata_port { struct ata_host_stats stats; struct ata_host_set *host_set; + struct timer_list hotplug_timer; + struct work_struct hotplug_task; struct work_struct packet_task; struct work_struct pio_task; @@ -323,6 +332,9 @@ struct ata_port { unsigned long pio_task_timeout; void *private_data; + + unsigned int orig_udma_mask; + enum hotplug_states plug; }; struct ata_port_operations { @@ -369,6 +381,8 @@ struct ata_port_operations { void (*bmdma_stop) (struct ata_queued_cmd *qc); u8 (*bmdma_status) (struct ata_port *ap); + void (*hotplug_plug_janitor) (struct ata_port *ap); + void (*hotplug_unplug_janitor) (struct ata_port *ap); }; struct ata_port_info { @@ -399,6 +413,8 @@ extern int ata_scsi_queuecmd(struct scsi extern int ata_scsi_error(struct Scsi_Host *host); extern int ata_scsi_release(struct Scsi_Host *host); extern unsigned int ata_host_intr(struct ata_port *ap, struct ata_queued_cmd *qc); +extern void ata_hotplug_plug(struct ata_port *ap); +extern void ata_hotplug_unplug(struct ata_port *ap); /* * Default driver ops implementations */