The following patches implement a more generalized infrastructure (than
latency.c) for connecting drivers and subsystem's that could implement
power performance optimizations with the data needed to implement such
policies.

These patches are following up on the discussions and presentations at
the power management summit last summer.

The idea is that from an abstract point of view how much to throttle
hardware can be expressed as a function of QoS types of parameters;
Latency, throughput, and idle time outs.  

The qos_parameter patch is intended to put into place the registration
and notification infrastructure for enabling QoS based policy choices by
drivers, where constraints on throttling are communicated through the
qos_params module.  

Note: it implements a user mode registration protocol where user mode
QoS dependents open a misc device node and write there constraints.  As
long as the file is held open the QoS constraint is included, when the
file closes this constraint is removed and a new constraint is computed.

I would like some feed back on the idea, implementation and possible
applications of this concept we could work on.

The current patch set is 2 patches.
1) qos_param : implements the infrastructure and user mode interface
2) no latency : removes latency.c from the kernel tree replacing it with
qos_param use.

I have a 3rd patch that's a hack application of this qos interface for
communicating latency constraints to e1000.c for tweaking the interrupt
processing of the e1000 based on acceptable latency gathered from the
infrastructure.  I'm not proud of this last one but it helps express the
idea.

Also there is some more documentation and commentary (and an older
patch set) on
http://www.lesswatts.org/projects/power-qos/

thanks,

--mgross

-

[ message continues ]

The following is the qos_param patch that implements a genralization of
latency.c.



Signed-off-by: Mark Gross <mgross@linux.intel.com>

diff -urN -X linux-2.6.23-rc8/Documentation/dontdiff linux-2.6.23-rc8/include/linux/qos_params.h linux-2.6.23-rc8-qos/include/linux/qos_params.h
--- linux-2.6.23-rc8/include/linux/qos_params.h	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.23-rc8-qos/include/linux/qos_params.h	2007-09-26 14:05:20.000000000 -0700
@@ -0,0 +1,35 @@
+/* interface for the qos_power infrastructure of the linux kernel.
+ *
+ * Mark Gross
+ */
+#include <linux/list.h>
+#include <linux/notifier.h>
+#include <linux/miscdevice.h>
+
+struct requirement_list {
+	struct list_head list;
+	union {
+		s32 value;
+		s32 usec;
+		s32 kbps;
+		};
+	char *name;
+};
+
+#define QOS_RESERVED 0
+#define QOS_CPU_DMA_LATENCY 1
+#define QOS_NETWORK_LATENCY 2
+#define QOS_NETWORK_THROUGHPUT 3
+
+#define QOS_NUM_CLASSES 4
+#define QOS_DEFAULT_VALUE -1
+
+int qos_add_requirement(int qos, char *name, s32 value);
+int qos_update_requirement(int qos, char *name, s32 new_value);
+void qos_remove_requirement(int qos, char *name);
+
+int qos_requirement(int qos);
+
+int qos_add_notifier(int qos, struct notifier_block *notifier);
+int qos_remove_notifier(int qos, struct notifier_block *notifier);
+
diff -urN -X linux-2.6.23-rc8/Documentation/dontdiff linux-2.6.23-rc8/kernel/Makefile linux-2.6.23-rc8-qos/kernel/Makefile
--- linux-2.6.23-rc8/kernel/Makefile	2007-09-26 13:54:54.000000000 -0700
+++ linux-2.6.23-rc8-qos/kernel/Makefile	2007-09-26 14:06:38.000000000 -0700
@@ -9,7 +9,7 @@
 	    rcupdate.o extable.o params.o posix-timers.o \
 	    kthread.o wait.o kfifo.o sys_ni.o posix-cpu-timers.o mutex.o \
 	    hrtimer.o rwsem.o latency.o nsproxy.o srcu.o die_notifier.o \
-	    utsname.o
+	    utsname.o qos_params.o
 
 obj-$(CONFIG_STACKTRACE) += stacktrace.o
 obj-y += time/
diff -urN -X linux-2.6.23-rc8/Documentation/dontdiff linux-2.6.23-rc8/kernel/qos_params.c ...
[ message continues ]

Just some general comments (as on irc):

- use 'diffstat -p1 -w70' to summarize each patch
- use checkpatch.pl to check for coding style and other buglets


---
~Randy
Phaedrus says that Quality is about caring.
-

[ message continues ]

updated patch:

Signed-off-by: mark gross <mgross@linux.intel.com>
 Makefile           |    2 
 linux/qos_params.h |   35 +++++
 qos_params.c       |  354 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 390 insertions(+), 1 deletion(-)



diff -urN -X linux-2.6.23-rc8/Documentation/dontdiff linux-2.6.23-rc8/include/linux/qos_params.h linux-2.6.23-rc8-qos/include/linux/qos_params.h
--- linux-2.6.23-rc8/include/linux/qos_params.h	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.23-rc8-qos/include/linux/qos_params.h	2007-09-26 14:05:20.000000000 -0700
@@ -0,0 +1,35 @@
+/* interface for the qos_power infrastructure of the linux kernel.
+ *
+ * Mark Gross
+ */
+#include <linux/list.h>
+#include <linux/notifier.h>
+#include <linux/miscdevice.h>
+
+struct requirement_list {
+	struct list_head list;
+	union {
+		s32 value;
+		s32 usec;
+		s32 kbps;
+		};
+	char *name;
+};
+
+#define QOS_RESERVED 0
+#define QOS_CPU_DMA_LATENCY 1
+#define QOS_NETWORK_LATENCY 2
+#define QOS_NETWORK_THROUGHPUT 3
+
+#define QOS_NUM_CLASSES 4
+#define QOS_DEFAULT_VALUE -1
+
+int qos_add_requirement(int qos, char *name, s32 value);
+int qos_update_requirement(int qos, char *name, s32 new_value);
+void qos_remove_requirement(int qos, char *name);
+
+int qos_requirement(int qos);
+
+int qos_add_notifier(int qos, struct notifier_block *notifier);
+int qos_remove_notifier(int qos, struct notifier_block *notifier);
+
diff -urN -X linux-2.6.23-rc8/Documentation/dontdiff linux-2.6.23-rc8/kernel/Makefile linux-2.6.23-rc8-qos/kernel/Makefile
--- linux-2.6.23-rc8/kernel/Makefile	2007-09-26 13:54:54.000000000 -0700
+++ linux-2.6.23-rc8-qos/kernel/Makefile	2007-09-26 14:06:38.000000000 -0700
@@ -9,7 +9,7 @@
 	    rcupdate.o extable.o params.o posix-timers.o \
 	    kthread.o wait.o kfifo.o sys_ni.o posix-cpu-timers.o mutex.o \
 	    hrtimer.o rwsem.o latency.o nsproxy.o srcu.o die_notifier.o \
-	    utsname.o
+	    utsname.o qos_params.o
 
 ...
[ message continues ]

On Wed, 26 Sep 2007 17:40:20 PDT, Mark Gross said:
(others here are probably better at spotting leaks and races than I am,

So I don't get a choice in the matter if I will be dragging this thing

It's unclear whether these are registering a differing QoS request for each
process/container/whatever that asks for one, or if they're global across the
system.  Also, even though it's "best effort", it would be good to document
what the failure mode is if we get conflicting requests, or an overcommit
situation - do new requests get refused, or allowed and ignored, or allowed
and only sometimes fulfilled.  For instance, assume a gigabit ethernet,
and 3 processes ask for 400 mbits/sec each - who wins, who gets part of what

/dev?  What /dev entry do you use for a network interface?  Should this

Blech.  Is it time for the yearly stamp-out-reinvention-of-max() already?
The use of pointer functions is interesting, but I have to wonder if there's

And then you just pass a pointer to this and kstrdup() it.  Why not kmalloc()
the space initially and just 'dep->name = name;' and be done with it?

General nit - why qos_power_*, when none of the supported QoS parameters
seem to be power-related?
[ message continues ]

You don't get that option with latency.c either at the moment, and it's
arguable whether it's even worth it. The more curious thing is that while
this qos params seems to be an evolution of Arjan's latency.c (and the
drivers that are using it are updated in the rest of the patch set),
latency.c itself is still compiled in. Is this an oversight, or was it
intentional? One set of latency hinting APIs only, please :-)
-

[ message continues ]

It was intentional to compile latnecy.c in (and hence qos_parms is
starting off doing the same thing)  

I agree, "there can be only be one".  (couldn't resist)

I split the patch set up this first patch puts in the qos_param.c and
the second patch yanks latency.c replacing things with its interfaces.

--mgross
-

[ message continues ]

There are no overcommit failure modes.  These QoS parameters are for
constraining aggressive power management throttling from breaking things
thereby enabling better power management solutions.

The QoS values are system wide parameters, not per process or driver.
So the code takes the max or min depending on what class of parameter is
in question.  So if you have multiple applications asking for network
throughput of 400Mbs the current code will let the nic driver find out
that it shouldn't throttle itself to the point where it can provide
400Mbs to any one.

You raise a perspective I overlooked.  For each of the parameter classes
I had been thinking of computing some extrema out of the set of
requests.  However; for throughput it may make more sense to sum up a
collative value.

I'm not sure which way would make more sense.

-

[ message continues ]

I think in many cases you could use a pointer to qos_array[i] instead of 
passing this index 'i' as a function argument.

Roel
-

[ message continues ]

Your } is in a strange place. It looks like it wants to join its friends

an enum would be better for this, especially as people are likely to add

This leaks. You'll have to scan down from i and clean up the kstrdup()
per qos_array element. Presently this will only free the first one to fail

kmalloc() under a spinlock. GFP_KERNEL implies __GFP_WAIT, which can

And here also, still under the spinlock. You can probably rework the
locking just to protect the list, if you really need it at all, it


You also didn't spin_unlock_irqrestore() in the error path, so this bails
out with the lock held and IRQs disabled.
-

[ message continues ]

Other code wants function pointers to the min & max functions.

Anything but 'i'.

---
~Randy
Phaedrus says that Quality is about caring.
-

[ message continues ]

The 'i' is gone.  Will post new patch later today.

--mgross
-

[ message continues ]

Updated qos PM parameter patch:
Note: the replacing of latency.c with this is a separate patch.

this patch attempts to address the issues raised so far.

--mgross



Signed-off-by: mark gross <mgross@linux.intel.com>

mgross@mgross-t43:~/workstuff/power_qos$ diffstat -p1 qos_params_sept_27.patch 
 include/linux/qos_params.h |   35 +++
 kernel/Makefile            |    2 
 kernel/qos_params.c        |  413 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 449 insertions(+), 1 deletion(-)


diff -urN -X linux-2.6.23-rc8/Documentation/dontdiff linux-2.6.23-rc8/include/linux/qos_params.h linux-2.6.23-rc8-qos/include/linux/qos_params.h
--- linux-2.6.23-rc8/include/linux/qos_params.h	1969-12-31 16:00:00.000000000 -0800
+++ linux-2.6.23-rc8-qos/include/linux/qos_params.h	2007-09-27 08:42:54.000000000 -0700
@@ -0,0 +1,35 @@
+/* interface for the qos_power infrastructure of the linux kernel.
+ *
+ * Mark Gross
+ */
+#include <linux/list.h>
+#include <linux/notifier.h>
+#include <linux/miscdevice.h>
+
+struct requirement_list {
+	struct list_head list;
+	union {
+		s32 value;
+		s32 usec;
+		s32 kbps;
+	};
+	char *name;
+};
+
+#define QOS_RESERVED 0
+#define QOS_CPU_DMA_LATENCY 1
+#define QOS_NETWORK_LATENCY 2
+#define QOS_NETWORK_THROUGHPUT 3
+
+#define QOS_NUM_CLASSES 4
+#define QOS_DEFAULT_VALUE -1
+
+int qos_add_requirement(int qos, char *name, s32 value);
+int qos_update_requirement(int qos, char *name, s32 new_value);
+void qos_remove_requirement(int qos, char *name);
+
+int qos_requirement(int qos);
+
+int qos_add_notifier(int qos, struct notifier_block *notifier);
+int qos_remove_notifier(int qos, struct notifier_block *notifier);
+
diff -urN -X linux-2.6.23-rc8/Documentation/dontdiff linux-2.6.23-rc8/kernel/Makefile linux-2.6.23-rc8-qos/kernel/Makefile
--- linux-2.6.23-rc8/kernel/Makefile	2007-09-26 13:54:54.000000000 -0700
+++ linux-2.6.23-rc8-qos/kernel/Makefile	2007-09-26 14:06:38.000000000 -0700
@@ -9,7 +9,7 @@
 	    rcupdate.o ...
[ message continues ]

Minor nit, ret is a pointless variable here, you can just return
misc_register directly.

Other than that, this looks much better!
-

[ message continues ]

I'm partial to the simplicity of not using an enum in this case.  We do






Re-worked.  I didn't need the lock to be held as long as I had it.


fixed by the re-work.

--mgross
-

[ message continues ]

It's a bit rude stealing the entire "qos" namespace like this - there are
many different forms of QoS, some already in-kernel.

s/qos/pm_qos/g ?
-

[ message continues ]

lat_qos or something might be more suitable.. it's a latency property, not
a power management one (even if pm ends up being the primary user of it).
-

[ message continues ]

Its not just latency otherwise I'd agree with you.  right now I'm
thinking of changing things to "qospm" unless there folks feel strongly
about pm_qos.

--mgross
-

[ message continues ]

I suppose it is a bit inconiderate.  I could grow to like pm_qos,
performance_throttling_constraint_hint_infrastructure is a bit too
wordy. 

I suppose I should use qospm as thats the way it was put up on that
lesswatts.org web page. 

Would qospm be good enough?

--mgross
-

[ message continues ]

Don't think it matters a lot, but kernel naming tends to be big-endian (ie:
we have net_ratelimit, not ratelimit_net), so the major part (pm) would
come first under that scheme.

-

[ message continues ]

this makes sense.  pm_qos it is.

--mgross
-

[ message continues ]

The following patch replaces latency.c with qos_params.c and fixes up
users of latency to use qos_params


Signed-off-by: Mark Gross <mgross@linux.intel.com>

diff -urN -X linux-2.6.23-rc8/Documentation/dontdiff linux-2.6.23-rc8-qos/drivers/acpi/processor_idle.c linux-2.6.23-rc8-qos-nolatency.c/drivers/acpi/processor_idle.c
--- linux-2.6.23-rc8-qos/drivers/acpi/processor_idle.c	2007-09-26 13:54:28.000000000 -0700
+++ linux-2.6.23-rc8-qos-nolatency.c/drivers/acpi/processor_idle.c	2007-09-26 14:09:27.000000000 -0700
@@ -38,7 +38,7 @@
 #include <linux/dmi.h>
 #include <linux/moduleparam.h>
 #include <linux/sched.h>	/* need_resched() */
-#include <linux/latency.h>
+#include <linux/qos_params.h>
 #include <linux/clockchips.h>
 
 /*
@@ -605,7 +605,7 @@
 	if (cx->promotion.state &&
 	    ((cx->promotion.state - pr->power.states) <= max_cstate)) {
 		if (sleep_ticks > cx->promotion.threshold.ticks &&
-		  cx->promotion.state->latency <= system_latency_constraint()) {
+		  cx->promotion.state->latency <= qos_requirement(QOS_CPU_DMA_LATENCY)) {
 			cx->promotion.count++;
 			cx->demotion.count = 0;
 			if (cx->promotion.count >=
@@ -649,7 +649,7 @@
 	 * or if the latency of the current state is unacceptable
 	 */
 	if ((pr->power.state - pr->power.states) > max_cstate ||
-		pr->power.state->latency > system_latency_constraint()) {
+		pr->power.state->latency > qos_requirement(QOS_CPU_DMA_LATENCY)) {
 		if (cx->demotion.state)
 			next_state = cx->demotion.state;
 	}
@@ -1173,7 +1173,7 @@
 		   "maximum allowed latency: %d usec\n",
 		   pr->power.state ? pr->power.state - pr->power.states : 0,
 		   max_cstate, (unsigned)pr->power.bm_activity,
-		   system_latency_constraint());
+		   qos_requirement(QOS_CPU_DMA_LATENCY));
 
 	seq_puts(seq, "states:\n");
 
@@ -1280,7 +1280,7 @@
 			       max_cstate);
 		first_run++;
 #ifdef CONFIG_SMP
-		register_latency_notifier(&acpi_processor_latency_notifier);
+		qos_add_notifier(QOS_CPU_DMA_LATENCY, ...
[ message continues ]

The following patch is a bit of a hack to illustrate how the qos
parameter infrastructure can communication information to the e1000
driver to use to set interrupt consolidation policy as a function of
acceptable network latency.  

Its just an example.


Signed-off-by: Mark Gross <mgross@linux.intel.com>

diff -urN -X linux-2.6.23-rc8/Documentation/dontdiff linux-2.6.23-rc8-qos-nolatency.c/drivers/net/e1000/e1000_main.c linux-2.6.23-rc8-qos-apps/drivers/net/e1000/e1000_main.c
--- linux-2.6.23-rc8-qos-nolatency.c/drivers/net/e1000/e1000_main.c	2007-09-26 13:54:33.000000000 -0700
+++ linux-2.6.23-rc8-qos-apps/drivers/net/e1000/e1000_main.c	2007-09-26 15:00:17.000000000 -0700
@@ -27,6 +27,7 @@
 *******************************************************************************/
 
 #include "e1000.h"
+#include <linux/qos_params.h>
 #include <net/ip6_checksum.h>
 
 char e1000_driver_name[] = "e1000";
@@ -2764,6 +2765,7 @@
 {
 	unsigned int retval = itr_setting;
 	struct e1000_hw *hw = &adapter->hw;
+	int requested_latency = qos_requirement(QOS_NETWORK_LATENCY);
 
 	if (unlikely(hw->mac_type < e1000_82540))
 		goto update_itr_done;
@@ -2803,6 +2805,13 @@
 		break;
 	}
 
+	if (requested_latency < 50)
+		retval = lowest_latency;
+	else if (requested_latency < 250)
+		retval = low_latency;
+	else
+		; //don't change the current algorithm
+
 update_itr_done:
 	return retval;
 }
-

[ message continues ]