[prev in list] [next in list] [prev in thread] [next in thread] 

List:       xen-cvs
Subject:    [Xen-changelog] [linux-2.6.18-xen] scsifront: respect full ring on reset processing
From:       Xen patchbot-linux-2.6.18-xen <patchbot () xen ! org>
Date:       2012-11-26 8:33:03
Message-ID: E1Tcu7j-0006Sq-Kd () xenbits ! xen ! org
[Download RAW message or body]

# HG changeset patch
# User Jan Beulich <jbeulich@suse.com>
# Date 1353918342 -3600
# Node ID 4d1471ca031e65eca5e508eab8330bef9799f5fb
# Parent  39a26cb5fb0eb16bbfca425084b345c7130b6ad1
scsifront: respect full ring on reset processing

Additionally, the error return case of the wait_event_interruptible()
at the end of scsifront_dev_reset_handler() wasn't handled, potentially
causing shadow slot corruption.

Doing this also revealed that io_lock was pointless - it was only used
inside the worker thread, i.e. protected nothing.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---


diff -r 39a26cb5fb0e -r 4d1471ca031e drivers/xen/scsifront/common.h
--- a/drivers/xen/scsifront/common.h	Fri Nov 23 11:53:50 2012 +0100
+++ b/drivers/xen/scsifront/common.h	Mon Nov 26 09:25:42 2012 +0100
@@ -99,7 +99,6 @@ struct vscsifrnt_info {
 
 	struct Scsi_Host *host;
 
-	spinlock_t io_lock;
 	spinlock_t shadow_lock;
 	unsigned int evtchn;
 	unsigned int irq;
@@ -113,8 +112,9 @@ struct vscsifrnt_info {
 
 	struct task_struct *kthread;
 	wait_queue_head_t wq;
-	unsigned int waiting_resp;
-
+	wait_queue_head_t wq_sync;
+	unsigned int waiting_resp:1;
+	unsigned int waiting_sync:1;
 };
 
 #define DPRINTK(_f, _a...)				\
diff -r 39a26cb5fb0e -r 4d1471ca031e drivers/xen/scsifront/scsifront.c
--- a/drivers/xen/scsifront/scsifront.c	Fri Nov 23 11:53:50 2012 +0100
+++ b/drivers/xen/scsifront/scsifront.c	Mon Nov 26 09:25:42 2012 +0100
@@ -49,16 +49,19 @@ static int get_id_from_freelist(struct v
 	return free;
 }
 
+static void _add_id_to_freelist(struct vscsifrnt_info *info, uint32_t id)
+{
+	info->shadow[id].next_free = info->shadow_free;
+	info->shadow[id].sc = NULL;
+	info->shadow_free = id;
+}
+
 static void add_id_to_freelist(struct vscsifrnt_info *info, uint32_t id)
 {
 	unsigned long flags;
 
 	spin_lock_irqsave(&info->shadow_lock, flags);
-
-	info->shadow[id].next_free = info->shadow_free;
-	info->shadow[id].sc = NULL;
-	info->shadow_free = id;
-
+	_add_id_to_freelist(info, id);
 	spin_unlock_irqrestore(&info->shadow_lock, flags);
 }
 
@@ -169,7 +172,19 @@ static void scsifront_sync_cmd_done(stru
 	
 	spin_lock_irqsave(&info->shadow_lock, flags);
 	info->shadow[id].wait_reset = 1;
-	info->shadow[id].rslt_reset = ring_res->rslt;
+	switch (info->shadow[id].rslt_reset) {
+	case 0:
+		info->shadow[id].rslt_reset = ring_res->rslt;
+		break;
+	case -1:
+		_add_id_to_freelist(info, id);
+		break;
+	default:
+		shost_printk(KERN_ERR "scsifront: ", info->host,
+			     "bad reset state %d, possibly leaking %u\n",
+			     info->shadow[id].rslt_reset, id);
+		break;
+	}
 	spin_unlock_irqrestore(&info->shadow_lock, flags);
 
 	wake_up(&(info->shadow[id].wq_reset));
@@ -184,7 +199,7 @@ static int scsifront_cmd_done(struct vsc
 	int more_to_do = 0;
 	unsigned long flags;
 
-	spin_lock_irqsave(&info->io_lock, flags);
+	spin_lock_irqsave(info->host->host_lock, flags);
 
 	rp = info->ring.sring->rsp_prod;
 	rmb();
@@ -206,8 +221,11 @@ static int scsifront_cmd_done(struct vsc
 		info->ring.sring->rsp_event = i + 1;
 	}
 
-	spin_unlock_irqrestore(&info->io_lock, flags);
+	info->waiting_sync = 0;
 
+	spin_unlock_irqrestore(info->host->host_lock, flags);
+
+	wake_up(&info->wq_sync);
 
 	/* Yield point for this unbounded loop. */
 	cond_resched();
@@ -425,17 +443,33 @@ static int scsifront_dev_reset_handler(s
 
 	vscsiif_request_t *ring_req;
 	uint16_t rqid;
-	int err;
+	int err = 0;
 
+	for (;;) {
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,12)
-	spin_lock_irq(host->host_lock);
+		spin_lock_irq(host->host_lock);
 #endif
+		if (!RING_FULL(&info->ring))
+			break;
+		if (err) {
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,12)
+			spin_unlock_irq(host->host_lock);
+#endif
+			return FAILED;
+		}
+		info->waiting_sync = 1;
+		spin_unlock_irq(host->host_lock);
+		err = wait_event_interruptible(info->wq_sync,
+					       !info->waiting_sync);
+		spin_lock_irq(host->host_lock);
+	}
 
 	ring_req      = scsifront_pre_request(info);
 	ring_req->act = VSCSIIF_ACT_SCSI_RESET;
 
 	rqid          = ring_req->rqid;
 	info->shadow[rqid].act = VSCSIIF_ACT_SCSI_RESET;
+	info->shadow[rqid].rslt_reset = 0;
 
 	ring_req->channel = sc->device->channel;
 	ring_req->id      = sc->device->id;
@@ -454,13 +488,19 @@ static int scsifront_dev_reset_handler(s
 	scsifront_do_request(info);	
 
 	spin_unlock_irq(host->host_lock);
-	wait_event_interruptible(info->shadow[rqid].wq_reset,
-			 info->shadow[rqid].wait_reset);
+	err = wait_event_interruptible(info->shadow[rqid].wq_reset,
+				       info->shadow[rqid].wait_reset);
 	spin_lock_irq(host->host_lock);
 
-	err = info->shadow[rqid].rslt_reset;
-
-	add_id_to_freelist(info, rqid);
+	if (!err) {
+		err = info->shadow[rqid].rslt_reset;
+		add_id_to_freelist(info, rqid);
+	} else {
+		spin_lock(&info->shadow_lock);
+		info->shadow[rqid].rslt_reset = -1;
+		spin_unlock(&info->shadow_lock);
+		err = FAILED;
+	}
 
 #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,12)
 	spin_unlock_irq(host->host_lock);
diff -r 39a26cb5fb0e -r 4d1471ca031e drivers/xen/scsifront/xenbus.c
--- a/drivers/xen/scsifront/xenbus.c	Fri Nov 23 11:53:50 2012 +0100
+++ b/drivers/xen/scsifront/xenbus.c	Mon Nov 26 09:25:42 2012 +0100
@@ -205,7 +205,7 @@ static int scsifront_probe(struct xenbus
 	}
 
 	init_waitqueue_head(&info->wq);
-	spin_lock_init(&info->io_lock);
+	init_waitqueue_head(&info->wq_sync);
 	spin_lock_init(&info->shadow_lock);
 
 	snprintf(name, DEFAULT_TASK_COMM_LEN, "vscsiif.%d", info->host->host_no);

_______________________________________________
Xen-changelog mailing list
Xen-changelog@lists.xen.org
http://lists.xensource.com/xen-changelog
[prev in list] [next in list] [prev in thread] [next in thread]