public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Martin Wilck <martin.wilck@fujitsu-siemens.com>
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Corey Minyard <minyard@acm.org>,
	openipmi-developer@lists.sourceforge.net
Subject: [PATCH] limit CPU time spent in kipmid
Date: Thu, 19 Mar 2009 17:27:45 +0100	[thread overview]
Message-ID: <49C27281.4040207@fujitsu-siemens.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1094 bytes --]

Hello Corey, hi everyone,

here is a patch that limits the CPU time spent in kipmid. I know that it 
was previously stated that current kipmid "works as designed" (e.g. 
http://lists.us.dell.com/pipermail/linux-poweredge/2008-October/037636.html), 
yet users are irritated by the high amount of CPU time kipmid may use up 
on current servers with many sensors, even though it is "nice" CPU time. 
Moreover, kipmid busy-waiting for the KCS interface to become ready also 
prevents CPUs from sleeping.

The attached patch was developed and tested on an enterprise 
distribution kernel where it caused the CPU load of kipmid to drop to 
essentially 0 while still delivering reliable IPMI communication.

I am looking forward for comments.
Martin

-- 
Martin Wilck
PRIMERGY System Software Engineer
FSC IP ESP DEV 6

Fujitsu Siemens Computers GmbH
Heinz-Nixdorf-Ring 1
33106 Paderborn
Germany

Tel:			++49 5251 525 2796
Fax:			++49 5251 525 2820
Email:			mailto:martin.wilck@fujitsu-siemens.com
Internet:		http://www.fujitsu-siemens.com
Company Details:	http://www.fujitsu-siemens.com/imprint.html

[-- Attachment #2: ipmi_si_max_busy-2.6.29-rc8.diff --]
[-- Type: text/x-patch, Size: 3493 bytes --]

While the current kipmid implementation is optimal in the sense that kipmid obtains
data as quickly as possible without stealing CPU time from other processes
(by running on low prio and calling schedule() in each loop iteration), it
may spend a lot of CPU time polling for data e.g. on the KCS interface. The
busy loop also prevents CPUs from entering sleep states when no KCS data is
available.

This patch adds a module parameter "kipmid_max_busy" that specifies how many
microseconds kipmid should busy-loop before going to sleep. It affects only the
SI_SM_CALL_WITH_DELAY case, where a delay is acceptible. My experiments have shown
that SI_SM_CALL_WITH_DELAY catches about 99% of smi_event_handler calls. The
new parameter defaults to 0 (previous behavior - busy-loop forever).

With this patch, at least on Fujitsu Siemens hardware, the kipmid CPU time can be
reduced to negligible values without noticeably affecting the performance of the
IPMI interface. Recommended values for kipmid_max_busy on this hardware are in the
100us range (50-500us).

Signed-off-by: Martin Wilck <martin.wilck@fujitsu-siemens.com>
--- linux-2.6.29-rc8/drivers/char/ipmi/ipmi_si_intf.c.orig	2009-03-13 03:39:28.000000000 +0100
+++ linux-2.6.29-rc8/drivers/char/ipmi/ipmi_si_intf.c	2009-03-19 13:24:10.009829167 +0100
@@ -298,6 +298,7 @@ static int force_kipmid[SI_MAX_PARMS];
 static int num_force_kipmid;
 
 static int unload_when_empty = 1;
+static unsigned int kipmid_max_busy;
 
 static int try_smi_init(struct smi_info *smi);
 static void cleanup_one_si(struct smi_info *to_clean);
@@ -927,20 +928,53 @@ static void set_run_to_completion(void *
 	}
 }
 
+
+
+static int ipmi_thread_must_sleep(enum si_sm_result smi_result, int *busy,
+				  struct timespec *busy_time)
+{
+	if (kipmid_max_busy == 0)
+		return 0;
+
+	if (smi_result != SI_SM_CALL_WITH_DELAY) {
+		*busy = 0;
+		return 0;
+	}
+
+	if (*busy == 0) {
+		*busy = 1;
+		getnstimeofday(busy_time);
+		timespec_add_ns(busy_time, kipmid_max_busy*NSEC_PER_USEC);
+	} else {
+		struct timespec now;
+		getnstimeofday(&now);
+		if (unlikely(timespec_compare(&now, busy_time))) {
+			*busy = 0;
+			return 1;
+		}
+	}
+	return 0;
+}
+
 static int ipmi_thread(void *data)
 {
 	struct smi_info *smi_info = data;
 	unsigned long flags;
 	enum si_sm_result smi_result;
+	struct timespec busy_time;
+	int busy = 0;
 
 	set_user_nice(current, 19);
 	while (!kthread_should_stop()) {
+		int must_sleep;
 		spin_lock_irqsave(&(smi_info->si_lock), flags);
 		smi_result = smi_event_handler(smi_info, 0);
 		spin_unlock_irqrestore(&(smi_info->si_lock), flags);
+		must_sleep = ipmi_thread_must_sleep(smi_result,
+						    &busy, &busy_time);
 		if (smi_result == SI_SM_CALL_WITHOUT_DELAY)
 			; /* do nothing */
-		else if (smi_result == SI_SM_CALL_WITH_DELAY)
+		else if (smi_result == SI_SM_CALL_WITH_DELAY && !must_sleep)
 			schedule();
 		else
 			schedule_timeout_interruptible(1);
@@ -1213,6 +1247,11 @@ module_param(unload_when_empty, int, 0);
 MODULE_PARM_DESC(unload_when_empty, "Unload the module if no interfaces are"
 		 " specified or found, default is 1.  Setting to 0"
 		 " is useful for hot add of devices using hotmod.");
+module_param(kipmid_max_busy, uint, 0);
+MODULE_PARM_DESC(kipmid_max_busy,
+		 "Max time (in microseconds) to busy-wait for IPMI data before"
+		 " sleeping. 0 (default) means to wait forever. Set to 100-500"
+		 " if kipmid is using up a lot of CPU time.");
 
 
 static void std_irq_cleanup(struct smi_info *info)

             reply	other threads:[~2009-03-19 16:28 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-03-19 16:27 Martin Wilck [this message]
2009-03-19 21:31 ` [PATCH] limit CPU time spent in kipmid Corey Minyard
2009-03-19 23:51   ` Greg KH
2009-03-20 15:30     ` Corey Minyard
2009-03-20 17:47       ` Greg KH
2009-03-20 18:28         ` Corey Minyard
2009-03-23 13:17           ` [PATCH] limit CPU time spent in kipmid (PREVIOUS WAS BROKEN) Martin Wilck
2009-03-23 15:32             ` Greg KH
2009-03-23 16:20               ` Martin Wilck
2009-03-23 20:39             ` Corey Minyard
2009-03-24  9:22               ` Martin Wilck
2009-03-24  9:30               ` Improving IPMI performance under load Martin Wilck
2009-03-24 13:08                 ` [Openipmi-developer] " Corey Minyard
2009-03-24 13:21                   ` Martin Wilck
2009-03-24 15:50                   ` Matt Domsch
2009-03-24 17:15                     ` Martin Wilck
2009-04-06 13:48               ` [PATCH] limit CPU time spent in kipmid (PREVIOUS WAS BROKEN) Martin Wilck
2009-06-04 18:39                 ` [PATCH] limit CPU time spent in kipmid (version 4) Martin Wilck
2009-03-23 13:25           ` [PATCH] limit CPU time spent in kipmid Martin Wilck
2009-03-19 22:41 ` [Openipmi-developer] " Bela Lubkin

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=49C27281.4040207@fujitsu-siemens.com \
    --to=martin.wilck@fujitsu-siemens.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=minyard@acm.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox