Re: [RFC] QoS params patch update.
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.
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.
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.
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/