Jan Beulich
2012-Nov-23 10:44 UTC
[PATCH] linux-2.6.18/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>
--- a/drivers/xen/scsifront/common.h
+++ b/drivers/xen/scsifront/common.h
@@ -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...) \
--- a/drivers/xen/scsifront/scsifront.c
+++ b/drivers/xen/scsifront/scsifront.c
@@ -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)
+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;
+}
+static void add_id_to_freelist(struct vscsifrnt_info *info, uint32_t id)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&info->shadow_lock, flags);
+ _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);
--- a/drivers/xen/scsifront/xenbus.c
+++ b/drivers/xen/scsifront/xenbus.c
@@ -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-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel