Re: [RFC] QoS params patch update.

2007-09-27 Thread Paul Mundt
On Thu, Sep 27, 2007 at 01:17:39PM -0700, Mark Gross wrote:
> 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.
> 
[snip]

> +static int register_new_qos_misc(struct qos_object *qos)
> +{
> + int ret;
> +
> + qos->qos_power_miscdev.minor = MISC_DYNAMIC_MINOR;
> + qos->qos_power_miscdev.name = qos->name;
> + qos->qos_power_miscdev.fops = _power_fops;
> +
> + ret = misc_register(>qos_power_miscdev);
> +
> + return ret;
> +}
> +
Minor nit, ret is a pointless variable here, you can just return
misc_register directly.

Other than that, this looks much better!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [RFC] QoS params patch update.

2007-09-27 Thread Mark Gross
On Wed, Sep 26, 2007 at 09:05:15PM -0700, Randy Dunlap wrote:
> On Thu, 27 Sep 2007 11:24:40 +0900 Paul Mundt wrote:
> 
> > > +/* static helper functions */
> > > +static s32 max_compare(s32 v1, s32 v2)
> > > +{
> > > + if (v1 < v2)
> > > + return v2;
> > > + else
> > > + return v1;
> > > +}
> > > +
> > > +static s32 min_compare(s32 v1, s32 v2)
> > > +{
> > > + if (v1 < v2)
> > > + return v1;
> > > + else
> > > + return v2;
> > > +}
> > > +
> > min()/max() instead?
> 
> Other code wants function pointers to the min & max functions.
> That's why they are here AFAICT.
> 
> 
> > > +/* assumes qos_lock is held */
> > > +static void update_target(int i)
> > > +{
> > 'target' might be a more meaningful variable name.
> 
> Anything but 'i'.
> 
> ---


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 <[EMAIL PROTECTED]>

[EMAIL PROTECTED]:~/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.0 
-0800
+++ linux-2.6.23-rc8-qos/include/linux/qos_params.h 2007-09-27 
08:42:54.0 -0700
@@ -0,0 +1,35 @@
+/* interface for the qos_power infrastructure of the linux kernel.
+ *
+ * Mark Gross
+ */
+#include 
+#include 
+#include 
+
+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/Makefile2007-09-26 13:54:54.0 -0700
+++ linux-2.6.23-rc8-qos/kernel/Makefile2007-09-26 14:06:38.0 
-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 linux-2.6.23-rc8-qos/kernel/qos_params.c
--- linux-2.6.23-rc8/kernel/qos_params.c1969-12-31 16:00:00.0 
-0800
+++ linux-2.6.23-rc8-qos/kernel/qos_params.c2007-09-27 12:46:13.0 
-0700
@@ -0,0 +1,413 @@
+/*
+ * This module exposes the interface to kernel space for specifying
+ * QoS dependencies.  It provides infrastructure for registration of:
+ *
+ * Dependents on a QoS value : register requirements
+ * Watchers of QoS value : get notified when target QoS value changes
+ *
+ * This QoS design is best effort based.  Dependents register their QoS needs.
+ * Watchers register to keep track of the current QoS needs of the system.
+ *
+ * There are 3 basic classes of QoS parameter: latency, timeout, throughput
+ * each have defined units:
+ * latency: usec
+ * timeout: usec <-- currently not used.
+ * throughput: kbs (kilo byte / sec)
+ *
+ * There are lists of qos_objects each one wrapping requirements, notifiers
+ *
+ * User mode requirements on a QOS parameter register themselves to the
+ * subsystem by opening the device node /dev/... and writing there request to
+ * the node.  As long as the process holds a file handle open to the node the
+ * client continues to be accounted for.  Upon file release the usermode
+ * requirement is removed and a new qos target is computed.  This way when the
+ * requirement that the application has is cleaned up when closes the file
+ * pointer or exits the qos_object will get an opportunity to clean up.
+ *
+ * mark gross [EMAIL PROTECTED]
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+/*
+ * locking rule: all changes to target_value or requirements or notifiers lists
+ * or qos_object list and qos_objects need to happen 

Re: [RFC] QoS params patch update.

2007-09-27 Thread Mark Gross
On Wed, Sep 26, 2007 at 09:05:15PM -0700, Randy Dunlap wrote:
 On Thu, 27 Sep 2007 11:24:40 +0900 Paul Mundt wrote:
 
   +/* static helper functions */
   +static s32 max_compare(s32 v1, s32 v2)
   +{
   + if (v1  v2)
   + return v2;
   + else
   + return v1;
   +}
   +
   +static s32 min_compare(s32 v1, s32 v2)
   +{
   + if (v1  v2)
   + return v1;
   + else
   + return v2;
   +}
   +
  min()/max() instead?
 
 Other code wants function pointers to the min  max functions.
 That's why they are here AFAICT.
 
 
   +/* assumes qos_lock is held */
   +static void update_target(int i)
   +{
  'target' might be a more meaningful variable name.
 
 Anything but 'i'.
 
 ---


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 [EMAIL PROTECTED]

[EMAIL PROTECTED]:~/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.0 
-0800
+++ linux-2.6.23-rc8-qos/include/linux/qos_params.h 2007-09-27 
08:42:54.0 -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/Makefile2007-09-26 13:54:54.0 -0700
+++ linux-2.6.23-rc8-qos/kernel/Makefile2007-09-26 14:06:38.0 
-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 linux-2.6.23-rc8-qos/kernel/qos_params.c
--- linux-2.6.23-rc8/kernel/qos_params.c1969-12-31 16:00:00.0 
-0800
+++ linux-2.6.23-rc8-qos/kernel/qos_params.c2007-09-27 12:46:13.0 
-0700
@@ -0,0 +1,413 @@
+/*
+ * This module exposes the interface to kernel space for specifying
+ * QoS dependencies.  It provides infrastructure for registration of:
+ *
+ * Dependents on a QoS value : register requirements
+ * Watchers of QoS value : get notified when target QoS value changes
+ *
+ * This QoS design is best effort based.  Dependents register their QoS needs.
+ * Watchers register to keep track of the current QoS needs of the system.
+ *
+ * There are 3 basic classes of QoS parameter: latency, timeout, throughput
+ * each have defined units:
+ * latency: usec
+ * timeout: usec -- currently not used.
+ * throughput: kbs (kilo byte / sec)
+ *
+ * There are lists of qos_objects each one wrapping requirements, notifiers
+ *
+ * User mode requirements on a QOS parameter register themselves to the
+ * subsystem by opening the device node /dev/... and writing there request to
+ * the node.  As long as the process holds a file handle open to the node the
+ * client continues to be accounted for.  Upon file release the usermode
+ * requirement is removed and a new qos target is computed.  This way when the
+ * requirement that the application has is cleaned up when closes the file
+ * pointer or exits the qos_object will get an opportunity to clean up.
+ *
+ * mark gross [EMAIL PROTECTED]
+ */
+
+#include linux/qos_params.h
+#include linux/sched.h
+#include linux/spinlock.h
+#include linux/slab.h
+#include linux/time.h
+#include linux/fs.h
+#include linux/device.h
+#include linux/miscdevice.h
+#include linux/string.h
+#include linux/platform_device.h
+#include linux/init.h
+
+#include 

Re: [RFC] QoS params patch update.

2007-09-27 Thread Paul Mundt
On Thu, Sep 27, 2007 at 01:17:39PM -0700, Mark Gross wrote:
 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.
 
[snip]

 +static int register_new_qos_misc(struct qos_object *qos)
 +{
 + int ret;
 +
 + qos-qos_power_miscdev.minor = MISC_DYNAMIC_MINOR;
 + qos-qos_power_miscdev.name = qos-name;
 + qos-qos_power_miscdev.fops = qos_power_fops;
 +
 + ret = misc_register(qos-qos_power_miscdev);
 +
 + return ret;
 +}
 +
Minor nit, ret is a pointless variable here, you can just return
misc_register directly.

Other than that, this looks much better!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/