[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [linux-2.6.18-xen] scsifront: respect full ring on reset processing


  • To: xen-changelog@xxxxxxxxxxxxxxxxxxx
  • From: Xen patchbot-linux-2.6.18-xen <patchbot@xxxxxxx>
  • Date: Mon, 26 Nov 2012 08:33:03 +0000
  • Delivery-date: Mon, 26 Nov 2012 08:33:16 +0000
  • List-id: "Change log for Mercurial \(receive only\)" <xen-changelog.lists.xen.org>

# HG changeset patch
# User Jan Beulich <jbeulich@xxxxxxxx>
# 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@xxxxxxxx>
---


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@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.