[PATCH linux-2.6.18] support suspend/resume in pvscsi drivers

[PATCH linux-2.6.18] support suspend/resume in pvscsi drivers

[Juergen Gross]

Up to now the pvscsi drivers haven't supported domain suspend and
resume. When a domain with an assigned pvscsi device was suspended
and resumed again, it was not able to use the device any more: trying
to do so resulted in hanging processes.

Support suspend and resume of pvscsi devices.

Signed-off-by: Juergen Gross <jgross@suse.com>

diff -r 578e5aea3cbb drivers/xen/scsiback/xenbus.c
--- a/drivers/xen/scsiback/xenbus.c	Mon Jan 19 11:51:46 2015 +0100
+++ b/drivers/xen/scsiback/xenbus.c	Fri Jan 30 13:57:29 2015 +0100
@@ -167,33 +167,48 @@ static void scsiback_do_lun_hotplug(stru
 
 		switch (op) {
 		case VSCSIBACK_OP_ADD_OR_DEL_LUN:
-			if (device_state == XenbusStateInitialising) {
+			switch (device_state) {
+			case XenbusStateInitialising:
+			case XenbusStateConnected:
 				sdev = scsiback_get_scsi_device(&phy);
-				if (!sdev)
-					xenbus_printf(XBT_NIL, dev->nodename, state_str, 
-							    "%d", XenbusStateClosed);
-				else {
-					err = scsiback_add_translation_entry(be->info, sdev, &vir);
-					if (!err) {
-						if (xenbus_printf(XBT_NIL, dev->nodename, state_str, 
-								    "%d", XenbusStateInitialised)) {
-							printk(KERN_ERR "scsiback: xenbus_printf error %s\n", state_str);
-							scsiback_del_translation_entry(be->info, &vir);
-						}
-					} else {
-						scsi_device_put(sdev);
-						xenbus_printf(XBT_NIL, dev->nodename, state_str, 
-								    "%d", XenbusStateClosed);
-					}
+				if (!sdev) {
+					xenbus_printf(XBT_NIL, dev->nodename,
+						      state_str,
+						      "%d", XenbusStateClosed);
+					break;
 				}
-			}
+				if (scsiback_add_translation_entry(be->info,
+								sdev, &vir)) {
+					scsi_device_put(sdev);
+					if (device_state == XenbusStateConnected)
+						break;
+					xenbus_printf(XBT_NIL, dev->nodename,
+						      state_str,
+						      "%d", XenbusStateClosed);
+					break;
+				}
+				if (!xenbus_printf(XBT_NIL, dev->nodename,
+						  state_str, "%d",
+						  XenbusStateInitialised))
+					break;
+				printk(KERN_ERR "scsiback: xenbus_printf error %s\n",
+				       state_str);
+				scsiback_del_translation_entry(be->info, &vir);
+				break;
 
-			if (device_state == XenbusStateClosing) {
-				if (!scsiback_del_translation_entry(be->info, &vir)) {
-					if (xenbus_printf(XBT_NIL, dev->nodename, state_str, 
-							    "%d", XenbusStateClosed))
-						printk(KERN_ERR "scsiback: xenbus_printf error %s\n", state_str);
-				}
+			case XenbusStateClosing:
+				if (scsiback_del_translation_entry(be->info,
+								   &vir))
+					break;
+				if (xenbus_printf(XBT_NIL, dev->nodename,
+						  state_str, "%d",
+						  XenbusStateClosed))
+					printk(KERN_ERR "scsiback: xenbus_printf error %s\n",
+					       state_str);
+				break;
+
+			default:
+				break;
 			}
 			break;
 
diff -r 578e5aea3cbb drivers/xen/scsifront/common.h
--- a/drivers/xen/scsifront/common.h	Mon Jan 19 11:51:46 2015 +0100
+++ b/drivers/xen/scsifront/common.h	Fri Jan 30 13:57:29 2015 +0100
@@ -75,7 +75,9 @@
 
 struct vscsifrnt_shadow {
 	uint16_t next_free;
-	
+#define VSCSIIF_IN_USE		0x0fff
+#define VSCSIIF_NONE		0x0ffe
+
 	/* command between backend and frontend
 	 * VSCSIIF_ACT_SCSI_CDB or VSCSIIF_ACT_SCSI_RESET */
 	unsigned char act;
@@ -113,8 +115,12 @@ struct vscsifrnt_info {
 	struct task_struct *kthread;
 	wait_queue_head_t wq;
 	wait_queue_head_t wq_sync;
+	wait_queue_head_t wq_pause;
 	unsigned char waiting_resp;
 	unsigned char waiting_sync;
+	unsigned char waiting_pause;
+	unsigned char pause;
+	unsigned callers;
 };
 
 #define DPRINTK(_f, _a...)				\
@@ -124,6 +130,7 @@ struct vscsifrnt_info {
 int scsifront_xenbus_init(void);
 void scsifront_xenbus_unregister(void);
 int scsifront_schedule(void *data);
+void scsifront_finish_all(struct vscsifrnt_info *info);
 irqreturn_t scsifront_intr(int irq, void *dev_id, struct pt_regs *ptregs);
 
 
diff -r 578e5aea3cbb drivers/xen/scsifront/scsifront.c
--- a/drivers/xen/scsifront/scsifront.c	Mon Jan 19 11:51:46 2015 +0100
+++ b/drivers/xen/scsifront/scsifront.c	Fri Jan 30 13:57:29 2015 +0100
@@ -40,9 +40,9 @@ static int get_id_from_freelist(struct v
 	spin_lock_irqsave(&info->shadow_lock, flags);
 
 	free = info->shadow_free;
-	BUG_ON(free > VSCSIIF_MAX_REQS);
+	BUG_ON(free >= VSCSIIF_MAX_REQS);
 	info->shadow_free = info->shadow[free].next_free;
-	info->shadow[free].next_free = 0x0fff;
+	info->shadow[free].next_free = VSCSIIF_IN_USE;
 
 	info->shadow[free].wait_reset = 0;
 
@@ -188,27 +188,26 @@ static void scsifront_sync_cmd_done(stru
 	wake_up(&(info->shadow[id].wq_reset));
 }
 
+static void scsifront_do_response(struct vscsifrnt_info *info,
+				  vscsiif_response_t *ring_res)
+{
+	if (info->shadow[ring_res->rqid].act == VSCSIIF_ACT_SCSI_CDB)
+		scsifront_cdb_cmd_done(info, ring_res);
+	else
+		scsifront_sync_cmd_done(info, ring_res);
+}
 
-static int scsifront_cmd_done(struct vscsifrnt_info *info)
+static int scsifront_ring_drain(struct vscsifrnt_info *info)
 {
 	vscsiif_response_t *ring_res;
-
 	RING_IDX i, rp;
 	int more_to_do = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(info->host->host_lock, flags);
 
 	rp = info->ring.sring->rsp_prod;
 	rmb();
 	for (i = info->ring.rsp_cons; i != rp; i++) {
-		
 		ring_res = RING_GET_RESPONSE(&info->ring, i);
-
-		if (info->shadow[ring_res->rqid].act == VSCSIIF_ACT_SCSI_CDB)
-			scsifront_cdb_cmd_done(info, ring_res);
-		else
-			scsifront_sync_cmd_done(info, ring_res);
+		scsifront_do_response(info, ring_res);
 	}
 
 	info->ring.rsp_cons = i;
@@ -219,6 +218,18 @@ static int scsifront_cmd_done(struct vsc
 		info->ring.sring->rsp_event = i + 1;
 	}
 
+	return more_to_do;
+}
+
+static int scsifront_cmd_done(struct vscsifrnt_info *info)
+{
+	int more_to_do;
+	unsigned long flags;
+
+	spin_lock_irqsave(info->host->host_lock, flags);
+
+	more_to_do = scsifront_ring_drain(info);
+
 	info->waiting_sync = 0;
 
 	spin_unlock_irqrestore(info->host->host_lock, flags);
@@ -231,8 +242,23 @@ static int scsifront_cmd_done(struct vsc
 	return more_to_do;
 }
 
+void scsifront_finish_all(struct vscsifrnt_info *info)
+{
+	unsigned i;
+	struct vscsiif_response resp;
 
+	scsifront_ring_drain(info);
 
+	for (i = 0; i < VSCSIIF_MAX_REQS; i++) {
+		if (info->shadow[i].next_free != VSCSIIF_IN_USE)
+			continue;
+		resp.rqid = i;
+		resp.sense_len = 0;
+		resp.rslt = DID_RESET << 16;
+		resp.residual_len = 0;
+		scsifront_do_response(info, &resp);
+	}
+}
 
 int scsifront_schedule(void *data)
 {
@@ -359,6 +385,27 @@ big_to_sg:
 	return ref_cnt;
 }
 
+static int scsifront_enter(struct vscsifrnt_info *info)
+{
+	if (info->pause)
+		return 1;
+	info->callers++;
+	return 0;
+}
+
+static void scsifront_return(struct vscsifrnt_info *info)
+{
+	info->callers--;
+	if (info->callers)
+		return;
+
+	if (!info->waiting_pause)
+		return;
+
+	info->waiting_pause = 0;
+	wake_up(&info->wq_pause);
+}
+
 static int scsifront_queuecommand(struct scsi_cmnd *sc,
 				  void (*done)(struct scsi_cmnd *))
 {
@@ -368,6 +415,9 @@ static int scsifront_queuecommand(struct
 	int ref_cnt;
 	uint16_t rqid;
 
+	if (scsifront_enter(info))
+		return SCSI_MLQUEUE_HOST_BUSY;
+
 /* debug printk to identify more missing scsi commands
 	shost_printk(KERN_INFO "scsicmd: ", sc->device->host,
 		     "len=%u %#x,%#x,%#x,%#x,%#x,%#x,%#x,%#x,%#x,%#x\n",
@@ -420,13 +470,16 @@ static int scsifront_queuecommand(struct
 
 	scsifront_do_request(info);
 
+	scsifront_return(info);
 	return 0;
 
 out_host_busy:
+	scsifront_return(info);
 	return SCSI_MLQUEUE_HOST_BUSY;
 
 out_fail_command:
 	done(sc);
+	scsifront_return(info);
 	return 0;
 }
 
@@ -451,7 +504,7 @@ static int scsifront_dev_reset_handler(s
 	spin_lock_irq(host->host_lock);
 #endif
 	while (RING_FULL(&info->ring)) {
-		if (err) {
+		if (err || info->pause) {
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,12)
 			spin_unlock_irq(host->host_lock);
 #endif
@@ -464,6 +517,11 @@ static int scsifront_dev_reset_handler(s
 		spin_lock_irq(host->host_lock);
 	}
 
+	if (scsifront_enter(info)) {
+		spin_unlock_irq(host->host_lock);
+		return FAILED;
+	}
+
 	ring_req      = scsifront_pre_request(info);
 	ring_req->act = VSCSIIF_ACT_SCSI_RESET;
 
@@ -502,6 +560,8 @@ static int scsifront_dev_reset_handler(s
 		err = FAILED;
 	}
 
+	scsifront_return(info);
+
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,12)
 	spin_unlock_irq(host->host_lock);
 #endif
diff -r 578e5aea3cbb drivers/xen/scsifront/xenbus.c
--- a/drivers/xen/scsifront/xenbus.c	Mon Jan 19 11:51:46 2015 +0100
+++ b/drivers/xen/scsifront/xenbus.c	Fri Jan 30 13:57:29 2015 +0100
@@ -43,6 +43,20 @@
 
 extern struct scsi_host_template scsifront_sht;
 
+static void scsifront_free_ring(struct vscsifrnt_info *info)
+{
+	if (info->ring_ref != GRANT_INVALID_REF) {
+		gnttab_end_foreign_access(info->ring_ref,
+					(unsigned long)info->ring.sring);
+		info->ring_ref = GRANT_INVALID_REF;
+		info->ring.sring = NULL;
+	}
+
+	if (info->irq)
+		unbind_from_irqhandler(info->irq, info);
+	info->irq = 0;
+}
+
 static void scsifront_free(struct vscsifrnt_info *info)
 {
 	struct Scsi_Host *host = info->host;
@@ -55,16 +69,7 @@ static void scsifront_free(struct vscsif
 		scsi_remove_host(info->host);
 	}
 
-	if (info->ring_ref != GRANT_INVALID_REF) {
-		gnttab_end_foreign_access(info->ring_ref,
-					(unsigned long)info->ring.sring);
-		info->ring_ref = GRANT_INVALID_REF;
-		info->ring.sring = NULL;
-	}
-
-	if (info->irq)
-		unbind_from_irqhandler(info->irq, info);
-	info->irq = 0;
+	scsifront_free_ring(info);
 
 	scsi_host_put(info->host);
 }
@@ -196,7 +201,7 @@ static int scsifront_probe(struct xenbus
 		init_waitqueue_head(&(info->shadow[i].wq_reset));
 		info->shadow[i].wait_reset = 0;
 	}
-	info->shadow[VSCSIIF_MAX_REQS - 1].next_free = 0x0fff;
+	info->shadow[VSCSIIF_MAX_REQS - 1].next_free = VSCSIIF_NONE;
 
 	err = scsifront_init_ring(info);
 	if (err) {
@@ -208,6 +213,11 @@ static int scsifront_probe(struct xenbus
 	init_waitqueue_head(&info->wq_sync);
 	spin_lock_init(&info->shadow_lock);
 
+	info->pause = 0;
+	info->callers = 0;
+	info->waiting_pause = 0;
+	init_waitqueue_head(&info->wq_pause);
+
 	snprintf(name, DEFAULT_TASK_COMM_LEN, "vscsiif.%d", info->host->host_no);
 
 	info->kthread = kthread_run(scsifront_schedule, info, name);
@@ -240,6 +250,66 @@ free_sring:
 	return err;
 }
 
+static int scsifront_resume(struct xenbus_device *dev)
+{
+	struct vscsifrnt_info *info = dev->dev.driver_data;
+	struct Scsi_Host *host = info->host;
+	int err;
+
+	spin_lock_irq(host->host_lock);
+
+	/* finish all still pending commands */
+	scsifront_finish_all(info);
+
+	spin_unlock_irq(host->host_lock);
+
+	/* reconnect to dom0 */
+	scsifront_free_ring(info);
+	err = scsifront_init_ring(info);
+	if (err) {
+		dev_err(&dev->dev, "fail to resume %d\n", err);
+		scsifront_free(info);
+		return err;
+	}
+
+	xenbus_switch_state(dev, XenbusStateInitialised);
+
+	return 0;
+}
+
+static int scsifront_suspend(struct xenbus_device *dev)
+{
+	struct vscsifrnt_info *info = dev->dev.driver_data;
+	struct Scsi_Host *host = info->host;
+	int err = 0;
+
+	/* no new commands for the backend */
+	spin_lock_irq(host->host_lock);
+	info->pause = 1;
+	while (info->callers && !err) {
+		info->waiting_pause = 1;
+		info->waiting_sync = 0;
+		spin_unlock_irq(host->host_lock);
+		wake_up(&info->wq_sync);
+		err = wait_event_interruptible(info->wq_pause,
+					       !info->waiting_pause);
+		spin_lock_irq(host->host_lock);
+	}
+	spin_unlock_irq(host->host_lock);
+	return err;
+}
+
+static int scsifront_suspend_cancel(struct xenbus_device *dev)
+{
+	struct vscsifrnt_info *info = dev->dev.driver_data;
+	struct Scsi_Host *host = info->host;
+
+	spin_lock_irq(host->host_lock);
+	info->pause = 0;
+	spin_unlock_irq(host->host_lock);
+	return 0;
+}
+
 static int scsifront_remove(struct xenbus_device *dev)
 {
 	struct vscsifrnt_info *info = dev->dev.driver_data;
@@ -278,6 +348,7 @@ static int scsifront_disconnect(struct v
 
 #define VSCSIFRONT_OP_ADD_LUN	1
 #define VSCSIFRONT_OP_DEL_LUN	2
+#define VSCSIFRONT_OP_READD_LUN	3
 
 static void scsifront_do_lun_hotplug(struct vscsifrnt_info *info, int op)
 {
@@ -339,6 +410,11 @@ static void scsifront_do_lun_hotplug(str
 				}
 			}
 			break;
+		case VSCSIFRONT_OP_READD_LUN:
+			if (device_state == XenbusStateConnected)
+				xenbus_printf(XBT_NIL, dev->nodename, state_str,
+					      "%d", XenbusStateConnected);
+			break;
 		default:
 			break;
 		}
@@ -366,6 +442,13 @@ static void scsifront_backend_changed(st
 		break;
 
 	case XenbusStateConnected:
+		if (info->pause) {
+			scsifront_do_lun_hotplug(info, VSCSIFRONT_OP_READD_LUN);
+			xenbus_switch_state(dev, XenbusStateConnected);
+			info->pause = 0;
+			return;
+		}
+
 		if (xenbus_read_driver_state(dev->nodename) ==
 			XenbusStateInitialised) {
 			scsifront_do_lun_hotplug(info, VSCSIFRONT_OP_ADD_LUN);
@@ -407,7 +490,9 @@ MODULE_ALIAS("xen:vscsi");
 static DEFINE_XENBUS_DRIVER(scsifront, ,
 	.probe			= scsifront_probe,
 	.remove			= scsifront_remove,
-/* 	.resume			= scsifront_resume, */
+	.resume			= scsifront_resume,
+	.suspend		= scsifront_suspend,
+	.suspend_cancel		= scsifront_suspend_cancel,
 	.otherend_changed	= scsifront_backend_changed,
 );

Re: [PATCH linux-2.6.18] support suspend/resume in pvscsi drivers

[Jan Beulich]

>>> On 30.01.15 at 14:52, <"jgross@suse.com".non-mime.internet> wrote:
> @@ -231,8 +242,23 @@ static int scsifront_cmd_done(struct vsc
>  	return more_to_do;
>  }
>  
> +void scsifront_finish_all(struct vscsifrnt_info *info)
> +{
> +	unsigned i;
> +	struct vscsiif_response resp;
>  
> +	scsifront_ring_drain(info);

Shouldn't you at least issue some kind of warning when this returns
non-zero?

Jan

Re: [PATCH linux-2.6.18] support suspend/resume in pvscsi drivers

[Juergen Gross]

On 01/30/2015 03:46 PM, Jan Beulich wrote:
>>>> On 30.01.15 at 14:52, <"jgross@suse.com".non-mime.internet> wrote:
>> @@ -231,8 +242,23 @@ static int scsifront_cmd_done(struct vsc
>>   	return more_to_do;
>>   }
>>
>> +void scsifront_finish_all(struct vscsifrnt_info *info)
>> +{
>> +	unsigned i;
>> +	struct vscsiif_response resp;
>>
>> +	scsifront_ring_drain(info);
>
> Shouldn't you at least issue some kind of warning when this returns
> non-zero?

If a warning should be issued, then this should be done after the
following loop in case of at least one request terminated there.

I'm really not sure, whether a warning is required here. If you like,
I can add one.

Juergen

Re: [PATCH linux-2.6.18] support suspend/resume in pvscsi drivers

[Jan Beulich]

>>> On 30.01.15 at 15:54, <JGross@suse.com> wrote:
> On 01/30/2015 03:46 PM, Jan Beulich wrote:
>>>>> On 30.01.15 at 14:52, <"jgross@suse.com".non-mime.internet> wrote:
>>> @@ -231,8 +242,23 @@ static int scsifront_cmd_done(struct vsc
>>>   	return more_to_do;
>>>   }
>>>
>>> +void scsifront_finish_all(struct vscsifrnt_info *info)
>>> +{
>>> +	unsigned i;
>>> +	struct vscsiif_response resp;
>>>
>>> +	scsifront_ring_drain(info);
>>
>> Shouldn't you at least issue some kind of warning when this returns
>> non-zero?
> 
> If a warning should be issued, then this should be done after the
> following loop in case of at least one request terminated there.
> 
> I'm really not sure, whether a warning is required here. If you like,
> I can add one.

I'm not sure, I'm merely asking because I saw the function return
value being ignored here.

Jan

Re: [PATCH linux-2.6.18] support suspend/resume in pvscsi drivers

[Juergen Gross]

On 01/30/2015 04:05 PM, Jan Beulich wrote:
>>>> On 30.01.15 at 15:54, <JGross@suse.com> wrote:
>> On 01/30/2015 03:46 PM, Jan Beulich wrote:
>>>>>> On 30.01.15 at 14:52, <"jgross@suse.com".non-mime.internet> wrote:
>>>> @@ -231,8 +242,23 @@ static int scsifront_cmd_done(struct vsc
>>>>    	return more_to_do;
>>>>    }
>>>>
>>>> +void scsifront_finish_all(struct vscsifrnt_info *info)
>>>> +{
>>>> +	unsigned i;
>>>> +	struct vscsiif_response resp;
>>>>
>>>> +	scsifront_ring_drain(info);
>>>
>>> Shouldn't you at least issue some kind of warning when this returns
>>> non-zero?
>>
>> If a warning should be issued, then this should be done after the
>> following loop in case of at least one request terminated there.
>>
>> I'm really not sure, whether a warning is required here. If you like,
>> I can add one.
>
> I'm not sure, I'm merely asking because I saw the function return
> value being ignored here.

I think it can be 0 only. We are handling resume, so the ring which is
being drained will no longer be filled by the backend.

Juergen