[PATCH Trusty SRU] Revert "ipmi: simplify locking"

tim.gardner at canonical.com tim.gardner at canonical.com
Tue Nov 18 20:43:29 UTC 2014


From: Tim Gardner <tim.gardner at canonical.com>

BugLink: http://bugs.launchpad.net/bugs/1383921

This reverts commit f60adf42ad55405d1b17e9e5c33fdb63f1eb8861.

This commit was originally intended as a code improvement. However,
it appears to have either opened a locking race (which I cannot find), or
has abused HW in such a way as to cause it to stop. Given that this was
intended as an improvemnet, the most expedient approach appears to be to
revert it until we develop an upstream solution.

Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
---
 drivers/char/ipmi/ipmi_si_intf.c | 54 ++++++++++++++++++++++++----------------
 1 file changed, 33 insertions(+), 21 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 4afc8b4..8c0ffdd 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -175,6 +175,7 @@ struct smi_info {
 	struct si_sm_handlers  *handlers;
 	enum si_type           si_type;
 	spinlock_t             si_lock;
+	spinlock_t             msg_lock;
 	struct list_head       xmit_msgs;
 	struct list_head       hp_xmit_msgs;
 	struct ipmi_smi_msg    *curr_msg;
@@ -356,6 +357,13 @@ static enum si_sm_result start_next_msg(struct smi_info *smi_info)
 	struct timeval t;
 #endif
 
+	/*
+	 * No need to save flags, we aleady have interrupts off and we
+	 * already hold the SMI lock.
+	 */
+	if (!smi_info->run_to_completion)
+		spin_lock(&(smi_info->msg_lock));
+
 	/* Pick the high priority queue first. */
 	if (!list_empty(&(smi_info->hp_xmit_msgs))) {
 		entry = smi_info->hp_xmit_msgs.next;
@@ -393,6 +401,9 @@ static enum si_sm_result start_next_msg(struct smi_info *smi_info)
 		rv = SI_SM_CALL_WITHOUT_DELAY;
 	}
  out:
+	if (!smi_info->run_to_completion)
+		spin_unlock(&(smi_info->msg_lock));
+
 	return rv;
 }
 
@@ -879,6 +890,19 @@ static void sender(void                *send_info,
 	printk("**Enqueue: %d.%9.9d\n", t.tv_sec, t.tv_usec);
 #endif
 
+	/*
+	 * last_timeout_jiffies is updated here to avoid
+	 * smi_timeout() handler passing very large time_diff
+	 * value to smi_event_handler() that causes
+	 * the send command to abort.
+	 */
+	smi_info->last_timeout_jiffies = jiffies;
+
+	mod_timer(&smi_info->si_timer, jiffies + SI_TIMEOUT_JIFFIES);
+
+	if (smi_info->thread)
+		wake_up_process(smi_info->thread);
+
 	if (smi_info->run_to_completion) {
 		/*
 		 * If we are running to completion, then throw it in
@@ -901,26 +925,15 @@ static void sender(void                *send_info,
 		return;
 	}
 
-	spin_lock_irqsave(&smi_info->si_lock, flags);
+	spin_lock_irqsave(&smi_info->msg_lock, flags);
 	if (priority > 0)
 		list_add_tail(&msg->link, &smi_info->hp_xmit_msgs);
 	else
 		list_add_tail(&msg->link, &smi_info->xmit_msgs);
+	spin_unlock_irqrestore(&smi_info->msg_lock, flags);
 
+	spin_lock_irqsave(&smi_info->si_lock, flags);
 	if (smi_info->si_state == SI_NORMAL && smi_info->curr_msg == NULL) {
-		/*
-		 * last_timeout_jiffies is updated here to avoid
-		 * smi_timeout() handler passing very large time_diff
-		 * value to smi_event_handler() that causes
-		 * the send command to abort.
-		 */
-		smi_info->last_timeout_jiffies = jiffies;
-
-		mod_timer(&smi_info->si_timer, jiffies + SI_TIMEOUT_JIFFIES);
-
-		if (smi_info->thread)
-			wake_up_process(smi_info->thread);
-
 		start_next_msg(smi_info);
 		smi_event_handler(smi_info, 0);
 	}
@@ -1024,19 +1037,16 @@ static int ipmi_thread(void *data)
 static void poll(void *send_info)
 {
 	struct smi_info *smi_info = send_info;
-	unsigned long flags = 0;
-	int run_to_completion = smi_info->run_to_completion;
+	unsigned long flags;
 
 	/*
 	 * Make sure there is some delay in the poll loop so we can
 	 * drive time forward and timeout things.
 	 */
 	udelay(10);
-	if (!run_to_completion)
-		spin_lock_irqsave(&smi_info->si_lock, flags);
+	spin_lock_irqsave(&smi_info->si_lock, flags);
 	smi_event_handler(smi_info, 10);
-	if (!run_to_completion)
-		spin_unlock_irqrestore(&smi_info->si_lock, flags);
+	spin_unlock_irqrestore(&smi_info->si_lock, flags);
 }
 
 static void request_events(void *send_info)
@@ -1702,8 +1712,10 @@ static struct smi_info *smi_info_alloc(void)
 {
 	struct smi_info *info = kzalloc(sizeof(*info), GFP_KERNEL);
 
-	if (info)
+	if (info) {
 		spin_lock_init(&info->si_lock);
+		spin_lock_init(&info->msg_lock);
+	}
 	return info;
 }
 
-- 
2.1.3





More information about the kernel-team mailing list