Re: [PATCH 2.6.14-rc1 2/3] Add disk hotswap support to libata RESEND #3

[Lukasz Kosewski]

... And here's the actual patch...

On 9/15/05, Lukasz Kosewski <lkosewsk@xxxxxxxxx> wrote:
> Patch 2/3 for libata hotswapping.  The libata hotswap infrastructure,
> mark 3.  More comments in patch header.
> 
> Luke Kosewski
>
15.09.05    Luke Kosewski   <lkosewsk@xxxxxx>

	* A patch to add a general-purpose hotswap framework to libata.  This is
	  resend #3 wherein we have a new 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.

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;
+	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 /* 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,12 @@ 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 +4662,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 +4718,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,52 @@ 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);
+
+	// 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_device_set_state(scd, SDEV_CANCEL);
+		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
  */