Attachment 97157 Details for Bug 153008

[patch] Bugfixes from 2.6.17

i2o-bugfixes-2.6.17 (text/plain), 6.39 KB, created by Jeff Mahoney on 2006-08-25 15:02:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Mahoney

Created: 2006-08-25 15:02:09 UTC

Size: 6.39 KB

patch

obsolete

>From: Markus Lidel <Markus.Lidel@shadowconnect.com>
>Date: Sat, 10 Jun 2006 16:54:14 +0000 (-0700)
>Subject: [PATCH] I2O: Bugfixes to get I2O working again
>X-Git-Tag: v2.6.17
>X-Git-Url: http://www.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=57a62fed871eb2a95f296fe6c5c250ce21b81a79
>
>[PATCH] I2O: Bugfixes to get I2O working again
>
>From: Markus Lidel <Markus.Lidel@shadowconnect.com>
>
>- Fixed locking of struct i2o_exec_wait in Executive-OSM
>
>- Removed LCT Notify in i2o_exec_probe() which caused freeing memory and
>  accessing freed memory during first enumeration of I2O devices
>
>- Added missing locking in i2o_exec_lct_notify()
>
>- removed put_device() of I2O controller in i2o_iop_remove() which caused
>  the controller structure get freed to early
>
>- Fixed size of mempool in i2o_iop_alloc()
>
>- Fixed access to freed memory in i2o_msg_get()
>
>See http://bugzilla.kernel.org/show_bug.cgi?id=6561
>
>Signed-off-by: Markus Lidel <Markus.Lidel@shadowconnect.com>
>Cc: <stable@kernel.org>
>Signed-off-by: Andrew Morton <akpm@osdl.org>
>Signed-off-by: Linus Torvalds <torvalds@osdl.org>
>---
>
>--- a/drivers/message/i2o/exec-osm.c
>+++ b/drivers/message/i2o/exec-osm.c
>@@ -55,6 +55,7 @@ struct i2o_exec_wait {
> 	u32 m;			/* message id */
> 	struct i2o_message *msg;	/* pointer to the reply message */
> 	struct list_head list;	/* node in global wait list */
>+	spinlock_t lock;	/* lock before modifying */
> };
> 
> /* Work struct needed to handle LCT NOTIFY replies */
>@@ -87,6 +88,7 @@ static struct i2o_exec_wait *i2o_exec_wa
> 		return NULL;
> 
> 	INIT_LIST_HEAD(&wait->list);
>+	spin_lock_init(&wait->lock);
> 
> 	return wait;
> };
>@@ -125,6 +127,7 @@ int i2o_msg_post_wait_mem(struct i2o_con
> 	DECLARE_WAIT_QUEUE_HEAD(wq);
> 	struct i2o_exec_wait *wait;
> 	static u32 tcntxt = 0x80000000;
>+	long flags;
> 	int rc = 0;
> 
> 	wait = i2o_exec_wait_alloc();
>@@ -146,33 +149,28 @@ int i2o_msg_post_wait_mem(struct i2o_con
> 	wait->tcntxt = tcntxt++;
> 	msg->u.s.tcntxt = cpu_to_le32(wait->tcntxt);
> 
>+	wait->wq = &wq;
>+	/*
>+	 * we add elements to the head, because if a entry in the list will
>+	 * never be removed, we have to iterate over it every time
>+	 */
>+	list_add(&wait->list, &i2o_exec_wait_list);
>+
> 	/*
> 	 * Post the message to the controller. At some point later it will
> 	 * return. If we time out before it returns then complete will be zero.
> 	 */
> 	i2o_msg_post(c, msg);
> 
>-	if (!wait->complete) {
>-		wait->wq = &wq;
>-		/*
>-		 * we add elements add the head, because if a entry in the list
>-		 * will never be removed, we have to iterate over it every time
>-		 */
>-		list_add(&wait->list, &i2o_exec_wait_list);
>-
>-		wait_event_interruptible_timeout(wq, wait->complete,
>-						 timeout * HZ);
>+	wait_event_interruptible_timeout(wq, wait->complete, timeout * HZ);
> 
>-		wait->wq = NULL;
>-	}
>+	spin_lock_irqsave(&wait->lock, flags);
> 
>-	barrier();
>+	wait->wq = NULL;
> 
>-	if (wait->complete) {
>+	if (wait->complete)
> 		rc = le32_to_cpu(wait->msg->body[0]) >> 24;
>-		i2o_flush_reply(c, wait->m);
>-		i2o_exec_wait_free(wait);
>-	} else {
>+	else {
> 		/*
> 		 * We cannot remove it now. This is important. When it does
> 		 * terminate (which it must do if the controller has not
>@@ -186,6 +184,13 @@ int i2o_msg_post_wait_mem(struct i2o_con
> 		rc = -ETIMEDOUT;
> 	}
> 
>+	spin_unlock_irqrestore(&wait->lock, flags);
>+
>+	if (rc != -ETIMEDOUT) {
>+		i2o_flush_reply(c, wait->m);
>+		i2o_exec_wait_free(wait);
>+	}
>+
> 	return rc;
> };
> 
>@@ -213,7 +218,6 @@ static int i2o_msg_post_wait_complete(st
> {
> 	struct i2o_exec_wait *wait, *tmp;
> 	unsigned long flags;
>-	static spinlock_t lock = SPIN_LOCK_UNLOCKED;
> 	int rc = 1;
> 
> 	/*
>@@ -223,23 +227,24 @@ static int i2o_msg_post_wait_complete(st
> 	 * already expired. Not much we can do about that except log it for
> 	 * debug purposes, increase timeout, and recompile.
> 	 */
>-	spin_lock_irqsave(&lock, flags);
> 	list_for_each_entry_safe(wait, tmp, &i2o_exec_wait_list, list) {
> 		if (wait->tcntxt == context) {
>-			list_del(&wait->list);
>+			spin_lock_irqsave(&wait->lock, flags);
> 
>-			spin_unlock_irqrestore(&lock, flags);
>+			list_del(&wait->list);
> 
> 			wait->m = m;
> 			wait->msg = msg;
> 			wait->complete = 1;
> 
>-			barrier();
>-
>-			if (wait->wq) {
>-				wake_up_interruptible(wait->wq);
>+			if (wait->wq)
> 				rc = 0;
>-			} else {
>+			else
>+				rc = -1;
>+
>+			spin_unlock_irqrestore(&wait->lock, flags);
>+
>+			if (rc) {
> 				struct device *dev;
> 
> 				dev = &c->pdev->dev;
>@@ -248,15 +253,13 @@ static int i2o_msg_post_wait_complete(st
> 					 c->name);
> 				i2o_dma_free(dev, &wait->dma);
> 				i2o_exec_wait_free(wait);
>-				rc = -1;
>-			}
>+			} else
>+				wake_up_interruptible(wait->wq);
> 
> 			return rc;
> 		}
> 	}
> 
>-	spin_unlock_irqrestore(&lock, flags);
>-
> 	osm_warn("%s: Bogus reply in POST WAIT (tr-context: %08x)!\n", c->name,
> 		 context);
> 
>@@ -322,14 +325,9 @@ static DEVICE_ATTR(product_id, S_IRUGO, 
> static int i2o_exec_probe(struct device *dev)
> {
> 	struct i2o_device *i2o_dev = to_i2o_device(dev);
>-	struct i2o_controller *c = i2o_dev->iop;
> 
> 	i2o_event_register(i2o_dev, &i2o_exec_driver, 0, 0xffffffff);
> 
>-	c->exec = i2o_dev;
>-
>-	i2o_exec_lct_notify(c, c->lct->change_ind + 1);
>-
> 	device_create_file(dev, &dev_attr_vendor_id);
> 	device_create_file(dev, &dev_attr_product_id);
> 
>@@ -523,6 +521,8 @@ static int i2o_exec_lct_notify(struct i2
> 	struct device *dev;
> 	struct i2o_message *msg;
> 
>+	down(&c->lct_lock);
>+
> 	dev = &c->pdev->dev;
> 
> 	if (i2o_dma_realloc
>@@ -545,6 +545,8 @@ static int i2o_exec_lct_notify(struct i2
> 
> 	i2o_msg_post(c, msg);
> 
>+	up(&c->lct_lock);
>+
> 	return 0;
> };
> 
>--- a/drivers/message/i2o/iop.c
>+++ b/drivers/message/i2o/iop.c
>@@ -804,8 +804,6 @@ void i2o_iop_remove(struct i2o_controlle
> 
> 	/* Ask the IOP to switch to RESET state */
> 	i2o_iop_reset(c);
>-
>-	put_device(&c->device);
> }
> 
> /**
>@@ -1059,7 +1057,7 @@ struct i2o_controller *i2o_iop_alloc(voi
> 
> 	snprintf(poolname, sizeof(poolname), "i2o_%s_msg_inpool", c->name);
> 	if (i2o_pool_alloc
>-	    (&c->in_msg, poolname, I2O_INBOUND_MSG_FRAME_SIZE * 4,
>+	    (&c->in_msg, poolname, I2O_INBOUND_MSG_FRAME_SIZE * 4 + sizeof(u32),
> 	     I2O_MSG_INPOOL_MIN)) {
> 		kfree(c);
> 		return ERR_PTR(-ENOMEM);
>--- a/include/linux/i2o.h
>+++ b/include/linux/i2o.h
>@@ -1114,8 +1114,11 @@ static inline struct i2o_message *i2o_ms
> 
> 	mmsg->mfa = readl(c->in_port);
> 	if (unlikely(mmsg->mfa >= c->in_queue.len)) {
>+		u32 mfa = mmsg->mfa;
>+
> 		mempool_free(mmsg, c->in_msg.mempool);
>-		if(mmsg->mfa == I2O_QUEUE_EMPTY)
>+
>+		if (mfa == I2O_QUEUE_EMPTY)
> 			return ERR_PTR(-EBUSY);
> 		return ERR_PTR(-EFAULT);
> 	}

Actions: View | Diff

Attachments on bug 153008: 97157