All of lore.kernel.org
 help / color / mirror / Atom feed
From: Corey Minyard <minyard@acm.org>
To: Linux Kernel <linux-kernel@vger.kernel.org>
Cc: Andrew Morton <akpm@osdl.org>,
	OpenIPMI Developers <openipmi-developer@lists.sourceforge.net>
Subject: [PATCH 3/8] IPMI: Run to completion fixes
Date: Wed, 13 Feb 2008 10:23:42 -0600	[thread overview]
Message-ID: <20080213162342.GC9830@minyard.local> (raw)

From: Corey Minyard <cminyard@mvista.com>

The "run_to_completion" mode was somewhat broken.  Locks need to be
avoided in run_to_completion mode, and it shouldn't be used by normal
users, just internally for panic situations.

This patch removes locks in run_to_completion mode and removes the
user call for setting the mode.  The only user was the poweroff
code, but it was easily converted to use the polling interface.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---

Index: linux-2.6.21/drivers/char/ipmi/ipmi_msghandler.c
===================================================================
--- linux-2.6.21.orig/drivers/char/ipmi/ipmi_msghandler.c
+++ linux-2.6.21/drivers/char/ipmi/ipmi_msghandler.c
@@ -1197,13 +1197,6 @@ int ipmi_unregister_for_cmd(ipmi_user_t 
 	return rv;
 }
 
-void ipmi_user_set_run_to_completion(ipmi_user_t user, int val)
-{
-	ipmi_smi_t intf = user->intf;
-	if (intf->handlers)
-		intf->handlers->set_run_to_completion(intf->send_info, val);
-}
-
 static unsigned char
 ipmb_checksum(unsigned char *data, int size)
 {
@@ -4201,5 +4194,4 @@ EXPORT_SYMBOL(ipmi_get_my_address);
 EXPORT_SYMBOL(ipmi_set_my_LUN);
 EXPORT_SYMBOL(ipmi_get_my_LUN);
 EXPORT_SYMBOL(ipmi_smi_add_proc_entry);
-EXPORT_SYMBOL(ipmi_user_set_run_to_completion);
 EXPORT_SYMBOL(ipmi_free_recv_msg);
Index: linux-2.6.21/drivers/char/ipmi/ipmi_poweroff.c
===================================================================
--- linux-2.6.21.orig/drivers/char/ipmi/ipmi_poweroff.c
+++ linux-2.6.21/drivers/char/ipmi/ipmi_poweroff.c
@@ -99,11 +99,14 @@ static unsigned char ipmi_version;
    allocate them, since we may be in a panic situation.  The whole
    thing is single-threaded, anyway, so multiple messages are not
    required. */
+static atomic_t dummy_count = ATOMIC_INIT(0);
 static void dummy_smi_free(struct ipmi_smi_msg *msg)
 {
+	atomic_dec(&dummy_count);
 }
 static void dummy_recv_free(struct ipmi_recv_msg *msg)
 {
+	atomic_dec(&dummy_count);
 }
 static struct ipmi_smi_msg halt_smi_msg =
 {
@@ -152,17 +155,28 @@ static int ipmi_request_wait_for_respons
 	return halt_recv_msg.msg.data[0];
 }
 
-/* We are in run-to-completion mode, no completion is desired. */
+/* Wait for message to complete, spinning. */
 static int ipmi_request_in_rc_mode(ipmi_user_t            user,
 				   struct ipmi_addr       *addr,
 				   struct kernel_ipmi_msg *send_msg)
 {
 	int rv;
 
+	atomic_set(&dummy_count, 2);
 	rv = ipmi_request_supply_msgs(user, addr, 0, send_msg, NULL,
 				      &halt_smi_msg, &halt_recv_msg, 0);
-	if (rv)
+	if (rv) {
+		atomic_set(&dummy_count, 0);
 		return rv;
+	}
+
+	/*
+	 * Spin until our message is done.
+	 */
+	while (atomic_read(&dummy_count) > 0) {
+		ipmi_poll_interface(user);
+		barrier();
+	}
 
 	return halt_recv_msg.msg.data[0];
 }
@@ -531,9 +545,7 @@ static void ipmi_poweroff_function (void
 		return;
 
 	/* Use run-to-completion mode, since interrupts may be off. */
-	ipmi_user_set_run_to_completion(ipmi_user, 1);
 	specific_poweroff_func(ipmi_user);
-	ipmi_user_set_run_to_completion(ipmi_user, 0);
 }
 
 /* Wait for an IPMI interface to be installed, the first one installed
Index: linux-2.6.21/drivers/char/ipmi/ipmi_si_intf.c
===================================================================
--- linux-2.6.21.orig/drivers/char/ipmi/ipmi_si_intf.c
+++ linux-2.6.21/drivers/char/ipmi/ipmi_si_intf.c
@@ -809,56 +809,54 @@ static void sender(void                *
 		return;
 	}
 
-	spin_lock_irqsave(&(smi_info->msg_lock), flags);
 #ifdef DEBUG_TIMING
 	do_gettimeofday(&t);
 	printk("**Enqueue: %d.%9.9d\n", t.tv_sec, t.tv_usec);
 #endif
 
 	if (smi_info->run_to_completion) {
-		/* If we are running to completion, then throw it in
-		   the list and run transactions until everything is
-		   clear.  Priority doesn't matter here. */
+		/*
+		 * If we are running to completion, then throw it in
+		 * the list and run transactions until everything is
+		 * clear.  Priority doesn't matter here.
+		 */
+
+		/*
+		 * Run to completion means we are single-threaded, no
+		 * need for locks.
+		 */
 		list_add_tail(&(msg->link), &(smi_info->xmit_msgs));
 
-		/* We have to release the msg lock and claim the smi
-		   lock in this case, because of race conditions. */
-		spin_unlock_irqrestore(&(smi_info->msg_lock), flags);
-
-		spin_lock_irqsave(&(smi_info->si_lock), flags);
 		result = smi_event_handler(smi_info, 0);
 		while (result != SI_SM_IDLE) {
 			udelay(SI_SHORT_TIMEOUT_USEC);
 			result = smi_event_handler(smi_info,
 						   SI_SHORT_TIMEOUT_USEC);
 		}
-		spin_unlock_irqrestore(&(smi_info->si_lock), flags);
 		return;
+	}
+
+	spin_lock_irqsave(&smi_info->msg_lock, flags);
+	if (priority > 0) {
+		list_add_tail(&msg->link, &smi_info->hp_xmit_msgs);
 	} else {
-		if (priority > 0) {
-			list_add_tail(&(msg->link), &(smi_info->hp_xmit_msgs));
-		} else {
-			list_add_tail(&(msg->link), &(smi_info->xmit_msgs));
-		}
+		list_add_tail(&msg->link, &smi_info->xmit_msgs);
 	}
-	spin_unlock_irqrestore(&(smi_info->msg_lock), flags);
+	spin_unlock_irqrestore(&smi_info->msg_lock, flags);
 
-	spin_lock_irqsave(&(smi_info->si_lock), flags);
+	spin_lock_irqsave(&smi_info->si_lock, flags);
 	if ((smi_info->si_state == SI_NORMAL)
 	    && (smi_info->curr_msg == NULL))
 	{
 		start_next_msg(smi_info);
 	}
-	spin_unlock_irqrestore(&(smi_info->si_lock), flags);
+	spin_unlock_irqrestore(&smi_info->si_lock, flags);
 }
 
 static void set_run_to_completion(void *send_info, int i_run_to_completion)
 {
 	struct smi_info   *smi_info = send_info;
 	enum si_sm_result result;
-	unsigned long     flags;
-
-	spin_lock_irqsave(&(smi_info->si_lock), flags);
 
 	smi_info->run_to_completion = i_run_to_completion;
 	if (i_run_to_completion) {
@@ -869,8 +867,6 @@ static void set_run_to_completion(void *
 						   SI_SHORT_TIMEOUT_USEC);
 		}
 	}
-
-	spin_unlock_irqrestore(&(smi_info->si_lock), flags);
 }
 
 static int ipmi_thread(void *data)
Index: linux-2.6.21/include/linux/ipmi.h
===================================================================
--- linux-2.6.21.orig/include/linux/ipmi.h
+++ linux-2.6.21/include/linux/ipmi.h
@@ -368,9 +368,8 @@ int ipmi_request_supply_msgs(ipmi_user_t
  * Poll the IPMI interface for the user.  This causes the IPMI code to
  * do an immediate check for information from the driver and handle
  * anything that is immediately pending.  This will not block in any
- * way.  This is useful if you need to implement polling from the user
- * for things like modifying the watchdog timeout when a panic occurs
- * or disabling the watchdog timer on a reboot.
+ * way.  This is useful if you need to spin waiting for something to
+ * happen in the IPMI driver.
  */
 void ipmi_poll_interface(ipmi_user_t user);
 
@@ -422,12 +421,6 @@ int ipmi_get_maintenance_mode(ipmi_user_
 int ipmi_set_maintenance_mode(ipmi_user_t user, int mode);
 
 /*
- * Allow run-to-completion mode to be set for the interface of
- * a specific user.
- */
-void ipmi_user_set_run_to_completion(ipmi_user_t user, int val);
-
-/*
  * When the user is created, it will not receive IPMI events by
  * default.  The user must set this to TRUE to get incoming events.
  * The first user that sets this to TRUE will receive all events that

             reply	other threads:[~2008-02-13 16:25 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-13 16:23 Corey Minyard [this message]
2008-02-13 23:20 ` [PATCH 3/8] IPMI: Run to completion fixes Andrew Morton
2008-02-14  0:20   ` Corey Minyard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080213162342.GC9830@minyard.local \
    --to=minyard@acm.org \
    --cc=akpm@osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=openipmi-developer@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.