Re: [PATCH 2.6.25-rc3] cpuidle: fix cpuidle time and usage overflow
On Mon, 2008-02-25 at 11:15 +0100, Ingo Molnar wrote: > * Yi Yang <[EMAIL PROTECTED]> wrote: > > > cpuidle C-state sysfs node time and usage are very easy to overflow > > because they are all of unsigned int type, time will overflow within > > about two hours, usage will take longer time to overflow, but they are > > increasing for ever. > > > > This patch will convert them to unsigned long long. > > what happens if such an overflow happens - any particular regression or > other misbehavior, or just funny looking stats in sysfs? They are just stats info in sysfs, cpuidle's behaviors don't depend on them. I didn't notice any regression or misbehaviors. > > Ingo > - > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/
[PATCH 2.6.25-rc3] cpuidle: fix cpuidle time and usage overflow
cpuidle C-state sysfs node time and usage are very easy to overflow because
they are all of unsigned int type, time will overflow within about two hours,
usage will take longer time to overflow, but they are increasing for ever.
This patch will convert them to unsigned long long.
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
drivers/cpuidle/cpuidle.c |2 +-
drivers/cpuidle/sysfs.c | 10 --
include/linux/cpuidle.h |4 ++--
--- a/include/linux/cpuidle.h 2008-02-25 02:31:26.0 -0500
+++ b/include/linux/cpuidle.h 2008-02-25 04:30:24.0 -0500
@@ -38,8 +38,8 @@ struct cpuidle_state {
unsigned intpower_usage; /* in mW */
unsigned inttarget_residency; /* in US */
- unsigned intusage;
- unsigned inttime; /* in US */
+ unsigned long long usage;
+ unsigned long long time; /* in US */
int (*enter)(struct cpuidle_device *dev,
struct cpuidle_state *state);
--- a/drivers/cpuidle/cpuidle.c 2008-02-25 02:37:14.0 -0500
+++ b/drivers/cpuidle/cpuidle.c 2008-02-25 04:29:19.0 -0500
@@ -67,7 +67,7 @@ static void cpuidle_idle_call(void)
/* enter the state and update stats */
dev->last_residency = target_state->enter(dev, target_state);
dev->last_state = target_state;
- target_state->time += dev->last_residency;
+ target_state->time += (unsigned long long)dev->last_residency;
target_state->usage++;
/* give the governor an opportunity to reflect on the outcome */
--- a/drivers/cpuidle/sysfs.c 2008-02-25 02:33:14.0 -0500
+++ b/drivers/cpuidle/sysfs.c 2008-02-25 03:10:50.0 -0500
@@ -218,6 +218,12 @@ static ssize_t show_state_##_name(struct
return sprintf(buf, "%u\n", state->_name);\
}
+#define define_show_state_ull_function(_name) \
+static ssize_t show_state_##_name(struct cpuidle_state *state, char *buf) \
+{ \
+ return sprintf(buf, "%llu\n", state->_name);\
+}
+
#define define_show_state_str_function(_name) \
static ssize_t show_state_##_name(struct cpuidle_state *state, char *buf) \
{ \
@@ -228,8 +234,8 @@ static ssize_t show_state_##_name(struct
define_show_state_function(exit_latency)
define_show_state_function(power_usage)
-define_show_state_function(usage)
-define_show_state_function(time)
+define_show_state_ull_function(usage)
+define_show_state_ull_function(time)
define_show_state_str_function(name)
define_show_state_str_function(desc)
--
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/
[PATCH 2.6.25-rc3] cpuidle: fix cpuidle time and usage overflow
cpuidle C-state sysfs node time and usage are very easy to overflow because
they are all of unsigned int type, time will overflow within about two hours,
usage will take longer time to overflow, but they are increasing for ever.
This patch will convert them to unsigned long long.
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
drivers/cpuidle/cpuidle.c |2 +-
drivers/cpuidle/sysfs.c | 10 --
include/linux/cpuidle.h |4 ++--
--- a/include/linux/cpuidle.h 2008-02-25 02:31:26.0 -0500
+++ b/include/linux/cpuidle.h 2008-02-25 04:30:24.0 -0500
@@ -38,8 +38,8 @@ struct cpuidle_state {
unsigned intpower_usage; /* in mW */
unsigned inttarget_residency; /* in US */
- unsigned intusage;
- unsigned inttime; /* in US */
+ unsigned long long usage;
+ unsigned long long time; /* in US */
int (*enter)(struct cpuidle_device *dev,
struct cpuidle_state *state);
--- a/drivers/cpuidle/cpuidle.c 2008-02-25 02:37:14.0 -0500
+++ b/drivers/cpuidle/cpuidle.c 2008-02-25 04:29:19.0 -0500
@@ -67,7 +67,7 @@ static void cpuidle_idle_call(void)
/* enter the state and update stats */
dev-last_residency = target_state-enter(dev, target_state);
dev-last_state = target_state;
- target_state-time += dev-last_residency;
+ target_state-time += (unsigned long long)dev-last_residency;
target_state-usage++;
/* give the governor an opportunity to reflect on the outcome */
--- a/drivers/cpuidle/sysfs.c 2008-02-25 02:33:14.0 -0500
+++ b/drivers/cpuidle/sysfs.c 2008-02-25 03:10:50.0 -0500
@@ -218,6 +218,12 @@ static ssize_t show_state_##_name(struct
return sprintf(buf, %u\n, state-_name);\
}
+#define define_show_state_ull_function(_name) \
+static ssize_t show_state_##_name(struct cpuidle_state *state, char *buf) \
+{ \
+ return sprintf(buf, %llu\n, state-_name);\
+}
+
#define define_show_state_str_function(_name) \
static ssize_t show_state_##_name(struct cpuidle_state *state, char *buf) \
{ \
@@ -228,8 +234,8 @@ static ssize_t show_state_##_name(struct
define_show_state_function(exit_latency)
define_show_state_function(power_usage)
-define_show_state_function(usage)
-define_show_state_function(time)
+define_show_state_ull_function(usage)
+define_show_state_ull_function(time)
define_show_state_str_function(name)
define_show_state_str_function(desc)
--
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: [PATCH 2.6.25-rc3] cpuidle: fix cpuidle time and usage overflow
On Mon, 2008-02-25 at 11:15 +0100, Ingo Molnar wrote: * Yi Yang [EMAIL PROTECTED] wrote: cpuidle C-state sysfs node time and usage are very easy to overflow because they are all of unsigned int type, time will overflow within about two hours, usage will take longer time to overflow, but they are increasing for ever. This patch will convert them to unsigned long long. what happens if such an overflow happens - any particular regression or other misbehavior, or just funny looking stats in sysfs? They are just stats info in sysfs, cpuidle's behaviors don't depend on them. I didn't notice any regression or misbehaviors. Ingo - To unsubscribe from this list: send the line unsubscribe linux-acpi in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/
[PATCH 2.6.25-rc1] cpufreq: fix cpufreq policy refcount imbalance
When one cpu is set to offline, the caller process will hang, according to
the trace data, the problem lies in the refcount error in cpufreq driver,
cpufreq_cpu_callback will wait for completion policy->kobj_unregister
which is nerver completed because a refcount error in function
__cpufreq_remove_dev in file driver/cpufreq/cpufreq.c results in not
calling kobject release method.
In driver/cpufreq/cpufreq.c, the refcount of data->kobj isn't 1 when it
will be unregistered, this problem didn't exist in 2.6.24 and earlier.
The root cause is kobject API switch, kobject_init_and_add and kobject_put
replace older kobject_register and kobject_unregister in 2.6.25-rc1,
compared to 2.6.24, kobject_unregister is deleted in function
__cpufreq_remove_dev but it isn't replaced with kobject_put.
This patch adds kobject_put to balance refcount. I noticed Greg suggests
it will fix a power-off issue to remove kobject_get statement block, but i
think that isn't the best way because those code block has existed very long
and it is helpful because the successive statements are invoking relevant
data.
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
cpufreq.c |5 +
1 file changed, 5 insertions(+)
--- a/drivers/cpufreq/cpufreq.c 2008-02-15 04:41:29.0 +0800
+++ b/drivers/cpufreq/cpufreq.c 2008-02-15 06:56:56.0 +0800
@@ -1057,12 +1057,17 @@ static int __cpufreq_remove_dev (struct
unlock_policy_rwsem_write(cpu);
+ /* it matches the previous kobject_get */
kobject_put(>kobj);
/* we need to make sure that the underlying kobj is actually
* not referenced anymore by anybody before we proceed with
* unloading.
*/
+
+ /* unregister data->kobj, it matches kobject_init_and_add */
+ kobject_put(>kobj);
+
dprintk("waiting for dropping of refcount\n");
wait_for_completion(>kobj_unregister);
dprintk("wait complete\n");
--
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/
[PATCH 2.6.25-rc1] cpufreq: fix cpufreq policy refcount imbalance
When one cpu is set to offline, the caller process will hang, according to
the trace data, the problem lies in the refcount error in cpufreq driver,
cpufreq_cpu_callback will wait for completion policy->kobj_unregister
which is nerver completed because a refcount error in function
__cpufreq_remove_dev in file driver/cpufreq/cpufreq.c results in not
calling kobject release method.
In driver/cpufreq/cpufreq.c, the refcount of data->kobj isn't 1 when it
will be unregistered, this problem didn't exist in 2.6.24 and earlier.
The root cause is kobject API switch, kobject_init_and_add and kobject_put
replace older kobject_register and kobject_unregister in 2.6.25-rc1,
compared to 2.6.24, kobject_unregister is deleted in function
__cpufreq_remove_dev but it isn't replaced with kobject_put.
This patch adds kobject_put to balance refcount. I noticed Greg suggests
it will fix a power-off issue to remove kobject_get statement block, but i
think that isn't the best way because those code block has existed very long
and it is helpful because the successive statements are invoking relevant
data.
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
cpufreq.c |5 +
1 file changed, 5 insertions(+)
--- a/drivers/cpufreq/cpufreq.c 2008-02-15 04:41:29.0 +0800
+++ b/drivers/cpufreq/cpufreq.c 2008-02-15 06:56:56.0 +0800
@@ -1057,12 +1057,17 @@ static int __cpufreq_remove_dev (struct
unlock_policy_rwsem_write(cpu);
+ /* it matches the previous kobject_get */
kobject_put(>kobj);
/* we need to make sure that the underlying kobj is actually
* not referenced anymore by anybody before we proceed with
* unloading.
*/
+
+ /* unregister data->kobj, it matches kobject_init_and_add */
+ kobject_put(>kobj);
+
dprintk("waiting for dropping of refcount\n");
wait_for_completion(>kobj_unregister);
dprintk("wait complete\n");
--
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/
[PATCH 2.6.25-rc1] cpufreq: fix cpufreq policy refcount imbalance
When one cpu is set to offline, the caller process will hang, according to the trace data, the problem lies in the refcount error in cpufreq driver, cpufreq_cpu_callback will wait for completion policy-kobj_unregister which is nerver completed because a refcount error in function __cpufreq_remove_dev in file driver/cpufreq/cpufreq.c results in not calling kobject release method. In driver/cpufreq/cpufreq.c, the refcount of data-kobj isn't 1 when it will be unregistered, this problem didn't exist in 2.6.24 and earlier. The root cause is kobject API switch, kobject_init_and_add and kobject_put replace older kobject_register and kobject_unregister in 2.6.25-rc1, compared to 2.6.24, kobject_unregister is deleted in function __cpufreq_remove_dev but it isn't replaced with kobject_put. This patch adds kobject_put to balance refcount. I noticed Greg suggests it will fix a power-off issue to remove kobject_get statement block, but i think that isn't the best way because those code block has existed very long and it is helpful because the successive statements are invoking relevant data. Signed-off-by: Yi Yang [EMAIL PROTECTED] --- cpufreq.c |5 + 1 file changed, 5 insertions(+) --- a/drivers/cpufreq/cpufreq.c 2008-02-15 04:41:29.0 +0800 +++ b/drivers/cpufreq/cpufreq.c 2008-02-15 06:56:56.0 +0800 @@ -1057,12 +1057,17 @@ static int __cpufreq_remove_dev (struct unlock_policy_rwsem_write(cpu); + /* it matches the previous kobject_get */ kobject_put(data-kobj); /* we need to make sure that the underlying kobj is actually * not referenced anymore by anybody before we proceed with * unloading. */ + + /* unregister data-kobj, it matches kobject_init_and_add */ + kobject_put(data-kobj); + dprintk(waiting for dropping of refcount\n); wait_for_completion(data-kobj_unregister); dprintk(wait complete\n); -- 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/
[PATCH 2.6.25-rc1] cpufreq: fix cpufreq policy refcount imbalance
When one cpu is set to offline, the caller process will hang, according to the trace data, the problem lies in the refcount error in cpufreq driver, cpufreq_cpu_callback will wait for completion policy-kobj_unregister which is nerver completed because a refcount error in function __cpufreq_remove_dev in file driver/cpufreq/cpufreq.c results in not calling kobject release method. In driver/cpufreq/cpufreq.c, the refcount of data-kobj isn't 1 when it will be unregistered, this problem didn't exist in 2.6.24 and earlier. The root cause is kobject API switch, kobject_init_and_add and kobject_put replace older kobject_register and kobject_unregister in 2.6.25-rc1, compared to 2.6.24, kobject_unregister is deleted in function __cpufreq_remove_dev but it isn't replaced with kobject_put. This patch adds kobject_put to balance refcount. I noticed Greg suggests it will fix a power-off issue to remove kobject_get statement block, but i think that isn't the best way because those code block has existed very long and it is helpful because the successive statements are invoking relevant data. Signed-off-by: Yi Yang [EMAIL PROTECTED] --- cpufreq.c |5 + 1 file changed, 5 insertions(+) --- a/drivers/cpufreq/cpufreq.c 2008-02-15 04:41:29.0 +0800 +++ b/drivers/cpufreq/cpufreq.c 2008-02-15 06:56:56.0 +0800 @@ -1057,12 +1057,17 @@ static int __cpufreq_remove_dev (struct unlock_policy_rwsem_write(cpu); + /* it matches the previous kobject_get */ kobject_put(data-kobj); /* we need to make sure that the underlying kobj is actually * not referenced anymore by anybody before we proceed with * unloading. */ + + /* unregister data-kobj, it matches kobject_init_and_add */ + kobject_put(data-kobj); + dprintk(waiting for dropping of refcount\n); wait_for_completion(data-kobj_unregister); dprintk(wait complete\n); -- 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/
[PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 3
Andrew, i'm really very very sorry for those mistakes, here is the latest, it adds documentation for every new function, please use it to replace the patch in -mm tree you added just now: add-new-string-functions-strict_strto-and-convert-kernel-params-to-use-them.patch Thank you, Andrew and Randy. Currently, for every sysfs node, the callers will be responsible for implementing store operation, so many many callers are doing duplicate things to validate input, they have the same mistakes because they are calling simple_strtol/ul/ll/uul, especially for module params, they are just numeric, but you can echo such values as 0x1234xxx, 0888 and 1234aaa, for these cases, module params store operation just ignores succesive invalid char and converts prefix part to a numeric although input is acctually invalid. This patch tries to fix the aforementioned issues and implements strict_strtox serial functions, kernel/params.c uses them to strictly validate input, so module params will reject such values as 0x1234 and returns an error: write error: Invalid argument Any modules which export numeric sysfs node can use strict_strtox instead of simple_strtox to reject any invalid input. Please consider to merge to -mm tree in order to test. Here are some test results: Before applying this patch: [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 01 > /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 018 > /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 01a > /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# After applying this patch: [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# echo 01 > /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# echo 018 > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# echo 01a > /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# Signed-off-by: Yi Yang <[EMAIL PROTECTED]> --- include/linux/kernel.h |4 + kernel/params.c| 20 +++ lib/vsprintf.c | 123 + 3 files changed, 137 insertions(+), 10 deletions(-) --- a/include/linux/kernel.h2008-01-31 00:41:46.0 +0800 +++ b/include/linux/kernel.h2008-02-01 04:30:49.0 +0800 @@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons extern long simple_strtol(const char *,char **,unsigned int); extern unsigned long long simple_strtoull(const char *,char **,unsigned int); extern long long simple_strtoll(const char *,char **,unsigned int); +extern int strict_strtoul(const char *, unsigned int, unsigned long *); +extern int strict_strtol(const char *, unsigned int, long *); +extern int strict_strtoull(const char *, unsigned int, unsigned long long *); +extern int strict_strtoll(const char *, unsigned int, long long *); extern int sprintf(char * buf, const char * fmt, ...) __attribute__ ((format (printf, 2, 3))); extern int vsprintf(char *buf, const char *, va_list) --- a/lib/vsprintf.c2008-01-30 08:13:03.0 +0800 +++ b/lib/vsprintf.c2008-02-01 07:36:57.0 +0800 @@ -126,6 +126,129 @@ long long simple_strtoll(const char *cp, return simple_strtoull(cp,endp,base); } + +/** + * strict_strtoul - convert a string to an unsigned long strictly + * @cp: The string to be converted + * @base: The number base to use + * @res: The converted result value + * + * strict_
[PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 3
Andrew, i'm really very very sorry for those mistakes, here is the latest, it adds documentation for every new function, please use it to replace the patch in -mm tree you added just now: add-new-string-functions-strict_strto-and-convert-kernel-params-to-use-them.patch Thank you, Andrew and Randy. Currently, for every sysfs node, the callers will be responsible for implementing store operation, so many many callers are doing duplicate things to validate input, they have the same mistakes because they are calling simple_strtol/ul/ll/uul, especially for module params, they are just numeric, but you can echo such values as 0x1234xxx, 0888 and 1234aaa, for these cases, module params store operation just ignores succesive invalid char and converts prefix part to a numeric although input is acctually invalid. This patch tries to fix the aforementioned issues and implements strict_strtox serial functions, kernel/params.c uses them to strictly validate input, so module params will reject such values as 0x1234 and returns an error: write error: Invalid argument Any modules which export numeric sysfs node can use strict_strtox instead of simple_strtox to reject any invalid input. Please consider to merge to -mm tree in order to test. Here are some test results: Before applying this patch: [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000 /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000g /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000 /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 01 /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 018 /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 01a /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# After applying this patch: [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000 /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000g /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo 0x1000 /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# echo 01 /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# echo 018 /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# echo 01a /sys/module/e1000/parameters/copybreak -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# echo -n 4096 /sys/module/e1000/parameters/copybreak [EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak 4096 [EMAIL PROTECTED] /]# Signed-off-by: Yi Yang [EMAIL PROTECTED] --- include/linux/kernel.h |4 + kernel/params.c| 20 +++ lib/vsprintf.c | 123 + 3 files changed, 137 insertions(+), 10 deletions(-) --- a/include/linux/kernel.h2008-01-31 00:41:46.0 +0800 +++ b/include/linux/kernel.h2008-02-01 04:30:49.0 +0800 @@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons extern long simple_strtol(const char *,char **,unsigned int); extern unsigned long long simple_strtoull(const char *,char **,unsigned int); extern long long simple_strtoll(const char *,char **,unsigned int); +extern int strict_strtoul(const char *, unsigned int, unsigned long *); +extern int strict_strtol(const char *, unsigned int, long *); +extern int strict_strtoull(const char *, unsigned int, unsigned long long *); +extern int strict_strtoll(const char *, unsigned int, long long *); extern int sprintf(char * buf, const char * fmt, ...) __attribute__ ((format (printf, 2, 3))); extern int vsprintf(char *buf, const char *, va_list) --- a/lib/vsprintf.c2008-01-30 08:13:03.0 +0800 +++ b/lib/vsprintf.c2008-02-01 07:36:57.0 +0800 @@ -126,6 +126,129 @@ long long simple_strtoll(const char *cp, return simple_strtoull(cp,endp,base); } + +/** + * strict_strtoul - convert a string to an unsigned long strictly + * @cp: The string to be converted + * @base: The number base to use + * @res: The converted result value + * + * strict_strtoul converts a string to an unsigned long only
[PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 2
This patch is a resend, it changes previous name "real_" to "strict_"
according to Randy Dunlap's feedback. Please consider to apply. thanks.
Currently, for every sysfs node, the callers will be responsible for
implementing store operation, so many many callers are doing duplicate
things to validate input, they have the same mistakes because they are
calling simple_strtol/ul/ll/uul, especially for module params, they are
just numeric, but you can echo such values as 0x1234xxx, 0888 and
1234aaa, for these cases, module params store operation just ignores
succesive invalid char and converts prefix part to a numeric although
input is acctually invalid.
This patch tries to fix the aforementioned issues and implements strict_strtox
serial functions, kernel/params.c uses them to strictly validate input,
so module params will reject such values as 0x1234 and returns an error:
write error: Invalid argument
Any modules which export numeric sysfs node can use strict_strtox instead of
simple_strtox to reject any invalid input.
Here are some test results:
Before applying this patch:
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 >
/sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 01 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 018 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 01a > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]#
After applying this patch:
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 >
/sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo 01 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# echo 018 > /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo 01a > /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]#
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
include/linux/kernel.h |4
kernel/params.c| 20 ++--
lib/vsprintf.c | 46 ++
3 files changed, 60 insertions(+), 10 deletions(-)
--- a/include/linux/kernel.h2008-01-31 00:41:46.0 +0800
--- b/include/linux/kernel.h2008-02-01 04:30:49.0 +0800
@@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
extern long simple_strtol(const char *,char **,unsigned int);
extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
extern long long simple_strtoll(const char *,char **,unsigned int);
+extern int strict_strtoul(const char *, unsigned int, unsigned long *);
+extern int strict_strtol(const char *, unsigned int, long *);
+extern int strict_strtoull(const char *, unsigned int, unsigned long long *);
+extern int strict_strtoll(const char *, unsigned int, long long *);
extern int sprintf(char * buf, const char * fmt, ...)
__attribute__ ((format (printf, 2, 3)));
extern int vsprintf(char *buf, const char *, va_list)
--- a/lib/vsprintf.c2008-01-30 08:13:03.0 +0800
--- b/lib/vsprintf.c2008-02-01 04:33:27.0 +0800
@@ -126,6 +126,52 @@ long long simple_strtoll(const char *cp,
return simple_strtoull(cp,endp,base);
}
+#define define_strict_strtoux(type, valtype) \
+int strict_strtou##type(const char *cp, unsigned int base, valtype *res)\
+{ \
+ char *tail; \
+ valtype val;
Re: [PATCH 2.6.24] Add new string functions real_strtoul and change kernel params to use them
On Thu, 2008-01-31 at 09:03 -0800, Randy Dunlap wrote: > On Thu, 31 Jan 2008 09:18:22 +0800 Yi Yang wrote: > > > Currently, for every sysfs node, the callers will be responsible for > > implementing store operation, so many many callers are doing duplicate > > things to validate input, they have the same mistakes because they are > > calling simple_strtol/ul/ll/ull, especially for module params, they are > > just numeric, but you can echo such values as 0x1234xxx, 0888 and > > 1234aaa, for these cases, module params store operation just ignores > > successive invalid char and converts prefix part to a numeric although > > input is actually invalid. > > > > This patch tries to fix the aforementioned issues and implements real_strtox > > serial functions, kernel/params.c uses them to strictly validate input, > > so module params will reject such values as 0x1234 and returns an error: > > How about a prefix of safe_ or strict_ or something other than real_? > real_ sounds too much like a real number function string parser. > I named it as strict_ at the beginning, but it results in some alignment issues checkpatch.pl will always warn, i don't know if warnings will make the patch out of the door. In kernel/params.c, STANDARD_PARAM_DEF(a function definition macro) will be over 80 chars, is it correct coding style to split it to two lines? -- 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/
[PATCH 2.6.24] Add new string functions real_strtoul and change kernel params to use them
Currently, for every sysfs node, the callers will be responsible for
implementing store operation, so many many callers are doing duplicate
things to validate input, they have the same mistakes because they are
calling simple_strtol/ul/ll/ull, especially for module params, they are
just numeric, but you can echo such values as 0x1234xxx, 0888 and
1234aaa, for these cases, module params store operation just ignores
successive invalid char and converts prefix part to a numeric although
input is actually invalid.
This patch tries to fix the aforementioned issues and implements real_strtox
serial functions, kernel/params.c uses them to strictly validate input,
so module params will reject such values as 0x1234 and returns an error:
write error: Invalid argument
Any modules which export numeric sysfs node can use real_strtox instead of
simple_strtox to reject any invalid input. Please consider to merge to -mm tree
in order to test.
Here are some test results:
Before applying this patch:
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 >
/sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 01 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 018 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 01a > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]#
After applying this patch:
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000g > /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 >
/sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo 01 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# echo 018 > /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo 01a > /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo -n 4096 > /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]#
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
include/linux/kernel.h |4
kernel/params.c| 20 ++--
lib/vsprintf.c | 46 ++
3 files changed, 60 insertions(+), 10 deletions(-)
--- a/include/linux/kernel.h2008-01-31 00:41:46.0 +0800
+++ b/include/linux/kernel.h2008-01-31 01:12:33.0 +0800
@@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
extern long simple_strtol(const char *,char **,unsigned int);
extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
extern long long simple_strtoll(const char *,char **,unsigned int);
+extern int real_strtoul(const char *, unsigned int, unsigned long *);
+extern int real_strtol(const char *, unsigned int, long *);
+extern int real_strtoull(const char *, unsigned int, unsigned long long *);
+extern int real_strtoll(const char *, unsigned int, long long *);
extern int sprintf(char * buf, const char * fmt, ...)
__attribute__ ((format (printf, 2, 3)));
extern int vsprintf(char *buf, const char *, va_list)
--- a/lib/vsprintf.c2008-01-30 08:13:03.0 +0800
+++ b/lib/vsprintf.c2008-01-31 05:19:31.0 +0800
@@ -126,6 +126,52 @@ long long simple_strtoll(const char *cp,
return simple_strtoull(cp,endp,base);
}
+#define define_real_strtoux(type, valtype) \
+int real_strtou##type(const char *cp, unsigned int base, valtype *res) \
+{ \
+ char *tail; \
+ valtype val;\
+ size_t len; \
+
[PATCH 2.6.24] Add new string functions real_strtoul and change kernel params to use them
Currently, for every sysfs node, the callers will be responsible for
implementing store operation, so many many callers are doing duplicate
things to validate input, they have the same mistakes because they are
calling simple_strtol/ul/ll/ull, especially for module params, they are
just numeric, but you can echo such values as 0x1234xxx, 0888 and
1234aaa, for these cases, module params store operation just ignores
successive invalid char and converts prefix part to a numeric although
input is actually invalid.
This patch tries to fix the aforementioned issues and implements real_strtox
serial functions, kernel/params.c uses them to strictly validate input,
so module params will reject such values as 0x1234 and returns an error:
write error: Invalid argument
Any modules which export numeric sysfs node can use real_strtox instead of
simple_strtox to reject any invalid input. Please consider to merge to -mm tree
in order to test.
Here are some test results:
Before applying this patch:
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000g /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000
/sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 01 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 018 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 01a /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]#
After applying this patch:
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000g /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000
/sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo 01 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# echo 018 /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo 01a /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo -n 4096 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]#
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
include/linux/kernel.h |4
kernel/params.c| 20 ++--
lib/vsprintf.c | 46 ++
3 files changed, 60 insertions(+), 10 deletions(-)
--- a/include/linux/kernel.h2008-01-31 00:41:46.0 +0800
+++ b/include/linux/kernel.h2008-01-31 01:12:33.0 +0800
@@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
extern long simple_strtol(const char *,char **,unsigned int);
extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
extern long long simple_strtoll(const char *,char **,unsigned int);
+extern int real_strtoul(const char *, unsigned int, unsigned long *);
+extern int real_strtol(const char *, unsigned int, long *);
+extern int real_strtoull(const char *, unsigned int, unsigned long long *);
+extern int real_strtoll(const char *, unsigned int, long long *);
extern int sprintf(char * buf, const char * fmt, ...)
__attribute__ ((format (printf, 2, 3)));
extern int vsprintf(char *buf, const char *, va_list)
--- a/lib/vsprintf.c2008-01-30 08:13:03.0 +0800
+++ b/lib/vsprintf.c2008-01-31 05:19:31.0 +0800
@@ -126,6 +126,52 @@ long long simple_strtoll(const char *cp,
return simple_strtoull(cp,endp,base);
}
+#define define_real_strtoux(type, valtype) \
+int real_strtou##type(const char *cp, unsigned int base, valtype *res) \
+{ \
+ char *tail; \
+ valtype val;\
+ size_t len; \
+ \
+ *res = 0
[PATCH 2.6.24] Add new string functions strict_strto* and convert kernel params to use them, take 2
This patch is a resend, it changes previous name real_ to strict_
according to Randy Dunlap's feedback. Please consider to apply. thanks.
Currently, for every sysfs node, the callers will be responsible for
implementing store operation, so many many callers are doing duplicate
things to validate input, they have the same mistakes because they are
calling simple_strtol/ul/ll/uul, especially for module params, they are
just numeric, but you can echo such values as 0x1234xxx, 0888 and
1234aaa, for these cases, module params store operation just ignores
succesive invalid char and converts prefix part to a numeric although
input is acctually invalid.
This patch tries to fix the aforementioned issues and implements strict_strtox
serial functions, kernel/params.c uses them to strictly validate input,
so module params will reject such values as 0x1234 and returns an error:
write error: Invalid argument
Any modules which export numeric sysfs node can use strict_strtox instead of
simple_strtox to reject any invalid input.
Here are some test results:
Before applying this patch:
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000g /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000
/sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 01 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 018 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 01a /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]#
After applying this patch:
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000g /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo 0x1000
/sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo 01 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# echo 018 /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo 01a /sys/module/e1000/parameters/copybreak
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]# echo -n 4096 /sys/module/e1000/parameters/copybreak
[EMAIL PROTECTED] /]# cat /sys/module/e1000/parameters/copybreak
4096
[EMAIL PROTECTED] /]#
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
include/linux/kernel.h |4
kernel/params.c| 20 ++--
lib/vsprintf.c | 46 ++
3 files changed, 60 insertions(+), 10 deletions(-)
--- a/include/linux/kernel.h2008-01-31 00:41:46.0 +0800
--- b/include/linux/kernel.h2008-02-01 04:30:49.0 +0800
@@ -141,6 +141,10 @@ extern unsigned long simple_strtoul(cons
extern long simple_strtol(const char *,char **,unsigned int);
extern unsigned long long simple_strtoull(const char *,char **,unsigned int);
extern long long simple_strtoll(const char *,char **,unsigned int);
+extern int strict_strtoul(const char *, unsigned int, unsigned long *);
+extern int strict_strtol(const char *, unsigned int, long *);
+extern int strict_strtoull(const char *, unsigned int, unsigned long long *);
+extern int strict_strtoll(const char *, unsigned int, long long *);
extern int sprintf(char * buf, const char * fmt, ...)
__attribute__ ((format (printf, 2, 3)));
extern int vsprintf(char *buf, const char *, va_list)
--- a/lib/vsprintf.c2008-01-30 08:13:03.0 +0800
--- b/lib/vsprintf.c2008-02-01 04:33:27.0 +0800
@@ -126,6 +126,52 @@ long long simple_strtoll(const char *cp,
return simple_strtoull(cp,endp,base);
}
+#define define_strict_strtoux(type, valtype) \
+int strict_strtou##type(const char *cp, unsigned int base, valtype *res)\
+{ \
+ char *tail; \
+ valtype val;\
+ size_t len
[RFC PATCH 2.6.24] x86: add sysfs interface for cpuid module, try 2
This patch is try 2, it should cover VIA/Cyrix and Transmeta Crusoe, please
help to test it if anybody has VIA/Cyrix and Transmeta Crusoe. This patch just
makes users get cpuid raw data more conveniently by
/sys/devices/syste/cpu/cpu*/cpuid/* without using any userspace application.
Current cpuid module will create a char device for every logical cpu,
when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop,
the root cause is that cpuid module doesn't decide wether a cpuid level
is valid, it just uses an offset to denote cpuid level and take it to
cpuid instruction, cpuid instruction will ignore it and return some data
specific to cpu model, cpuid doesn't an error return value because it is
void type. So cpuid module will execute cpuid continuously and return
data although most of data make no sense.
This patch tries to add a sysfs interface for cpuid, users can see all the
available cpuid levels, specify a specific level and get cpuid corresponding
to this cpuid level.
For every logical cpu, this patch will create a cpuid directory under
/sys/devices/system/cpu/cpu*/, there are three entries under cpuid:
avail_levelscur_level cur_cpuid
A user can get all the available cpuid levels from avail_levels, he/she can
set one available cpuid level to cur_level, then he/she can get cpuid from
cur_cpuid, cur_cpuid corresponds to cur_level.
Here are some example output:
[EMAIL PROTECTED] /]# ls /sys/devices/system/cpu/cpu0/cpuid
avail_levels cur_cpuid cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/avail_levels
0x
0x0001
0x0002
0x0003
0x0004
0x0005
0x0006
0x0007
0x0008
0x0009
0x000a
0x8000
0x8001
0x8002
0x8003
0x8004
0x8005
0x8006
0x8007
0x8008
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_level
0x
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x000a 0x756e6547 0x6c65746e 0x49656e69
[EMAIL PROTECTED] /]# echo 0x8007 >
/sys/devices/system/cpu/cpu0/cpuid/cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x 0x 0x 0x
[EMAIL PROTECTED] /]# echo 0x8001 >
/sys/devices/system/cpu/cpu0/cpuid/cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x 0x 0x0001 0x2010
[EMAIL PROTECTED] /]# ls /sys/devices/system/cpu/cpu1/cpuid
avail_levels cur_cpuid cur_level
[EMAIL PROTECTED] /]#
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
cpuid.c | 255 +++-
1 file changed, 254 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpuid.c 2008-01-28 01:28:48.0 +0800
+++ b/arch/x86/kernel/cpuid.c 2008-01-30 07:31:40.0 +0800
@@ -175,6 +175,250 @@ static struct notifier_block __cpuinitda
.notifier_call = cpuid_class_cpu_callback,
};
+struct cpuid_info {
+ int cpu;
+ unsigned int cur_level;
+ unsigned int min_level;
+ unsigned int max_level;
+ unsigned int min_ext_level;
+ unsigned int max_ext_level;
+ struct kobject kobj;
+};
+
+struct cpuid_attr {
+ struct attribute attr;
+ ssize_t (*show)(struct cpuid_info *, char *);
+ ssize_t (*store)(struct cpuid_info *, const char *, size_t count);
+};
+
+static struct cpuid_info cpuid_infos[NR_CPUS];
+
+static cpumask_t cpu_map = CPU_MASK_NONE;
+
+#define A32(a, b, c, d) (((d) << 24) + ((c) << 16) + ((b) << 8) + (a))
+
+static int is_centaur(u32 *data)
+{
+ return data[1] == A32('C', 'e', 'n', 't') &&
+ data[3] == A32('a', 'u', 'r', 'H') &&
+ data[2] == A32('a', 'u', 'l', 's');
+}
+
+static int is_transmeta(u32 *data)
+{
+ return data[1] == A32('G', 'e', 'n', 'u') &&
+ data[3] == A32('i', 'n', 'e', 'T') &&
+ data[2] == A32('M', 'x', '8', '6');
+}
+
+static ssize_t show_cur_level(struct cpuid_info *cpuid_info_p, char *buf)
+{
+ return sprintf(buf, "0x%08x\n", cpuid_info_p->cur_level);
+}
+
+static ssize_t store_cur_level(struct cpuid_info *cpuid_info_p,
+ const char *buf, size_t count)
+{
+ char *p;
+ unsigned int val;
+ size_t len = 13;
+ char tmpbuf[16];
+
+ if ((count > len) || (count <= 0))
+ return -EINVAL;
+
+ len = count;
+ if (buf[count-1] == '\n')
+len--;
+
+ memcpy(tmpbuf, buf, len);
+ tmpbuf[len] = '\0';
+ val = simple_strtoul(tmpbuf, , 0);
+
+ if (*p != '\0')
+ return -EINVAL;
+ if (((val >= cpuid_info_p->min_level)
+ && (val <= cpuid_info_p->max_level))
+ || ((val >= cpuid_info_p->min_ext_level)
+ && (val <= cpuid_info_p->max_ext_level))) {
+
[RFC PATCH 2.6.24] x86: add sysfs interface for cpuid module, try 2
This patch is try 2, it should cover VIA/Cyrix and Transmeta Crusoe, please
help to test it if anybody has VIA/Cyrix and Transmeta Crusoe. This patch just
makes users get cpuid raw data more conveniently by
/sys/devices/syste/cpu/cpu*/cpuid/* without using any userspace application.
Current cpuid module will create a char device for every logical cpu,
when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop,
the root cause is that cpuid module doesn't decide wether a cpuid level
is valid, it just uses an offset to denote cpuid level and take it to
cpuid instruction, cpuid instruction will ignore it and return some data
specific to cpu model, cpuid doesn't an error return value because it is
void type. So cpuid module will execute cpuid continuously and return
data although most of data make no sense.
This patch tries to add a sysfs interface for cpuid, users can see all the
available cpuid levels, specify a specific level and get cpuid corresponding
to this cpuid level.
For every logical cpu, this patch will create a cpuid directory under
/sys/devices/system/cpu/cpu*/, there are three entries under cpuid:
avail_levelscur_level cur_cpuid
A user can get all the available cpuid levels from avail_levels, he/she can
set one available cpuid level to cur_level, then he/she can get cpuid from
cur_cpuid, cur_cpuid corresponds to cur_level.
Here are some example output:
[EMAIL PROTECTED] /]# ls /sys/devices/system/cpu/cpu0/cpuid
avail_levels cur_cpuid cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/avail_levels
0x
0x0001
0x0002
0x0003
0x0004
0x0005
0x0006
0x0007
0x0008
0x0009
0x000a
0x8000
0x8001
0x8002
0x8003
0x8004
0x8005
0x8006
0x8007
0x8008
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_level
0x
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x000a 0x756e6547 0x6c65746e 0x49656e69
[EMAIL PROTECTED] /]# echo 0x8007
/sys/devices/system/cpu/cpu0/cpuid/cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x 0x 0x 0x
[EMAIL PROTECTED] /]# echo 0x8001
/sys/devices/system/cpu/cpu0/cpuid/cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x 0x 0x0001 0x2010
[EMAIL PROTECTED] /]# ls /sys/devices/system/cpu/cpu1/cpuid
avail_levels cur_cpuid cur_level
[EMAIL PROTECTED] /]#
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
cpuid.c | 255 +++-
1 file changed, 254 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpuid.c 2008-01-28 01:28:48.0 +0800
+++ b/arch/x86/kernel/cpuid.c 2008-01-30 07:31:40.0 +0800
@@ -175,6 +175,250 @@ static struct notifier_block __cpuinitda
.notifier_call = cpuid_class_cpu_callback,
};
+struct cpuid_info {
+ int cpu;
+ unsigned int cur_level;
+ unsigned int min_level;
+ unsigned int max_level;
+ unsigned int min_ext_level;
+ unsigned int max_ext_level;
+ struct kobject kobj;
+};
+
+struct cpuid_attr {
+ struct attribute attr;
+ ssize_t (*show)(struct cpuid_info *, char *);
+ ssize_t (*store)(struct cpuid_info *, const char *, size_t count);
+};
+
+static struct cpuid_info cpuid_infos[NR_CPUS];
+
+static cpumask_t cpu_map = CPU_MASK_NONE;
+
+#define A32(a, b, c, d) (((d) 24) + ((c) 16) + ((b) 8) + (a))
+
+static int is_centaur(u32 *data)
+{
+ return data[1] == A32('C', 'e', 'n', 't')
+ data[3] == A32('a', 'u', 'r', 'H')
+ data[2] == A32('a', 'u', 'l', 's');
+}
+
+static int is_transmeta(u32 *data)
+{
+ return data[1] == A32('G', 'e', 'n', 'u')
+ data[3] == A32('i', 'n', 'e', 'T')
+ data[2] == A32('M', 'x', '8', '6');
+}
+
+static ssize_t show_cur_level(struct cpuid_info *cpuid_info_p, char *buf)
+{
+ return sprintf(buf, 0x%08x\n, cpuid_info_p-cur_level);
+}
+
+static ssize_t store_cur_level(struct cpuid_info *cpuid_info_p,
+ const char *buf, size_t count)
+{
+ char *p;
+ unsigned int val;
+ size_t len = 13;
+ char tmpbuf[16];
+
+ if ((count len) || (count = 0))
+ return -EINVAL;
+
+ len = count;
+ if (buf[count-1] == '\n')
+len--;
+
+ memcpy(tmpbuf, buf, len);
+ tmpbuf[len] = '\0';
+ val = simple_strtoul(tmpbuf, p, 0);
+
+ if (*p != '\0')
+ return -EINVAL;
+ if (((val = cpuid_info_p-min_level)
+(val = cpuid_info_p-max_level))
+ || ((val = cpuid_info_p-min_ext_level)
+(val = cpuid_info_p-max_ext_level))) {
+ cpuid_info_p-cur_level = val;
+ return count;
+ } else
+ return -EINVAL;
+}
+
+static ssize_t show_avail_levels(struct
Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module
On Tue, 2008-01-29 at 09:44 +0100, Sam Ravnborg wrote:
> > +
> > +static struct notifier_block __cpuinitdata cpuid_sysfs_cpu_notifier = {
> > + .notifier_call = cpuid_sysfs_cpu_callback,
> > +};
> Data is annotated _cpuintidata
>
> but
>
> > +
> Data is annotated _cpuintidata
>
> > @@ -217,11 +445,14 @@ static void __exit cpuid_exit(void)
> > {
> > int cpu = 0;
> >
> > - for_each_online_cpu(cpu)
> > + for_each_online_cpu(cpu) {
> > cpuid_device_destroy(cpu);
> > + remove_cpuid_sysfs(cpu);
> > + }
> > class_destroy(cpuid_class);
> > unregister_chrdev(CPUID_MAJOR, "cpu/cpuid");
> > unregister_hotcpu_notifier(_class_cpu_notifier);
> > + unregister_hotcpu_notifier(_sysfs_cpu_notifier);
>
> used in an __exit function.
>
> You should have seen a Section mismatch warning for this.
> The right fix is to annotate the cpuid_sysfs_cpu_notifier
> with __initdata_refok (soon to be named __refdata)
> Or even better to declare it const and use _refconst.
I think __cpuinitdata is different from __initdata, i have tested it
by insmod, rmmod, echo 0/1 > /sys/devices/system/cpu/cpu1/online
repeatly, it hasn't any issue.
>
> Sam
--
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: [PATCH 2.6.24] x86: add sysfs interface for cpuid module
On Tue, 2008-01-29 at 23:17 -0800, H. Peter Anvin wrote:
> Yi Yang wrote:
> >>
> >> It's broken, because it doesn't take into account the fact that Intel
> >> broke CPUID level 4 and made it "repeating" (neither did the cpuid char
> >> device, because it predated the Intel braindamage; I've had a patch for
> >> it privately for a while, but didn't push it upstream because paravirt
> >> broke it royally and I wanted the situation to settle down.)
>
> > level 4 doesn't result in repeating on Intel CPU, cpuid module sets
> > file offset to level, so cat /dev/cpu/*/cpuid will run cpuid instruction
> > continuously.
>
> The issue is that Intel suddenly made CPUID ECX-sensitive, which there
> is no way to represent.
Function cpuid has reset ecx to 0 immediate before calling to __cpuid,
so this shouldn't be a problem now.
in include/asm-x86/processor_32.h
/*
* Generic CPUID function
* clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx
* resulting in stale register contents being returned.
*/
static inline void cpuid(unsigned int op,
unsigned int *eax, unsigned int *ebx,
unsigned int *ecx, unsigned int *edx)
{
*eax = op;
*ecx = 0;
__cpuid(eax, ebx, ecx, edx);
}
>
> As far as cat /dev/cpu/*/cpuid, that's a user error.
>
> -hpa
>
--
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: [PATCH 2.6.24] x86: add sysfs interface for cpuid module
On Tue, 2008-01-29 at 07:51 -0800, H. Peter Anvin wrote: > Yi Yang wrote: > > Current cpuid module will create a char device for every logical cpu, > > when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop, > > the root cause is that cpuid module doesn't decide wether a cpuid level > > is valid, it just uses an offset to denote cpuid level and take it to > > cpuid instruction, cpuid instruction will ignore it and return some data > > > > This patch uses sysfs to avoid limitless loop and provide more flexible > > interface for cpuid, please consider to merge to -mm tree in order to test. > > This is broken. > > Triple broken. > > It's broken, because it doesn't take into account the fact that Intel > broke CPUID level 4 and made it "repeating" (neither did the cpuid char > device, because it predated the Intel braindamage; I've had a patch for > it privately for a while, but didn't push it upstream because paravirt > broke it royally and I wanted the situation to settle down.) level 4 doesn't result in repeating on Intel CPU, cpuid module sets file offset to level, so cat /dev/cpu/*/cpuid will run cpuid instruction continuously. -- 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: [PATCH 2.6.24] x86: add sysfs interface for cpuid module
On Tue, 2008-01-29 at 07:51 -0800, H. Peter Anvin wrote: > Yi Yang wrote: > > Current cpuid module will create a char device for every logical cpu, > > when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop, > > the root cause is that cpuid module doesn't decide wether a cpuid level > > is valid, it just uses an offset to denote cpuid level and take it to > > cpuid instruction, cpuid instruction will ignore it and return some data > > specific to cpu model, cpuid doesn't an error return value because it is > > void type. So cpuid module will execute cpuid continuously and return > > data although most of data make no sense. > > > > This patch tries to add a sysfs interface for cpuid, users can see all the > > available cpuid levels, specify a specific level and get cpuid corresponding > > to this cpuid level. > > > > For every logical cpu, this patch will create a cpuid directory under > > /sys/devices/system/cpu/cpu*/, there are three entries under cpuid: > > > > avail_levelscur_level cur_cpuid > > > > A user can get all the available cpuid levels from avail_levels, he/she can > > set one available cpuid level to cur_level, then he/she can get cpuid from > > cur_cpuid, cur_cpuid corresponds to cur_level. > > > > This patch uses sysfs to avoid limitless loop and provide more flexible > > interface for cpuid, please consider to merge to -mm tree in order to test. > > This is broken. > > Triple broken. > > It's broken, because it doesn't take into account the fact that Intel > broke CPUID level 4 and made it "repeating" (neither did the cpuid char > device, because it predated the Intel braindamage; I've had a patch for > it privately for a while, but didn't push it upstream because paravirt > broke it royally and I wanted the situation to settle down.) > > It's broken, because the algorithm used to determine valid CPUID levels > is incorrect; it fails to recognize any CPUID levels other than the main > Intel and AMD ones, e.g. the Transmeta 0x8086 (and sometimes more) > and VIA 0xc000 levels. Thank you for pointing out these issues, i think we can let users input any cpuid level and output the corresponding cpuid, in this way we can avoid to consider cpu differences and left this to userspace. We can also consider all the x86 platforms to do cpuid for every one. > > It's broken, because it is better for the userspace extractor to have > this logic than to stuff it into the kernel, where it sits hogging > unswappable memory at all times. It seems not to be very appropriate to let user space consider hardware details. /proc/cpuinfo should be an example to justify this. Is there any user application using /dev/cpu/*/cpuid? if no, i think it is feasible to provide an interface in the kernel. I noticed an application cpu-z on Windows, maybe we can clone it on Linux. > > -hpa -- 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/
[PATCH 2.6.24] x86: add sysfs interface for cpuid module
Current cpuid module will create a char device for every logical cpu,
when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop,
the root cause is that cpuid module doesn't decide wether a cpuid level
is valid, it just uses an offset to denote cpuid level and take it to
cpuid instruction, cpuid instruction will ignore it and return some data
specific to cpu model, cpuid doesn't an error return value because it is
void type. So cpuid module will execute cpuid continuously and return
data although most of data make no sense.
This patch tries to add a sysfs interface for cpuid, users can see all the
available cpuid levels, specify a specific level and get cpuid corresponding
to this cpuid level.
For every logical cpu, this patch will create a cpuid directory under
/sys/devices/system/cpu/cpu*/, there are three entries under cpuid:
avail_levelscur_level cur_cpuid
A user can get all the available cpuid levels from avail_levels, he/she can
set one available cpuid level to cur_level, then he/she can get cpuid from
cur_cpuid, cur_cpuid corresponds to cur_level.
This patch uses sysfs to avoid limitless loop and provide more flexible
interface for cpuid, please consider to merge to -mm tree in order to test.
Here are some example output:
[EMAIL PROTECTED] /]# ls /sys/devices/system/cpu/cpu0/cpuid
avail_levels cur_cpuid cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/avail_levels
0x
0x0001
0x0002
0x0003
0x0004
0x0005
0x0006
0x0007
0x0008
0x0009
0x000a
0x8000
0x8001
0x8002
0x8003
0x8004
0x8005
0x8006
0x8007
0x8008
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_level
0x
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x000a 0x756e6547 0x6c65746e 0x49656e69
[EMAIL PROTECTED] /]# echo 0x8007 >
/sys/devices/system/cpu/cpu0/cpuid/cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x 0x 0x 0x
[EMAIL PROTECTED] /]# echo 0x8001 >
/sys/devices/system/cpu/cpu0/cpuid/cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x 0x 0x0001 0x2010
[EMAIL PROTECTED] /]# ls /sys/devices/system/cpu/cpu1/cpuid
avail_levels cur_cpuid cur_level
[EMAIL PROTECTED] /]#
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
cpuid.c | 233 +++-
1 file changed, 232 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpuid.c 2008-01-28 01:28:48.0 +0800
+++ b/arch/x86/kernel/cpuid.c 2008-01-29 06:27:15.0 +0800
@@ -175,6 +175,228 @@ static struct notifier_block __cpuinitda
.notifier_call = cpuid_class_cpu_callback,
};
+struct cpuid_info {
+ int cpu;
+ unsigned int cur_level;
+ unsigned int min_level;
+ unsigned int max_level;
+ unsigned int min_ext_level;
+ unsigned int max_ext_level;
+ struct kobject kobj;
+};
+
+struct cpuid_attr {
+ struct attribute attr;
+ ssize_t (*show)(struct cpuid_info *, char *);
+ ssize_t (*store)(struct cpuid_info *, const char *, size_t count);
+};
+
+static struct cpuid_info cpuid_infos[NR_CPUS];
+
+static cpumask_t cpu_map = CPU_MASK_NONE;
+
+static ssize_t show_cur_level(struct cpuid_info *cpuid_info_p, char *buf)
+{
+ return sprintf(buf, "0x%08x\n", cpuid_info_p->cur_level);
+}
+
+static ssize_t store_cur_level(struct cpuid_info *cpuid_info_p,
+ const char *buf, size_t count)
+{
+ char *p;
+ unsigned int val;
+ size_t len = 13;
+ char tmpbuf[16];
+
+ if ((count > len) || (count <= 0))
+ return -EINVAL;
+
+ len = count;
+ if (buf[count-1] == '\n')
+len--;
+
+ memcpy(tmpbuf, buf, len);
+ tmpbuf[len] = '\0';
+ val = simple_strtoul(tmpbuf, , 0);
+
+ if (*p != '\0')
+ return -EINVAL;
+ if (((val >= cpuid_info_p->min_level)
+ && (val <= cpuid_info_p->max_level))
+ || ((val >= cpuid_info_p->min_ext_level)
+ && (val <= cpuid_info_p->max_ext_level))) {
+ cpuid_info_p->cur_level = val;
+ return count;
+ } else
+ return -EINVAL;
+}
+
+static ssize_t show_avail_levels(struct cpuid_info *cpuid_info_p, char *buf)
+{
+ unsigned int level;
+ ssize_t len = 0;
+
+ level = cpuid_info_p->min_level;
+ while (level <= cpuid_info_p->max_level) {
+ len += sprintf(buf + len, "0x%08x\n", level);
+ level++;
+ }
+
+ level = cpuid_info_p->min_ext_level;
+ while (level <= cpuid_info_p->max_ext_level) {
+ len += sprintf(buf + len, "0x%08x\n", level);
+
[PATCH 2.6.24] x86: add sysfs interface for cpuid module
Current cpuid module will create a char device for every logical cpu,
when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop,
the root cause is that cpuid module doesn't decide wether a cpuid level
is valid, it just uses an offset to denote cpuid level and take it to
cpuid instruction, cpuid instruction will ignore it and return some data
specific to cpu model, cpuid doesn't an error return value because it is
void type. So cpuid module will execute cpuid continuously and return
data although most of data make no sense.
This patch tries to add a sysfs interface for cpuid, users can see all the
available cpuid levels, specify a specific level and get cpuid corresponding
to this cpuid level.
For every logical cpu, this patch will create a cpuid directory under
/sys/devices/system/cpu/cpu*/, there are three entries under cpuid:
avail_levelscur_level cur_cpuid
A user can get all the available cpuid levels from avail_levels, he/she can
set one available cpuid level to cur_level, then he/she can get cpuid from
cur_cpuid, cur_cpuid corresponds to cur_level.
This patch uses sysfs to avoid limitless loop and provide more flexible
interface for cpuid, please consider to merge to -mm tree in order to test.
Here are some example output:
[EMAIL PROTECTED] /]# ls /sys/devices/system/cpu/cpu0/cpuid
avail_levels cur_cpuid cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/avail_levels
0x
0x0001
0x0002
0x0003
0x0004
0x0005
0x0006
0x0007
0x0008
0x0009
0x000a
0x8000
0x8001
0x8002
0x8003
0x8004
0x8005
0x8006
0x8007
0x8008
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_level
0x
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x000a 0x756e6547 0x6c65746e 0x49656e69
[EMAIL PROTECTED] /]# echo 0x8007
/sys/devices/system/cpu/cpu0/cpuid/cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x 0x 0x 0x
[EMAIL PROTECTED] /]# echo 0x8001
/sys/devices/system/cpu/cpu0/cpuid/cur_level
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpuid/cur_cpuid
0x 0x 0x0001 0x2010
[EMAIL PROTECTED] /]# ls /sys/devices/system/cpu/cpu1/cpuid
avail_levels cur_cpuid cur_level
[EMAIL PROTECTED] /]#
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
cpuid.c | 233 +++-
1 file changed, 232 insertions(+), 1 deletion(-)
--- a/arch/x86/kernel/cpuid.c 2008-01-28 01:28:48.0 +0800
+++ b/arch/x86/kernel/cpuid.c 2008-01-29 06:27:15.0 +0800
@@ -175,6 +175,228 @@ static struct notifier_block __cpuinitda
.notifier_call = cpuid_class_cpu_callback,
};
+struct cpuid_info {
+ int cpu;
+ unsigned int cur_level;
+ unsigned int min_level;
+ unsigned int max_level;
+ unsigned int min_ext_level;
+ unsigned int max_ext_level;
+ struct kobject kobj;
+};
+
+struct cpuid_attr {
+ struct attribute attr;
+ ssize_t (*show)(struct cpuid_info *, char *);
+ ssize_t (*store)(struct cpuid_info *, const char *, size_t count);
+};
+
+static struct cpuid_info cpuid_infos[NR_CPUS];
+
+static cpumask_t cpu_map = CPU_MASK_NONE;
+
+static ssize_t show_cur_level(struct cpuid_info *cpuid_info_p, char *buf)
+{
+ return sprintf(buf, 0x%08x\n, cpuid_info_p-cur_level);
+}
+
+static ssize_t store_cur_level(struct cpuid_info *cpuid_info_p,
+ const char *buf, size_t count)
+{
+ char *p;
+ unsigned int val;
+ size_t len = 13;
+ char tmpbuf[16];
+
+ if ((count len) || (count = 0))
+ return -EINVAL;
+
+ len = count;
+ if (buf[count-1] == '\n')
+len--;
+
+ memcpy(tmpbuf, buf, len);
+ tmpbuf[len] = '\0';
+ val = simple_strtoul(tmpbuf, p, 0);
+
+ if (*p != '\0')
+ return -EINVAL;
+ if (((val = cpuid_info_p-min_level)
+(val = cpuid_info_p-max_level))
+ || ((val = cpuid_info_p-min_ext_level)
+(val = cpuid_info_p-max_ext_level))) {
+ cpuid_info_p-cur_level = val;
+ return count;
+ } else
+ return -EINVAL;
+}
+
+static ssize_t show_avail_levels(struct cpuid_info *cpuid_info_p, char *buf)
+{
+ unsigned int level;
+ ssize_t len = 0;
+
+ level = cpuid_info_p-min_level;
+ while (level = cpuid_info_p-max_level) {
+ len += sprintf(buf + len, 0x%08x\n, level);
+ level++;
+ }
+
+ level = cpuid_info_p-min_ext_level;
+ while (level = cpuid_info_p-max_ext_level) {
+ len += sprintf(buf + len, 0x%08x\n, level);
+ level++;
+ }
+ return len;
+}
+
+static ssize_t show_cur_cpuid(struct cpuid_info *cpuid_info_p, char *buf)
+{
+ u32 data[4
Re: [PATCH 2.6.24] x86: add sysfs interface for cpuid module
On Tue, 2008-01-29 at 07:51 -0800, H. Peter Anvin wrote: Yi Yang wrote: Current cpuid module will create a char device for every logical cpu, when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop, the root cause is that cpuid module doesn't decide wether a cpuid level is valid, it just uses an offset to denote cpuid level and take it to cpuid instruction, cpuid instruction will ignore it and return some data specific to cpu model, cpuid doesn't an error return value because it is void type. So cpuid module will execute cpuid continuously and return data although most of data make no sense. This patch tries to add a sysfs interface for cpuid, users can see all the available cpuid levels, specify a specific level and get cpuid corresponding to this cpuid level. For every logical cpu, this patch will create a cpuid directory under /sys/devices/system/cpu/cpu*/, there are three entries under cpuid: avail_levelscur_level cur_cpuid A user can get all the available cpuid levels from avail_levels, he/she can set one available cpuid level to cur_level, then he/she can get cpuid from cur_cpuid, cur_cpuid corresponds to cur_level. This patch uses sysfs to avoid limitless loop and provide more flexible interface for cpuid, please consider to merge to -mm tree in order to test. This is broken. Triple broken. It's broken, because it doesn't take into account the fact that Intel broke CPUID level 4 and made it repeating (neither did the cpuid char device, because it predated the Intel braindamage; I've had a patch for it privately for a while, but didn't push it upstream because paravirt broke it royally and I wanted the situation to settle down.) It's broken, because the algorithm used to determine valid CPUID levels is incorrect; it fails to recognize any CPUID levels other than the main Intel and AMD ones, e.g. the Transmeta 0x8086 (and sometimes more) and VIA 0xc000 levels. Thank you for pointing out these issues, i think we can let users input any cpuid level and output the corresponding cpuid, in this way we can avoid to consider cpu differences and left this to userspace. We can also consider all the x86 platforms to do cpuid for every one. It's broken, because it is better for the userspace extractor to have this logic than to stuff it into the kernel, where it sits hogging unswappable memory at all times. It seems not to be very appropriate to let user space consider hardware details. /proc/cpuinfo should be an example to justify this. Is there any user application using /dev/cpu/*/cpuid? if no, i think it is feasible to provide an interface in the kernel. I noticed an application cpu-z on Windows, maybe we can clone it on Linux. -hpa -- 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: [PATCH 2.6.24] x86: add sysfs interface for cpuid module
On Tue, 2008-01-29 at 23:17 -0800, H. Peter Anvin wrote:
Yi Yang wrote:
It's broken, because it doesn't take into account the fact that Intel
broke CPUID level 4 and made it repeating (neither did the cpuid char
device, because it predated the Intel braindamage; I've had a patch for
it privately for a while, but didn't push it upstream because paravirt
broke it royally and I wanted the situation to settle down.)
level 4 doesn't result in repeating on Intel CPU, cpuid module sets
file offset to level, so cat /dev/cpu/*/cpuid will run cpuid instruction
continuously.
The issue is that Intel suddenly made CPUID ECX-sensitive, which there
is no way to represent.
Function cpuid has reset ecx to 0 immediate before calling to __cpuid,
so this shouldn't be a problem now.
in include/asm-x86/processor_32.h
/*
* Generic CPUID function
* clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx
* resulting in stale register contents being returned.
*/
static inline void cpuid(unsigned int op,
unsigned int *eax, unsigned int *ebx,
unsigned int *ecx, unsigned int *edx)
{
*eax = op;
*ecx = 0;
__cpuid(eax, ebx, ecx, edx);
}
As far as cat /dev/cpu/*/cpuid, that's a user error.
-hpa
--
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: [PATCH 2.6.24] x86: add sysfs interface for cpuid module
On Tue, 2008-01-29 at 07:51 -0800, H. Peter Anvin wrote: Yi Yang wrote: Current cpuid module will create a char device for every logical cpu, when a user cats /dev/cpu/*/cpuid, he/she will enter a limitless loop, the root cause is that cpuid module doesn't decide wether a cpuid level is valid, it just uses an offset to denote cpuid level and take it to cpuid instruction, cpuid instruction will ignore it and return some data This patch uses sysfs to avoid limitless loop and provide more flexible interface for cpuid, please consider to merge to -mm tree in order to test. This is broken. Triple broken. It's broken, because it doesn't take into account the fact that Intel broke CPUID level 4 and made it repeating (neither did the cpuid char device, because it predated the Intel braindamage; I've had a patch for it privately for a while, but didn't push it upstream because paravirt broke it royally and I wanted the situation to settle down.) level 4 doesn't result in repeating on Intel CPU, cpuid module sets file offset to level, so cat /dev/cpu/*/cpuid will run cpuid instruction continuously. -- 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: [PATCH 2.6.24] x86: add sysfs interface for cpuid module
On Tue, 2008-01-29 at 09:44 +0100, Sam Ravnborg wrote:
+
+static struct notifier_block __cpuinitdata cpuid_sysfs_cpu_notifier = {
+ .notifier_call = cpuid_sysfs_cpu_callback,
+};
Data is annotated _cpuintidata
but
+
Data is annotated _cpuintidata
@@ -217,11 +445,14 @@ static void __exit cpuid_exit(void)
{
int cpu = 0;
- for_each_online_cpu(cpu)
+ for_each_online_cpu(cpu) {
cpuid_device_destroy(cpu);
+ remove_cpuid_sysfs(cpu);
+ }
class_destroy(cpuid_class);
unregister_chrdev(CPUID_MAJOR, cpu/cpuid);
unregister_hotcpu_notifier(cpuid_class_cpu_notifier);
+ unregister_hotcpu_notifier(cpuid_sysfs_cpu_notifier);
used in an __exit function.
You should have seen a Section mismatch warning for this.
The right fix is to annotate the cpuid_sysfs_cpu_notifier
with __initdata_refok (soon to be named __refdata)
Or even better to declare it const and use _refconst.
I think __cpuinitdata is different from __initdata, i have tested it
by insmod, rmmod, echo 0/1 /sys/devices/system/cpu/cpu1/online
repeatly, it hasn't any issue.
Sam
--
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/
[PATCH] ACPI: create proc entry 'power' only if C2 or C3 is supported
Subject: ACPI: Create proc entry 'power' only C2 or C3 is supported
From: Yi Yang <[EMAIL PROTECTED]>
ACPI processor idle driver makes sense only if the processor supports
C2 or C3. For legacy C0 and C1, just the original pm_idle is working
, statistics info about promotion, demotion, latency, usage and
duration are empty or 0, so these are misleading, users'll think their
CPUs support C states (C2 or C3), /proc/acpi/processor/CPU*/power
shouldn't exist for this case at all.
This patch fixes this issue, it makes ACPI processor idle driver to create
proc entry only if the processor supports C2 or C3.
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
processor_idle.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
--- a/drivers/acpi/processor_idle.c 2008-01-24 05:34:29.0 +0800
+++ b/drivers/acpi/processor_idle.c 2008-01-24 07:04:59.0 +0800
@@ -1738,17 +1738,17 @@ int __cpuinit acpi_processor_power_init(
pm_idle = acpi_processor_idle;
}
#endif
- }
- /* 'power' [R] */
- entry = create_proc_entry(ACPI_PROCESSOR_FILE_POWER,
- S_IRUGO, acpi_device_dir(device));
- if (!entry)
- return -EIO;
- else {
- entry->proc_fops = _processor_power_fops;
- entry->data = acpi_driver_data(device);
- entry->owner = THIS_MODULE;
+ /* 'power' [R] */
+ entry = create_proc_entry(ACPI_PROCESSOR_FILE_POWER,
+ S_IRUGO, acpi_device_dir(device));
+ if (!entry)
+ return -EIO;
+ else {
+ entry->proc_fops = _processor_power_fops;
+ entry->data = acpi_driver_data(device);
+ entry->owner = THIS_MODULE;
+ }
}
return 0;
@@ -1763,7 +1763,8 @@ int acpi_processor_power_exit(struct acp
#endif
pr->flags.power_setup_done = 0;
- if (acpi_device_dir(device))
+ if (acpi_device_dir(device) &&
+ ((pr->flags.power) && (!boot_option_idle_override)))
remove_proc_entry(ACPI_PROCESSOR_FILE_POWER,
acpi_device_dir(device));
--
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/
[PATCH] ACPI: create proc entry 'power' only if C2 or C3 is supported
Subject: ACPI: Create proc entry 'power' only C2 or C3 is supported
From: Yi Yang [EMAIL PROTECTED]
ACPI processor idle driver makes sense only if the processor supports
C2 or C3. For legacy C0 and C1, just the original pm_idle is working
, statistics info about promotion, demotion, latency, usage and
duration are empty or 0, so these are misleading, users'll think their
CPUs support C states (C2 or C3), /proc/acpi/processor/CPU*/power
shouldn't exist for this case at all.
This patch fixes this issue, it makes ACPI processor idle driver to create
proc entry only if the processor supports C2 or C3.
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
processor_idle.c | 23 ---
1 file changed, 12 insertions(+), 11 deletions(-)
--- a/drivers/acpi/processor_idle.c 2008-01-24 05:34:29.0 +0800
+++ b/drivers/acpi/processor_idle.c 2008-01-24 07:04:59.0 +0800
@@ -1738,17 +1738,17 @@ int __cpuinit acpi_processor_power_init(
pm_idle = acpi_processor_idle;
}
#endif
- }
- /* 'power' [R] */
- entry = create_proc_entry(ACPI_PROCESSOR_FILE_POWER,
- S_IRUGO, acpi_device_dir(device));
- if (!entry)
- return -EIO;
- else {
- entry-proc_fops = acpi_processor_power_fops;
- entry-data = acpi_driver_data(device);
- entry-owner = THIS_MODULE;
+ /* 'power' [R] */
+ entry = create_proc_entry(ACPI_PROCESSOR_FILE_POWER,
+ S_IRUGO, acpi_device_dir(device));
+ if (!entry)
+ return -EIO;
+ else {
+ entry-proc_fops = acpi_processor_power_fops;
+ entry-data = acpi_driver_data(device);
+ entry-owner = THIS_MODULE;
+ }
}
return 0;
@@ -1763,7 +1763,8 @@ int acpi_processor_power_exit(struct acp
#endif
pr-flags.power_setup_done = 0;
- if (acpi_device_dir(device))
+ if (acpi_device_dir(device)
+ ((pr-flags.power) (!boot_option_idle_override)))
remove_proc_entry(ACPI_PROCESSOR_FILE_POWER,
acpi_device_dir(device));
--
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/
[PATCH 2.6.24-rc8] cpufreq: fix obvious condition statement error
The function __cpufreq_set_policy in file drivers/cpufreq/cpufreq.c
has a very obvious error:
if (policy->min > data->min && policy->min > policy->max) {
ret = -EINVAL;
goto error_out;
}
This condtion statement is wrong because it returns -EINVAL only if
policy->min is greater than policy->max (in this case,
"policy->min > data->min" is true for ever.). In fact, it should
return -EINVAL as well if policy->max is less than data->min.
The correct condition should be:
if (policy->min > data->max || policy->max < data->min) {
The following test result testifies the above conclusion:
Before applying this patch:
[EMAIL PROTECTED] /]# cat
/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies
2394000 1596000
[EMAIL PROTECTED] /]# echo 1596000 >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
1596000
[EMAIL PROTECTED] /]# echo "200" >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
1596000
[EMAIL PROTECTED] /]# echo "0" >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]# echo "1595000" >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]#
After applying this patch:
[EMAIL PROTECTED] /]# cat
/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies
2394000 1596000
[EMAIL PROTECTED] /]# echo 1596000 >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
1596000
[EMAIL PROTECTED] /]# echo "200" >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
1596000
[EMAIL PROTECTED] /]# echo "0" >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo "1595000" >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]# echo "1596000" >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# echo "2394000" >
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
2394000
[EMAIL PROTECTED] /]
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
cpufreq.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/cpufreq/cpufreq.c 2008-01-23 05:02:28.0 +0800
+++ b/drivers/cpufreq/cpufreq.c 2008-01-23 06:11:21.0 +0800
@@ -1608,7 +1608,7 @@ static int __cpufreq_set_policy(struct c
memcpy(>cpuinfo, >cpuinfo,
sizeof(struct cpufreq_cpuinfo));
- if (policy->min > data->min && policy->min > policy->max) {
+ if (policy->min > data->max || policy->max < data->min) {
ret = -EINVAL;
goto error_out;
}
--
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/
[PATCH 2.6.24-rc8] cpufreq: fix obvious condition statement error
The function __cpufreq_set_policy in file drivers/cpufreq/cpufreq.c
has a very obvious error:
if (policy-min data-min policy-min policy-max) {
ret = -EINVAL;
goto error_out;
}
This condtion statement is wrong because it returns -EINVAL only if
policy-min is greater than policy-max (in this case,
policy-min data-min is true for ever.). In fact, it should
return -EINVAL as well if policy-max is less than data-min.
The correct condition should be:
if (policy-min data-max || policy-max data-min) {
The following test result testifies the above conclusion:
Before applying this patch:
[EMAIL PROTECTED] /]# cat
/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies
2394000 1596000
[EMAIL PROTECTED] /]# echo 1596000
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
1596000
[EMAIL PROTECTED] /]# echo 200
/sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
1596000
[EMAIL PROTECTED] /]# echo 0
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]# echo 1595000
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]#
After applying this patch:
[EMAIL PROTECTED] /]# cat
/sys/devices/system/cpu/cpu0/cpufreq/scaling_available_frequencies
2394000 1596000
[EMAIL PROTECTED] /]# echo 1596000
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
1596000
[EMAIL PROTECTED] /]# echo 200
/sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_min_freq
1596000
[EMAIL PROTECTED] /]# echo 0
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# echo 1595000
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
1596000
[EMAIL PROTECTED] /]# echo 1596000
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# echo 2394000
/sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
[EMAIL PROTECTED] /]# cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_max_freq
2394000
[EMAIL PROTECTED] /]
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
cpufreq.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/cpufreq/cpufreq.c 2008-01-23 05:02:28.0 +0800
+++ b/drivers/cpufreq/cpufreq.c 2008-01-23 06:11:21.0 +0800
@@ -1608,7 +1608,7 @@ static int __cpufreq_set_policy(struct c
memcpy(policy-cpuinfo, data-cpuinfo,
sizeof(struct cpufreq_cpuinfo));
- if (policy-min data-min policy-min policy-max) {
+ if (policy-min data-max || policy-max data-min) {
ret = -EINVAL;
goto error_out;
}
--
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: [PATCH] ACPI: Add sysfs interface for acpi device wakeup
> > I think that it would be much much better to place wake-up attributes under > > corresponding PCI and PNP devices. > > Probably it is even better to link this code to PCI code, so PCI drivers > > will be aware of ACPI. > I like this idea, maxim. :) > And that's what we actually did about half a year ago. > > Yi, > Please refer to http://bugzilla.kernel.org/show_bug.cgi?id=6892 > and David's patch set here: > http://marc.info/?l=linux-mm-commits=117701595209299=2 > http://marc.info/?l=linux-mm-commits=117701866524935=2 > You can have a look at this thread as well: > http://marc.info/?l=linux-acpi=119982937409968=2 > I checked those patches you mentioned, they did bind two wakeup flag to some extent, but they can't be exchanged to use and they are just partly in current linus tree, two flags bind isn't in linus tree. According to my test on the latest linus tree, wakeup flag of acpi_device hasn't any association with device's, i don't know if they are the same thing. if we enbale or disable it manually, what will happen? From source code, it is just a flag, it doesn't trigger any event or hardware operation. > thanks, > Rui > > For example it will fix the 'EHCI instantly wakes up the system from S4' on > > my system, since here USB doesn't wake > > up anything from S4, and ACPI tables correctly show that. > > > > If ehci driver was aware of that it could disable #PME on entrance to S4. > > And we even can reuse its 'wakeup' attribute, thus enabling wakeup > > automatically. > > > > Going ever further, I think that it will be great to get rid of ACPI device > > tree, since > > most acpi devices are ether PCI of PNP ones. > > > > Or, even better have a small ACPI tree, that will contain 'true' ACPI > > devices, like cpus > > thermal sensors, buttons, etc. > > > > -- 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: [PATCH] ACPI: Add sysfs interface for acpi device wakeup
I think that it would be much much better to place wake-up attributes under corresponding PCI and PNP devices. Probably it is even better to link this code to PCI code, so PCI drivers will be aware of ACPI. I like this idea, maxim. :) And that's what we actually did about half a year ago. Yi, Please refer to http://bugzilla.kernel.org/show_bug.cgi?id=6892 and David's patch set here: http://marc.info/?l=linux-mm-commitsm=117701595209299w=2 http://marc.info/?l=linux-mm-commitsm=117701866524935w=2 You can have a look at this thread as well: http://marc.info/?l=linux-acpim=119982937409968w=2 I checked those patches you mentioned, they did bind two wakeup flag to some extent, but they can't be exchanged to use and they are just partly in current linus tree, two flags bind isn't in linus tree. According to my test on the latest linus tree, wakeup flag of acpi_device hasn't any association with device's, i don't know if they are the same thing. if we enbale or disable it manually, what will happen? From source code, it is just a flag, it doesn't trigger any event or hardware operation. thanks, Rui For example it will fix the 'EHCI instantly wakes up the system from S4' on my system, since here USB doesn't wake up anything from S4, and ACPI tables correctly show that. If ehci driver was aware of that it could disable #PME on entrance to S4. And we even can reuse its 'wakeup' attribute, thus enabling wakeup automatically. Going ever further, I think that it will be great to get rid of ACPI device tree, since most acpi devices are ether PCI of PNP ones. Or, even better have a small ACPI tree, that will contain 'true' ACPI devices, like cpus thermal sensors, buttons, etc. -- 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: [PATCH] ACPI: Add sysfs interface for acpi device wakeup
On Thu, 2008-01-10 at 09:43 +0200, Maxim Levitsky wrote: > On Thursday, 10 January 2008 00:21:46 Yi Yang wrote: > > Subject: ACPI: convert procfs to sysfs for /proc/acpi/wakeup > > From: Yi Yang <[EMAIL PROTECTED]> > > > > /proc/acpi/wakeup is deprecated but it has to exist because > > we haven't a sysfs interface to replace it yet, this patch > > converts /proc/acpi/wakeup to sysfs interface, under every > > acpi device sysfs node, a user can see a directory "wakeup" > > if the acpi device can support wakeup, there are six files > > under this directory: > > > > acpi_bus_id bus_id pci_id run_wakeup sleep_state status > > > > All the files are read-only exclude "status" which is used > > to enable or disable wakeup of the acpi device. > > > > "acpi_bus_id" is acpi bus ID of the acpi device. > > > > "bus_id" is pci bus id of the device associated to the acpi > > device, it will be empty if there isn't any device associated > > to it. > > > > "pci_id" is PCI ID of the pci device associated to the acpi > > device, it will be empty if there isn't any device associated > > to it. > > > > "run_wake" is a flag indicating if a wakeup process is being > > handled. > > > > "sleep_state" is sleep state of the acpi device such as "S0". > > > > "status" is wakeup status of the acpi device, it is enabled > > or disabled, a user can change it be echoing "0", "1", > > "disabled" or "enabled" to /sys/devices/.../wakeup/status. > > > > Here is the test result: > > > > [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup > > Device S-state Status Sysfs node PCI ID > > C093 S5 disabled pci::00:1e.0 0x2448 > > C0E8 S3 disabled pci::00:1d.0 0x27c8 > > C0EF S3 disabled pci::00:1d.1 0x27c9 > > C0F0 S3 disabled pci::00:1d.2 0x27ca > > C0F1 S3 disabled pci::00:1d.3 0x27cb > > C0F2 S3 disabled pci::00:1d.7 0x27cc > > C0F9 S0 disabled pci::00:1c.0 0x27d0 > > C21D S0 disabled pci::08:00.0 0x16fd > > C109 S5 disabled pci::00:1c.1 0x27d2 > > C228 S5 disabled pci::10:00.0 0x4222 > > C10F S5 disabled pci::00:1c.3 0x27d6 > > C229 S5 disabled > > [EMAIL PROTECTED] ~]# find /sys -name "*" | grep sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:05/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:0d/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:11/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:15/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:1d/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/device:2c/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/device:2e/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/wakeup/sleep_state > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state > > [EMAIL PROTECTED] ~]# ls > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup > > acpi_bus_id bus_id pci_id run_wakeup sleep_state status > > [EMAIL PROTECTED] ~]# cat > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id > > cat: > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id: > > No such file or directory > > [EMAIL PROTECTED] ~]# cat > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/acpi_bus_id > > C229 > > [EMAIL PROTECTED] ~]# cat > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state > > S5 > > [EMAIL PROTECTED] ~]# cat > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status > > disabled > > [EMAIL PROTECTED] ~]# cat > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/bus_id > > > > [EMAIL PROTECTED] ~]# cat > > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/p
Re: [PATCH] ACPI: Add sysfs interface for acpi device wakeup
On Thu, 2008-01-10 at 09:43 +0200, Maxim Levitsky wrote: On Thursday, 10 January 2008 00:21:46 Yi Yang wrote: Subject: ACPI: convert procfs to sysfs for /proc/acpi/wakeup From: Yi Yang [EMAIL PROTECTED] /proc/acpi/wakeup is deprecated but it has to exist because we haven't a sysfs interface to replace it yet, this patch converts /proc/acpi/wakeup to sysfs interface, under every acpi device sysfs node, a user can see a directory wakeup if the acpi device can support wakeup, there are six files under this directory: acpi_bus_id bus_id pci_id run_wakeup sleep_state status All the files are read-only exclude status which is used to enable or disable wakeup of the acpi device. acpi_bus_id is acpi bus ID of the acpi device. bus_id is pci bus id of the device associated to the acpi device, it will be empty if there isn't any device associated to it. pci_id is PCI ID of the pci device associated to the acpi device, it will be empty if there isn't any device associated to it. run_wake is a flag indicating if a wakeup process is being handled. sleep_state is sleep state of the acpi device such as S0. status is wakeup status of the acpi device, it is enabled or disabled, a user can change it be echoing 0, 1, disabled or enabled to /sys/devices/.../wakeup/status. Here is the test result: [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 disabled pci::00:1c.0 0x27d0 C21D S0 disabled pci::08:00.0 0x16fd C109 S5 disabled pci::00:1c.1 0x27d2 C228 S5 disabled pci::10:00.0 0x4222 C10F S5 disabled pci::00:1c.3 0x27d6 C229 S5 disabled [EMAIL PROTECTED] ~]# find /sys -name * | grep sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:05/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:0d/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:11/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:15/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:1d/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/device:2c/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/device:2e/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state [EMAIL PROTECTED] ~]# ls /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup acpi_bus_id bus_id pci_id run_wakeup sleep_state status [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id cat: /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id: No such file or directory [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/acpi_bus_id C229 [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state S5 [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status disabled [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/bus_id [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/pci_id [EMAIL PROTECTED] ~]# echo 1 /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status enabled [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 enabled pci::00:1c.0 0x27d0 C21D S0 enabled pci::08:00.0 0x16fd C109 S5 enabled pci::00:1c.1 0x27d2
[PATCH] ACPI: Add sysfs interface for acpi device wakeup
Subject: ACPI: convert procfs to sysfs for /proc/acpi/wakeup From: Yi Yang <[EMAIL PROTECTED]> /proc/acpi/wakeup is deprecated but it has to exist because we haven't a sysfs interface to replace it yet, this patch converts /proc/acpi/wakeup to sysfs interface, under every acpi device sysfs node, a user can see a directory "wakeup" if the acpi device can support wakeup, there are six files under this directory: acpi_bus_id bus_id pci_id run_wakeup sleep_state status All the files are read-only exclude "status" which is used to enable or disable wakeup of the acpi device. "acpi_bus_id" is acpi bus ID of the acpi device. "bus_id" is pci bus id of the device associated to the acpi device, it will be empty if there isn't any device associated to it. "pci_id" is PCI ID of the pci device associated to the acpi device, it will be empty if there isn't any device associated to it. "run_wake" is a flag indicating if a wakeup process is being handled. "sleep_state" is sleep state of the acpi device such as "S0". "status" is wakeup status of the acpi device, it is enabled or disabled, a user can change it be echoing "0", "1", "disabled" or "enabled" to /sys/devices/.../wakeup/status. Here is the test result: [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 disabled pci::00:1c.0 0x27d0 C21D S0 disabled pci::08:00.0 0x16fd C109 S5 disabled pci::00:1c.1 0x27d2 C228 S5 disabled pci::10:00.0 0x4222 C10F S5 disabled pci::00:1c.3 0x27d6 C229 S5 disabled [EMAIL PROTECTED] ~]# find /sys -name "*" | grep sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:05/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:0d/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:11/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:15/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:1d/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/device:2c/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/device:2e/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state [EMAIL PROTECTED] ~]# ls /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup acpi_bus_id bus_id pci_id run_wakeup sleep_state status [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id cat: /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id: No such file or directory [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/acpi_bus_id C229 [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state S5 [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status disabled [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/bus_id [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/pci_id [EMAIL PROTECTED] ~]# echo 1 > /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status enabled [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 enabled pci::00:1c.0 0x27d0 C21D S0 enabled pci::08:00.0 0x16fd C109 S5 enabled pci::00:1c.1 0x27d2 C228 S5 enabled pci::10:00.0 0x4222 C10F S5 enabled pci::00:1c.3 0x27d6 C229 S5 e
[PATCH] ACPI: Add sysfs interface for acpi device wakeup
Subject: ACPI: convert procfs to sysfs for /proc/acpi/wakeup From: Yi Yang [EMAIL PROTECTED] /proc/acpi/wakeup is deprecated but it has to exist because we haven't a sysfs interface to replace it yet, this patch converts /proc/acpi/wakeup to sysfs interface, under every acpi device sysfs node, a user can see a directory wakeup if the acpi device can support wakeup, there are six files under this directory: acpi_bus_id bus_id pci_id run_wakeup sleep_state status All the files are read-only exclude status which is used to enable or disable wakeup of the acpi device. acpi_bus_id is acpi bus ID of the acpi device. bus_id is pci bus id of the device associated to the acpi device, it will be empty if there isn't any device associated to it. pci_id is PCI ID of the pci device associated to the acpi device, it will be empty if there isn't any device associated to it. run_wake is a flag indicating if a wakeup process is being handled. sleep_state is sleep state of the acpi device such as S0. status is wakeup status of the acpi device, it is enabled or disabled, a user can change it be echoing 0, 1, disabled or enabled to /sys/devices/.../wakeup/status. Here is the test result: [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 disabled pci::00:1c.0 0x27d0 C21D S0 disabled pci::08:00.0 0x16fd C109 S5 disabled pci::00:1c.1 0x27d2 C228 S5 disabled pci::10:00.0 0x4222 C10F S5 disabled pci::00:1c.3 0x27d6 C229 S5 disabled [EMAIL PROTECTED] ~]# find /sys -name * | grep sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:05/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:0d/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:11/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:15/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:19/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:1d/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2b/device:2c/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2d/device:2e/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/wakeup/sleep_state /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state [EMAIL PROTECTED] ~]# ls /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup acpi_bus_id bus_id pci_id run_wakeup sleep_state status [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id cat: /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/acpi_bus_id: No such file or directory [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/acpi_bus_id C229 [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/sleep_state S5 [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status disabled [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/bus_id [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/pci_id [EMAIL PROTECTED] ~]# echo 1 /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status [EMAIL PROTECTED] ~]# cat /sys/devices/LNXSYSTM:00/device:00/PNP0A08:00/device:2f/device:30/wakeup/status enabled [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 enabled pci::00:1c.0 0x27d0 C21D S0 enabled pci::08:00.0 0x16fd C109 S5 enabled pci::00:1c.1 0x27d2 C228 S5 enabled pci::10:00.0 0x4222 C10F S5 enabled pci::00:1c.3 0x27d6 C229 S5 enabled [EMAIL PROTECTED] ~]# vi /var/log/dmesg [EMAIL PROTECTED] ~]# dmesg | grep same GPE ACPI: 'C0F9' and 'C229' have the same GPE, can't disable/enable one seperately ACPI: 'C21D
[PATCH] ACPI: fix processor limit set error
Subject: ACPI: fix processor limit set error From: Yi Yang <[EMAIL PROTECTED]> when echo some invalid values to /proc/acpi/processor/CPU*/limit, it doesn't return any error info, on the contrary, it successes and sets some other values, for example: [EMAIL PROTECTED] /]# echo "0:0A" >/proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo "0:0F" >/proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo "0:0 0:1 0:2" >/proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# A correct way is that it should fail and return error info. This patch fixed this issue, it accepts not only such inputs as "*:*", but also accepts such inputs as "p*:t*" or "P*:T*" or "p*:*" or "*:t*", the former "*" in inputs means the allowed processor performance state, currently it is only a stub and not set to the processor limit data structure, the latter "*" means the allowed processor throttling state which rages from 0 to 7 generally. This patch strictly limits inputs to meet the above conditions, any input which can't meet the above conditions is considered as invalid input. Before applying this patch, the test result is below: [EMAIL PROTECTED] /]# echo "0:0A" >/proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo "0:0F" >/proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo "10:2F" >/proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T2 user limit: P0:T2 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo "0:0 0:1 0:2" >/proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# After applying this patch, the test result is below: [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo "0:0A" > /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo "0:0F" > /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo "P0:T1" > /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo "p0:t1" > /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo "p0:x1" > /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo "q0:x1" > /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo "p0:1" > /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# echo "q0:x1" > /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo "0:t1" > /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo "0:t1F" > /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo "" > /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo "0:0 0:0 0:2 " > /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo "P0:T1" > /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1
[PATCH] ACPI: fix processor limit set error
Subject: ACPI: fix processor limit set error From: Yi Yang [EMAIL PROTECTED] when echo some invalid values to /proc/acpi/processor/CPU*/limit, it doesn't return any error info, on the contrary, it successes and sets some other values, for example: [EMAIL PROTECTED] /]# echo 0:0A /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo 0:0F /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo 0:0 0:1 0:2 /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# A correct way is that it should fail and return error info. This patch fixed this issue, it accepts not only such inputs as *:*, but also accepts such inputs as p*:t* or P*:T* or p*:* or *:t*, the former * in inputs means the allowed processor performance state, currently it is only a stub and not set to the processor limit data structure, the latter * means the allowed processor throttling state which rages from 0 to 7 generally. This patch strictly limits inputs to meet the above conditions, any input which can't meet the above conditions is considered as invalid input. Before applying this patch, the test result is below: [EMAIL PROTECTED] /]# echo 0:0A /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo 0:0F /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo 10:2F /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T2 user limit: P0:T2 thermal limit: P0:T0 [EMAIL PROTECTED] /]# echo 0:0 0:1 0:2 /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] /]# After applying this patch, the test result is below: [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T0 user limit: P0:T0 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo 0:0A /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo 0:0F /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo P0:T1 /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo p0:t1 /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo p0:x1 /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo q0:x1 /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo p0:1 /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# echo q0:x1 /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo 0:t1 /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# echo 0:t1F /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo 0:0 0:0 0:2 /proc/acpi/processor/CPU0/limit -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo P0:T1 /proc/acpi/processor/CPU0/limit [EMAIL PROTECTED] ~]# cat /proc/acpi/processor/CPU0/limit active limit:P0:T1 user limit: P0:T1 thermal limit: P0:T0 [EMAIL PROTECTED] ~]# Signed-off-by: Yi Yang [EMAIL PROTECTED] --- processor_thermal.c | 42 +++--- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/drivers/acpi/processor_thermal.c b/drivers/acpi/processor_thermal.c index 06e6f3f..262d56e 100644 --- a/drivers/acpi/processor_thermal.c +++ b/drivers/acpi/processor_thermal.c
[PATCH] ACPI: fix processor throttling set error
Subject: ACPI: fix processor throttling set error From: Yi Yang <[EMAIL PROTECTED]> When echo some invalid values to /proc/acpi/processor/*/throttling, there isn't any error info returned, on the contray, it sets throttling value to some T* successfully, obviously, this is incorrect, a correct way should be to let it fail and return error info. This patch fixed the aforementioned issue, it also enables /proc/acpi/processor/*/throttling to accept such values as 't0' and 'T0', it also strictly limits /proc/acpi/processor/*/throttling only to accept "*", "t*" and "T*", "*" is the throttling state value the processor can support, current, it is 0 - 7. Before applying this patch, the test result is below: [EMAIL PROTECTED] acpi]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T1 state available: T0 to T7 states: T0: 100% *T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] acpi]# echo "1xx" > /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] acpi]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T1 state available: T0 to T7 states: T0: 100% *T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] acpi]# echo "0" > /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] acpi]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] acpi]# cd / [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo "T0" > /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo "T7" > /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo "T100" > /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo "xxx" > /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo "2" > /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T2 state available: T0 to T7 states: T0: 100% T1: 87% *T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo "" > /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/p
Re: [linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device
On Mon, 2008-01-07 at 02:57 +0100, Rafael J. Wysocki wrote: > On Monday, 7 of January 2008, Yi Yang wrote: > > On Fri, 2008-01-04 at 08:09 -0800, David Brownell wrote: > > > > > This patch changes empty output to "unsupported" in order that a user > > > > > knows > > > > > wakeup feature isn't supported by this device when he/she > > > > > 'cat /sys/devices/.../power/wakeup', please consider to apply, > > > > > thanks. > > > > > > > > What about simply removing "wakuep" file if wakeup is not supported? > > > > > > It may not *stay* unsupported, so I think changing it in either > > > of those permanent ways would be confusing/misleading. > > > > > > For example, USB devices have multiple states ... minimally, an > > > unconfigured state, and a configured state. Some have multiple > > > configurations. Only configured states can be wakeup-capable. > > > So a given device might have three states, but support wakeup > > > except in one of them... > > If so, we can change "unsupported" to "unconfigurable" or "inoperable" > > which can cover the states "unconfigured", "unsupported" and other > > unknown states. :-). > > The main problem with your patch is that is changes a documented behavior > visible by the user space. That should be done very cautiously. Which applications are using this interface? if they assume "\n" means the unconfigurable or unavailable state, this is very fragile. > > Thanks, > Rafael -- 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: [linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 18:20 +0100, Oliver Neukum wrote: > Am Freitag, 4. Januar 2008 17:52:14 schrieb Olivier Galibert: > > On Fri, Jan 04, 2008 at 11:38:29AM -0500, Alan Stern wrote: > > > How about changing it to say "unavailable"? That doesn't imply > > > permanence. > > > > How about not changing a userland-visible interface gratuitously? > Why not do if we can make it better? > Very good idea. > > Regards > Oliver -- 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: [linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 17:52 +0100, Olivier Galibert wrote: > On Fri, Jan 04, 2008 at 11:38:29AM -0500, Alan Stern wrote: > > How about changing it to say "unavailable"? That doesn't imply > > permanence. > > How about not changing a userland-visible interface gratuitously? "empty" can't tell a user the state of wakeup of a device, so it is necessary to change it. > > OG. > -- 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: [linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 11:38 -0500, Alan Stern wrote: > On Fri, 4 Jan 2008, David Brownell wrote: > > > > > This patch changes empty output to "unsupported" in order that a user > > > > knows > > > > wakeup feature isn't supported by this device when he/she > > > > 'cat /sys/devices/.../power/wakeup', please consider to apply, > > > > thanks. > > > > > > What about simply removing "wakuep" file if wakeup is not supported? > > > > It may not *stay* unsupported, so I think changing it in either > > of those permanent ways would be confusing/misleading. > > How about changing it to say "unavailable"? That doesn't imply > permanence. This should be an option. > > Alan Stern > -- 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: [linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 08:31 -0800, David Brownell wrote:
> > This patch changes empty output to "unsupported" in order that a user knows
> > wakeup feature isn't supported by this device when he/she
> > 'cat /sys/devices/.../power/wakeup', please consider to apply, thanks.
>
> I don't much like this patch. Not just for the technical reasons
> mentioned in my previous note ... but also because it doesn't update
> the documention at the top, which clearly states that "\n" is
> returned for "temporary or permanent inability to issue wakeup".
> And then it gives the example I gave before ...
>
> Though I suppose a patch that changes the *entire* userspace interface,
> (which includes its documentation, and all out-of-tree users) would have
> shown more clearly why it wasn't a good idea. ;)
Really, "\n" should be changed to show that change.
Anyway, "\n" isn't a good indicator for that state. :-)
>
>
> > --- a/drivers/base/power/sysfs.c2008-01-04 16:50:54.0 +0800
> > +++ b/drivers/base/power/sysfs.c2008-01-04 17:14:42.0 +0800
> > @@ -49,7 +49,7 @@ wake_show(struct device * dev, struct de
> > {
> > return sprintf(buf, "%s\n", device_can_wakeup(dev)
> > ? (device_may_wakeup(dev) ? enabled : disabled)
> > - : "");
> > + : "unsupported");
> > }
> >
> > static ssize_t
> >
> >
--
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: [linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 08:09 -0800, David Brownell wrote: > > > This patch changes empty output to "unsupported" in order that a user > > > knows > > > wakeup feature isn't supported by this device when he/she > > > 'cat /sys/devices/.../power/wakeup', please consider to apply, > > > thanks. > > > > What about simply removing "wakuep" file if wakeup is not supported? > > It may not *stay* unsupported, so I think changing it in either > of those permanent ways would be confusing/misleading. > > For example, USB devices have multiple states ... minimally, an > unconfigured state, and a configured state. Some have multiple > configurations. Only configured states can be wakeup-capable. > So a given device might have three states, but support wakeup > except in one of them... If so, we can change "unsupported" to "unconfigurable" or "inoperable" which can cover the states "unconfigured", "unsupported" and other unknown states. :-). > > - Dave > -- 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: [linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 12:48 +0100, Pavel Machek wrote: > Hi! > > > If a device can't support wakeup, its /sys/devices/.../power/wakeup output > > is > > empty, this is confusing, a user doesn't know if it supports wakeup feature > > unless he/she read the ralated source code, for this case, it is more > > reasonable to output "unsupported". Otherwise, no matter what value the user > > sets to /sys/devices/.../power/wakeup, the result is the same: Invalid > > argument, > > so the user doesn't know why. > > > > This patch changes empty output to "unsupported" in order that a user knows > > wakeup feature isn't supported by this device when he/she > > 'cat /sys/devices/.../power/wakeup', please consider to apply, > > thanks. > > What about simply removing "wakuep" file if wakeup is not supported? > Pavel At the beginning, i did that as you said, but it can't work, some power/wakeup will disappear although they can be disabled or enabled, so it is only one feasible way to make it exist permanently. > -- 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: [linux-pm][PATCH] base: Change power/wakeup output from to unsupported if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 12:48 +0100, Pavel Machek wrote: Hi! If a device can't support wakeup, its /sys/devices/.../power/wakeup output is empty, this is confusing, a user doesn't know if it supports wakeup feature unless he/she read the ralated source code, for this case, it is more reasonable to output unsupported. Otherwise, no matter what value the user sets to /sys/devices/.../power/wakeup, the result is the same: Invalid argument, so the user doesn't know why. This patch changes empty output to unsupported in order that a user knows wakeup feature isn't supported by this device when he/she 'cat /sys/devices/.../power/wakeup', please consider to apply, thanks. What about simply removing wakuep file if wakeup is not supported? Pavel At the beginning, i did that as you said, but it can't work, some power/wakeup will disappear although they can be disabled or enabled, so it is only one feasible way to make it exist permanently. -- 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: [linux-pm][PATCH] base: Change power/wakeup output from to unsupported if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 08:09 -0800, David Brownell wrote: This patch changes empty output to unsupported in order that a user knows wakeup feature isn't supported by this device when he/she 'cat /sys/devices/.../power/wakeup', please consider to apply, thanks. What about simply removing wakuep file if wakeup is not supported? It may not *stay* unsupported, so I think changing it in either of those permanent ways would be confusing/misleading. For example, USB devices have multiple states ... minimally, an unconfigured state, and a configured state. Some have multiple configurations. Only configured states can be wakeup-capable. So a given device might have three states, but support wakeup except in one of them... If so, we can change unsupported to unconfigurable or inoperable which can cover the states unconfigured, unsupported and other unknown states. :-). - Dave -- 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: [linux-pm][PATCH] base: Change power/wakeup output from to unsupported if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 08:31 -0800, David Brownell wrote:
This patch changes empty output to unsupported in order that a user knows
wakeup feature isn't supported by this device when he/she
'cat /sys/devices/.../power/wakeup', please consider to apply, thanks.
I don't much like this patch. Not just for the technical reasons
mentioned in my previous note ... but also because it doesn't update
the documention at the top, which clearly states that \n is
returned for temporary or permanent inability to issue wakeup.
And then it gives the example I gave before ...
Though I suppose a patch that changes the *entire* userspace interface,
(which includes its documentation, and all out-of-tree users) would have
shown more clearly why it wasn't a good idea. ;)
Really, \n should be changed to show that change.
Anyway, \n isn't a good indicator for that state. :-)
--- a/drivers/base/power/sysfs.c2008-01-04 16:50:54.0 +0800
+++ b/drivers/base/power/sysfs.c2008-01-04 17:14:42.0 +0800
@@ -49,7 +49,7 @@ wake_show(struct device * dev, struct de
{
return sprintf(buf, %s\n, device_can_wakeup(dev)
? (device_may_wakeup(dev) ? enabled : disabled)
- : );
+ : unsupported);
}
static ssize_t
--
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: [linux-pm][PATCH] base: Change power/wakeup output from to unsupported if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 11:38 -0500, Alan Stern wrote: On Fri, 4 Jan 2008, David Brownell wrote: This patch changes empty output to unsupported in order that a user knows wakeup feature isn't supported by this device when he/she 'cat /sys/devices/.../power/wakeup', please consider to apply, thanks. What about simply removing wakuep file if wakeup is not supported? It may not *stay* unsupported, so I think changing it in either of those permanent ways would be confusing/misleading. How about changing it to say unavailable? That doesn't imply permanence. This should be an option. Alan Stern -- 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: [linux-pm][PATCH] base: Change power/wakeup output from to unsupported if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 17:52 +0100, Olivier Galibert wrote: On Fri, Jan 04, 2008 at 11:38:29AM -0500, Alan Stern wrote: How about changing it to say unavailable? That doesn't imply permanence. How about not changing a userland-visible interface gratuitously? empty can't tell a user the state of wakeup of a device, so it is necessary to change it. OG. -- 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: [linux-pm][PATCH] base: Change power/wakeup output from to unsupported if wakeup feature isn't supported by a device
On Fri, 2008-01-04 at 18:20 +0100, Oliver Neukum wrote: Am Freitag, 4. Januar 2008 17:52:14 schrieb Olivier Galibert: On Fri, Jan 04, 2008 at 11:38:29AM -0500, Alan Stern wrote: How about changing it to say unavailable? That doesn't imply permanence. How about not changing a userland-visible interface gratuitously? Why not do if we can make it better? Very good idea. Regards Oliver -- 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: [linux-pm][PATCH] base: Change power/wakeup output from to unsupported if wakeup feature isn't supported by a device
On Mon, 2008-01-07 at 02:57 +0100, Rafael J. Wysocki wrote: On Monday, 7 of January 2008, Yi Yang wrote: On Fri, 2008-01-04 at 08:09 -0800, David Brownell wrote: This patch changes empty output to unsupported in order that a user knows wakeup feature isn't supported by this device when he/she 'cat /sys/devices/.../power/wakeup', please consider to apply, thanks. What about simply removing wakuep file if wakeup is not supported? It may not *stay* unsupported, so I think changing it in either of those permanent ways would be confusing/misleading. For example, USB devices have multiple states ... minimally, an unconfigured state, and a configured state. Some have multiple configurations. Only configured states can be wakeup-capable. So a given device might have three states, but support wakeup except in one of them... If so, we can change unsupported to unconfigurable or inoperable which can cover the states unconfigured, unsupported and other unknown states. :-). The main problem with your patch is that is changes a documented behavior visible by the user space. That should be done very cautiously. Which applications are using this interface? if they assume \n means the unconfigurable or unavailable state, this is very fragile. Thanks, Rafael -- 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/
[PATCH] ACPI: fix processor throttling set error
Subject: ACPI: fix processor throttling set error From: Yi Yang [EMAIL PROTECTED] When echo some invalid values to /proc/acpi/processor/*/throttling, there isn't any error info returned, on the contray, it sets throttling value to some T* successfully, obviously, this is incorrect, a correct way should be to let it fail and return error info. This patch fixed the aforementioned issue, it also enables /proc/acpi/processor/*/throttling to accept such values as 't0' and 'T0', it also strictly limits /proc/acpi/processor/*/throttling only to accept *, t* and T*, * is the throttling state value the processor can support, current, it is 0 - 7. Before applying this patch, the test result is below: [EMAIL PROTECTED] acpi]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T1 state available: T0 to T7 states: T0: 100% *T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] acpi]# echo 1xx /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] acpi]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T1 state available: T0 to T7 states: T0: 100% *T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] acpi]# echo 0 /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] acpi]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] acpi]# cd / [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo T0 /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo T7 /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo T100 /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo xxx /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo 2 /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T2 state available: T0 to T7 states: T0: 100% T1: 87% *T2: 75% T3: 62% T4: 50% T5: 37% T6: 25% T7: 12% [EMAIL PROTECTED] /]# echo /proc/acpi/processor/CPU0/throttling [EMAIL PROTECTED] /]# cat /proc/acpi/processor/CPU0/throttling state count: 8 active state:T0 state available: T0 to T7 states: *T0: 100% T1: 87% T2
[linux-pm][PATCH] base: Change power/wakeup output from "" to "unsupported" if wakeup feature isn't supported by a device
If a device can't support wakeup, its /sys/devices/.../power/wakeup output is
empty, this is confusing, a user doesn't know if it supports wakeup feature
unless he/she read the ralated source code, for this case, it is more
reasonable to output "unsupported". Otherwise, no matter what value the user
sets to /sys/devices/.../power/wakeup, the result is the same: Invalid
argument,
so the user doesn't know why.
This patch changes empty output to "unsupported" in order that a user knows
wakeup feature isn't supported by this device when he/she
'cat /sys/devices/.../power/wakeup', please consider to apply, thanks.
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
sysfs.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/base/power/sysfs.c2008-01-04 16:50:54.0 +0800
+++ b/drivers/base/power/sysfs.c2008-01-04 17:14:42.0 +0800
@@ -49,7 +49,7 @@ wake_show(struct device * dev, struct de
{
return sprintf(buf, "%s\n", device_can_wakeup(dev)
? (device_may_wakeup(dev) ? enabled : disabled)
- : "");
+ : "unsupported");
}
static ssize_t
--
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/
[PATCH linux-acpi] fix acpi fan state set error
Subject: ACPI: fix acpi fan state set error
From: Yi Yang <[EMAIL PROTECTED]>
Under /proc/acpi, there is a fan control interface, a user can
set 0 or 3 to /proc/acpi/fan/*/state, 0 denotes D0 state, 3
denotes D3 state, but in current implementation, a user can
set a fan to D1 state by any char excluding '1', '2' and '3'.
For example:
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo "" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] acpi]# echo "3" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo "x" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
Obviously, such inputs as "" and "x" are invalid for fan state.
This patch fixes this issue, it strictly limits fan state only to
accept 0, 1, 2 and 3, any other inputs are invalid.
Before applying this patch, the test result is:
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo "" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] acpi]# echo "3" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo "x" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] acpi]# echo "3" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo "3x" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo "-1x" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] acpi]#
After applying this patch, the test result is:
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo "" > /proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo "3" > /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo "x" > /proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo "-1x" > /proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo "0" > //proc/acpi/fan/C31B/state
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] ~]# echo "4" > //proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] ~]# echo "3" > //proc/acpi/fan/C31B/state
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo "0" > //proc/acpi/fan/C31B/state
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] ~]# echo "3x" > //proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]#
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
fan.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index a5a5532..53f7c72 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -98,7 +98,7 @@ acpi_fan_write_state(struct file *file, const char __user *
buffer,
int result = 0;
struct seq_file *m = file->private_data;
struct acpi_device *device = m->private;
- char state_string[12] = { '\0' };
+ char state_string[3] = { '\0' };
if (count > sizeof(state_string) - 1)
return -EINVAL;
@@ -107,6 +107,12 @@ acpi_fan_write_state(struct file *file, const char __user
* buffer,
return -EFAULT;
state_string[count] = '\0';
+ if ((state_string[0] < '0') || (state_string[0] > '3'))
+ return -EINVAL;
+ if (state_string[1] == '\n')
+ state_string[1] = '\0';
+ if (state_string[1] != '\0')
+ return -EINVAL;
result = acpi_bus_set_power(device-
[PATCH linux-acpi] fix acpi fan state set error
Subject: ACPI: fix acpi fan state set error
From: Yi Yang [EMAIL PROTECTED]
Under /proc/acpi, there is a fan control interface, a user can
set 0 or 3 to /proc/acpi/fan/*/state, 0 denotes D0 state, 3
denotes D3 state, but in current implementation, a user can
set a fan to D1 state by any char excluding '1', '2' and '3'.
For example:
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] acpi]# echo 3 /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo x /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
Obviously, such inputs as and x are invalid for fan state.
This patch fixes this issue, it strictly limits fan state only to
accept 0, 1, 2 and 3, any other inputs are invalid.
Before applying this patch, the test result is:
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] acpi]# echo 3 /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo x /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] acpi]# echo 3 /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo 3x /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] acpi]# echo -1x /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] acpi]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] acpi]#
After applying this patch, the test result is:
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo /proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo 3 /proc/acpi/fan/C31B/state
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo x /proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo -1x /proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo 0 //proc/acpi/fan/C31B/state
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] ~]# echo 4 //proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] ~]# echo 3 //proc/acpi/fan/C31B/state
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: off
[EMAIL PROTECTED] ~]# echo 0 //proc/acpi/fan/C31B/state
[EMAIL PROTECTED] ~]# cat /proc/acpi/fan/C31B/state
status: on
[EMAIL PROTECTED] ~]# echo 3x //proc/acpi/fan/C31B/state
-bash: echo: write error: Invalid argument
[EMAIL PROTECTED] ~]#
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
fan.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index a5a5532..53f7c72 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -98,7 +98,7 @@ acpi_fan_write_state(struct file *file, const char __user *
buffer,
int result = 0;
struct seq_file *m = file-private_data;
struct acpi_device *device = m-private;
- char state_string[12] = { '\0' };
+ char state_string[3] = { '\0' };
if (count sizeof(state_string) - 1)
return -EINVAL;
@@ -107,6 +107,12 @@ acpi_fan_write_state(struct file *file, const char __user
* buffer,
return -EFAULT;
state_string[count] = '\0';
+ if ((state_string[0] '0') || (state_string[0] '3'))
+ return -EINVAL;
+ if (state_string[1] == '\n')
+ state_string[1] = '\0';
+ if (state_string[1] != '\0')
+ return -EINVAL;
result = acpi_bus_set_power(device-handle,
simple_strtoul(state_string, NULL, 0));
--
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/
[linux-pm][PATCH] base: Change power/wakeup output from to unsupported if wakeup feature isn't supported by a device
If a device can't support wakeup, its /sys/devices/.../power/wakeup output is
empty, this is confusing, a user doesn't know if it supports wakeup feature
unless he/she read the ralated source code, for this case, it is more
reasonable to output unsupported. Otherwise, no matter what value the user
sets to /sys/devices/.../power/wakeup, the result is the same: Invalid
argument,
so the user doesn't know why.
This patch changes empty output to unsupported in order that a user knows
wakeup feature isn't supported by this device when he/she
'cat /sys/devices/.../power/wakeup', please consider to apply, thanks.
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
sysfs.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/drivers/base/power/sysfs.c2008-01-04 16:50:54.0 +0800
+++ b/drivers/base/power/sysfs.c2008-01-04 17:14:42.0 +0800
@@ -49,7 +49,7 @@ wake_show(struct device * dev, struct de
{
return sprintf(buf, %s\n, device_can_wakeup(dev)
? (device_may_wakeup(dev) ? enabled : disabled)
- : );
+ : unsupported);
}
static ssize_t
--
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: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID
On Wed, 2008-01-02 at 17:09 +0100, Pavel Machek wrote: > On Wed 2008-01-02 10:03:59, Yi Yang wrote: > > On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote: > > > Hi! > > > > > > > /proc/acpi/wakeup is also case-sensitive, case-insensitive is better. > > > > > > Why? > > A user uses device bus id like 'C093' to enable or disable wakeup of the > > device, for example > > > > echo "C093" > /proc/acpi/wakeup > > > > but i think "c093" should also be ok. i.e. > > Why do you think so? Unix is generally case-sensitive. I see ascii > text in .../wakeup. Maybe some bios vendor is crazy enough to have > wakeup devices called 'wake', 'Wake', 'wAke', 'waKe', 'wakE'? Of course, when you cat/proc/acpi/wakeup, you get "wake", but when you want to enable or disable wakeup of the device "wake", you can echo "wAke" > /proc/acpi/wakeup or echo "wake" > /proc/acpi/wakeup Don't you think it is reasonable? This is just for user's convenience. > > > > Maybe this file should be left for compatibility and we should present > > > something reasonable in /sys? Can't you already get PCI ID from sysfs > > > node? > > PCI ID can be gotten from sysfs, but it is a unique identifier for a > > device, a user can get device name from /usr/share/hwdata/pci.ids in any > > dstribution by PCI ID, he/she is unnecessary to use bus number to get > > device name, bus number is platform-specific, but PCI ID is > > platform-independent. > > If the same info can be gotten from 'sysfs node' field, new field > should not be added. > > -- 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: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID
On Wed, 2008-01-02 at 17:09 +0100, Pavel Machek wrote: > On Wed 2008-01-02 10:03:59, Yi Yang wrote: > > On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote: > > > Hi! > > > > > > > /proc/acpi/wakeup is also case-sensitive, case-insensitive is better. > > > > > > Why? > > A user uses device bus id like 'C093' to enable or disable wakeup of the > > device, for example > > > > echo "C093" > /proc/acpi/wakeup > > > > but i think "c093" should also be ok. i.e. > > Why do you think so? Unix is generally case-sensitive. I see ascii > text in .../wakeup. Maybe some bios vendor is crazy enough to have > wakeup devices called 'wake', 'Wake', 'wAke', 'waKe', 'wakE'? This is just for users' convenience, i believe you must think 0xff and 0xFF are the same. > > > > Maybe this file should be left for compatibility and we should present > > > something reasonable in /sys? Can't you already get PCI ID from sysfs > > > node? > > PCI ID can be gotten from sysfs, but it is a unique identifier for a > > device, a user can get device name from /usr/share/hwdata/pci.ids in any > > dstribution by PCI ID, he/she is unnecessary to use bus number to get > > device name, bus number is platform-specific, but PCI ID is > > platform-independent. > > If the same info can be gotten from 'sysfs node' field, new field > should not be added. Assume you are a user of /proc/acpi/wakeup, when you cat /proc/acpi/wakeup, you only get PCI bus id, then you need use PCI bus id to get the device info, that is platform-specific, if you want to use this PCI bus id to get the device info from another machines, that is absolutely impossible, but it is ok if it is PCI ID. Moreover, you can very easily get the device info from /usr/share/hwdata/pci.ids. grep /usr/share/hwdata/pci.ids That is more convenient than PCI bus id. If we can provide PCI ID in /proc/acpi/wakeup, why we let users get that from /sys/devices/pci...? > > -- 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: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID
On Wed, 2008-01-02 at 17:09 +0100, Pavel Machek wrote: On Wed 2008-01-02 10:03:59, Yi Yang wrote: On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote: Hi! /proc/acpi/wakeup is also case-sensitive, case-insensitive is better. Why? A user uses device bus id like 'C093' to enable or disable wakeup of the device, for example echo C093 /proc/acpi/wakeup but i think c093 should also be ok. i.e. Why do you think so? Unix is generally case-sensitive. I see ascii text in .../wakeup. Maybe some bios vendor is crazy enough to have wakeup devices called 'wake', 'Wake', 'wAke', 'waKe', 'wakE'? This is just for users' convenience, i believe you must think 0xff and 0xFF are the same. Maybe this file should be left for compatibility and we should present something reasonable in /sys? Can't you already get PCI ID from sysfs node? PCI ID can be gotten from sysfs, but it is a unique identifier for a device, a user can get device name from /usr/share/hwdata/pci.ids in any dstribution by PCI ID, he/she is unnecessary to use bus number to get device name, bus number is platform-specific, but PCI ID is platform-independent. If the same info can be gotten from 'sysfs node' field, new field should not be added. Assume you are a user of /proc/acpi/wakeup, when you cat /proc/acpi/wakeup, you only get PCI bus id, then you need use PCI bus id to get the device info, that is platform-specific, if you want to use this PCI bus id to get the device info from another machines, that is absolutely impossible, but it is ok if it is PCI ID. Moreover, you can very easily get the device info from /usr/share/hwdata/pci.ids. grep PCI ID /usr/share/hwdata/pci.ids That is more convenient than PCI bus id. If we can provide PCI ID in /proc/acpi/wakeup, why we let users get that from /sys/devices/pci...? -- 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: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID
On Wed, 2008-01-02 at 17:09 +0100, Pavel Machek wrote: On Wed 2008-01-02 10:03:59, Yi Yang wrote: On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote: Hi! /proc/acpi/wakeup is also case-sensitive, case-insensitive is better. Why? A user uses device bus id like 'C093' to enable or disable wakeup of the device, for example echo C093 /proc/acpi/wakeup but i think c093 should also be ok. i.e. Why do you think so? Unix is generally case-sensitive. I see ascii text in .../wakeup. Maybe some bios vendor is crazy enough to have wakeup devices called 'wake', 'Wake', 'wAke', 'waKe', 'wakE'? Of course, when you cat/proc/acpi/wakeup, you get wake, but when you want to enable or disable wakeup of the device wake, you can echo wAke /proc/acpi/wakeup or echo wake /proc/acpi/wakeup Don't you think it is reasonable? This is just for user's convenience. Maybe this file should be left for compatibility and we should present something reasonable in /sys? Can't you already get PCI ID from sysfs node? PCI ID can be gotten from sysfs, but it is a unique identifier for a device, a user can get device name from /usr/share/hwdata/pci.ids in any dstribution by PCI ID, he/she is unnecessary to use bus number to get device name, bus number is platform-specific, but PCI ID is platform-independent. If the same info can be gotten from 'sysfs node' field, new field should not be added. -- 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: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID
On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote: > Hi! > > > /proc/acpi/wakeup is also case-sensitive, case-insensitive is better. > > Why? A user uses device bus id like 'C093' to enable or disable wakeup of the device, for example echo "C093" > /proc/acpi/wakeup but i think "c093" should also be ok. i.e. "echo 'c093' > /proc/acpi/wakeup" should have the same result as "echo 'C093' > /proc/acpi/wakeup". That is to say, it should be case-insensitive. > > > In addtion, this patch appends a new column 'PCI ID' to /proc/acpi/wakeup > > , the user can use it to get the corresponding device name very > > conveniently because PCI ID is a unique identifier and platform-independent. > > Userland interface change...? Not at all, i didn't find any userland application assumes /proc/acpi/wakeup must be that kind of format. In fact, /proc output is always changing. :-) > > Maybe this file should be left for compatibility and we should present > something reasonable in /sys? Can't you already get PCI ID from sysfs > node? PCI ID can be gotten from sysfs, but it is a unique identifier for a device, a user can get device name from /usr/share/hwdata/pci.ids in any dstribution by PCI ID, he/she is unnecessary to use bus number to get device name, bus number is platform-specific, but PCI ID is platform-independent. > Pavel > > > [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup > > Device S-state Status Sysfs node > > C093 S5 disabled pci::00:1e.0 > > C0E8 S3 disabled pci::00:1d.0 > > C0EF S3 disabled pci::00:1d.1 > > C0F0 S3 disabled pci::00:1d.2 > > C0F1 S3 disabled pci::00:1d.3 > > C0F2 S3 disabled pci::00:1d.7 > > C0F9 S0 disabled pci::00:1c.0 > > C21D S0 disabled pci::08:00.0 > > C109 S5 disabled pci::00:1c.1 > > C228 S5 disabled pci::10:00.0 > > C10F S5 disabled pci::00:1c.3 > > C229 S5 disabled > > [EMAIL PROTECTED] ~]# > > > > After applying this patch, the output is: > > > > [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup > > Device S-state Status Sysfs node PCI ID > > C093 S5 disabled pci::00:1e.0 0x2448 > > C0E8 S3 disabled pci::00:1d.0 0x27c8 > > C0EF S3 disabled pci::00:1d.1 0x27c9 > > C0F0 S3 disabled pci::00:1d.2 0x27ca > > C0F1 S3 disabled pci::00:1d.3 0x27cb > > C0F2 S3 disabled pci::00:1d.7 0x27cc > > C0F9 S0 disabled pci::00:1c.0 0x27d0 > > C21D S0 disabled pci::08:00.0 0x16fd > > C109 S5 disabled pci::00:1c.1 0x27d2 > > C228 S5 disabled pci::10:00.0 0x4222 > > C10F S5 disabled pci::00:1c.3 0x27d6 > > C229 S5 disabled > -- 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: [PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID
On Wed, 2008-01-02 at 00:20 +0100, Pavel Machek wrote: Hi! /proc/acpi/wakeup is also case-sensitive, case-insensitive is better. Why? A user uses device bus id like 'C093' to enable or disable wakeup of the device, for example echo C093 /proc/acpi/wakeup but i think c093 should also be ok. i.e. echo 'c093' /proc/acpi/wakeup should have the same result as echo 'C093' /proc/acpi/wakeup. That is to say, it should be case-insensitive. In addtion, this patch appends a new column 'PCI ID' to /proc/acpi/wakeup , the user can use it to get the corresponding device name very conveniently because PCI ID is a unique identifier and platform-independent. Userland interface change...? Not at all, i didn't find any userland application assumes /proc/acpi/wakeup must be that kind of format. In fact, /proc output is always changing. :-) Maybe this file should be left for compatibility and we should present something reasonable in /sys? Can't you already get PCI ID from sysfs node? PCI ID can be gotten from sysfs, but it is a unique identifier for a device, a user can get device name from /usr/share/hwdata/pci.ids in any dstribution by PCI ID, he/she is unnecessary to use bus number to get device name, bus number is platform-specific, but PCI ID is platform-independent. Pavel [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node C093 S5 disabled pci::00:1e.0 C0E8 S3 disabled pci::00:1d.0 C0EF S3 disabled pci::00:1d.1 C0F0 S3 disabled pci::00:1d.2 C0F1 S3 disabled pci::00:1d.3 C0F2 S3 disabled pci::00:1d.7 C0F9 S0 disabled pci::00:1c.0 C21D S0 disabled pci::08:00.0 C109 S5 disabled pci::00:1c.1 C228 S5 disabled pci::10:00.0 C10F S5 disabled pci::00:1c.3 C229 S5 disabled [EMAIL PROTECTED] ~]# After applying this patch, the output is: [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 disabled pci::00:1c.0 0x27d0 C21D S0 disabled pci::08:00.0 0x16fd C109 S5 disabled pci::00:1c.1 0x27d2 C228 S5 disabled pci::10:00.0 0x4222 C10F S5 disabled pci::00:1c.3 0x27d6 C229 S5 disabled -- 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/
[PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID
Subject: ACPI: Correct wakeup set error and append a new column PCI ID From: Yi Yang <[EMAIL PROTECTED]> The user can't get any information when echo an invalid value to /proc/acpi/wakeup although it is failed, but the user can set /proc/acpi/wakeup successfully if echo an value whose prefix is a valid value for /proc/acpi/wakeup no matter what the suffix is. /proc/acpi/wakeup is also case-sensitive, case-insensitive is better. This patch will fix the aforementioned issues, it will return the error information if input is an invalid value, it also make /proc/acpi/wakeup case-insensitive, i.e. it consider 'SLPB' and 'sLpb'are the same. In addtion, this patch appends a new column 'PCI ID' to /proc/acpi/wakeup , the user can use it to get the corresponding device name very conveniently because PCI ID is a unique identifier and platform-independent. Before applying this patch, the output is: [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node C093 S5 disabled pci::00:1e.0 C0E8 S3 disabled pci::00:1d.0 C0EF S3 disabled pci::00:1d.1 C0F0 S3 disabled pci::00:1d.2 C0F1 S3 disabled pci::00:1d.3 C0F2 S3 disabled pci::00:1d.7 C0F9 S0 disabled pci::00:1c.0 C21D S0 disabled pci::08:00.0 C109 S5 disabled pci::00:1c.1 C228 S5 disabled pci::10:00.0 C10F S5 disabled pci::00:1c.3 C229 S5 disabled [EMAIL PROTECTED] ~]# echo "xyzw" > /proc/acpi/wakeup [EMAIL PROTECTED] ~]# echo "C093x" > /proc/acpi/wakeup [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node C093 S5 enabled pci::00:1e.0 C0E8 S3 disabled pci::00:1d.0 C0EF S3 disabled pci::00:1d.1 C0F0 S3 disabled pci::00:1d.2 C0F1 S3 disabled pci::00:1d.3 C0F2 S3 disabled pci::00:1d.7 C0F9 S0 disabled pci::00:1c.0 C21D S0 disabled pci::08:00.0 C109 S5 disabled pci::00:1c.1 C228 S5 disabled pci::10:00.0 C10F S5 disabled pci::00:1c.3 C229 S5 disabled [EMAIL PROTECTED] ~]# echo "C093x" > /proc/acpi/wakeup [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node C093 S5 disabled pci::00:1e.0 C0E8 S3 disabled pci::00:1d.0 C0EF S3 disabled pci::00:1d.1 C0F0 S3 disabled pci::00:1d.2 C0F1 S3 disabled pci::00:1d.3 C0F2 S3 disabled pci::00:1d.7 C0F9 S0 disabled pci::00:1c.0 C21D S0 disabled pci::08:00.0 C109 S5 disabled pci::00:1c.1 C228 S5 disabled pci::10:00.0 C10F S5 disabled pci::00:1c.3 C229 S5 disabled [EMAIL PROTECTED] ~]# echo "c093" > /proc/acpi/wakeup [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node C093 S5 disabled pci::00:1e.0 C0E8 S3 disabled pci::00:1d.0 C0EF S3 disabled pci::00:1d.1 C0F0 S3 disabled pci::00:1d.2 C0F1 S3 disabled pci::00:1d.3 C0F2 S3 disabled pci::00:1d.7 C0F9 S0 disabled pci::00:1c.0 C21D S0 disabled pci::08:00.0 C109 S5 disabled pci::00:1c.1 C228 S5 disabled pci::10:00.0 C10F S5 disabled pci::00:1c.3 C229 S5 disabled [EMAIL PROTECTED] ~]# After applying this patch, the output is: [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 disabled pci::00:1c.0 0x27d0 C21D S0 disabled pci::08:00.0 0x16fd C109 S5 disabled pci::00:1c.1 0x27d2 C228 S5 disabled pci::10:00.0 0x4222 C10F S5 disabled pci::00:1c.3 0x27d6 C229 S5 disabled [EMAIL PROTECTED] ~]# echo "xyzw" > /proc/acpi/wakeup -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo "C093x" > /proc/acpi/wakeup -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci
[PATCH linux-acpi] Correct wakeup set error and append a new column PCI ID
Subject: ACPI: Correct wakeup set error and append a new column PCI ID From: Yi Yang [EMAIL PROTECTED] The user can't get any information when echo an invalid value to /proc/acpi/wakeup although it is failed, but the user can set /proc/acpi/wakeup successfully if echo an value whose prefix is a valid value for /proc/acpi/wakeup no matter what the suffix is. /proc/acpi/wakeup is also case-sensitive, case-insensitive is better. This patch will fix the aforementioned issues, it will return the error information if input is an invalid value, it also make /proc/acpi/wakeup case-insensitive, i.e. it consider 'SLPB' and 'sLpb'are the same. In addtion, this patch appends a new column 'PCI ID' to /proc/acpi/wakeup , the user can use it to get the corresponding device name very conveniently because PCI ID is a unique identifier and platform-independent. Before applying this patch, the output is: [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node C093 S5 disabled pci::00:1e.0 C0E8 S3 disabled pci::00:1d.0 C0EF S3 disabled pci::00:1d.1 C0F0 S3 disabled pci::00:1d.2 C0F1 S3 disabled pci::00:1d.3 C0F2 S3 disabled pci::00:1d.7 C0F9 S0 disabled pci::00:1c.0 C21D S0 disabled pci::08:00.0 C109 S5 disabled pci::00:1c.1 C228 S5 disabled pci::10:00.0 C10F S5 disabled pci::00:1c.3 C229 S5 disabled [EMAIL PROTECTED] ~]# echo xyzw /proc/acpi/wakeup [EMAIL PROTECTED] ~]# echo C093x /proc/acpi/wakeup [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node C093 S5 enabled pci::00:1e.0 C0E8 S3 disabled pci::00:1d.0 C0EF S3 disabled pci::00:1d.1 C0F0 S3 disabled pci::00:1d.2 C0F1 S3 disabled pci::00:1d.3 C0F2 S3 disabled pci::00:1d.7 C0F9 S0 disabled pci::00:1c.0 C21D S0 disabled pci::08:00.0 C109 S5 disabled pci::00:1c.1 C228 S5 disabled pci::10:00.0 C10F S5 disabled pci::00:1c.3 C229 S5 disabled [EMAIL PROTECTED] ~]# echo C093x /proc/acpi/wakeup [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node C093 S5 disabled pci::00:1e.0 C0E8 S3 disabled pci::00:1d.0 C0EF S3 disabled pci::00:1d.1 C0F0 S3 disabled pci::00:1d.2 C0F1 S3 disabled pci::00:1d.3 C0F2 S3 disabled pci::00:1d.7 C0F9 S0 disabled pci::00:1c.0 C21D S0 disabled pci::08:00.0 C109 S5 disabled pci::00:1c.1 C228 S5 disabled pci::10:00.0 C10F S5 disabled pci::00:1c.3 C229 S5 disabled [EMAIL PROTECTED] ~]# echo c093 /proc/acpi/wakeup [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node C093 S5 disabled pci::00:1e.0 C0E8 S3 disabled pci::00:1d.0 C0EF S3 disabled pci::00:1d.1 C0F0 S3 disabled pci::00:1d.2 C0F1 S3 disabled pci::00:1d.3 C0F2 S3 disabled pci::00:1d.7 C0F9 S0 disabled pci::00:1c.0 C21D S0 disabled pci::08:00.0 C109 S5 disabled pci::00:1c.1 C228 S5 disabled pci::10:00.0 C10F S5 disabled pci::00:1c.3 C229 S5 disabled [EMAIL PROTECTED] ~]# After applying this patch, the output is: [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 disabled pci::00:1c.0 0x27d0 C21D S0 disabled pci::08:00.0 0x16fd C109 S5 disabled pci::00:1c.1 0x27d2 C228 S5 disabled pci::10:00.0 0x4222 C10F S5 disabled pci::00:1c.3 0x27d6 C229 S5 disabled [EMAIL PROTECTED] ~]# echo xyzw /proc/acpi/wakeup -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# echo C093x /proc/acpi/wakeup -bash: echo: write error: Invalid argument [EMAIL PROTECTED] ~]# cat /proc/acpi/wakeup Device S-state Status Sysfs node PCI ID C093 S5 disabled pci::00:1e.0 0x2448 C0E8 S3 disabled pci::00:1d.0 0x27c8 C0EF S3 disabled pci::00:1d.1 0x27c9 C0F0 S3 disabled pci::00:1d.2 0x27ca C0F1 S3 disabled pci::00:1d.3 0x27cb C0F2 S3 disabled pci::00:1d.7 0x27cc C0F9 S0 disabled pci::00:1c.0 0x27d0 C21D S0 disabled
[PATCH linux-acpi] Remove superfluous code and correct counting error in function acpi_system_write_alarm
In function acpi_system_write_alarm in file drivers/acpi/sleep/proc.c,
big sec, min, hr, mo, day and yr are counted twice to get reasonable
values, that is very superfluous, we can do that only once.
In additon, /proc/acpi/alarm can set a related value which can be
specified as years MM months DD days HH hours MM minutes SS
senconds, it isn't a date, so you can specify as +-00-00 96:00:00
, that means 3 days later, current code can't handle such a case.
This patch removes unnecessary code and does with the aforementioned
situation.
Before applying this patch:
[EMAIL PROTECTED] /]# cat /proc/acpi/alarm
2007-12-00 00:00:00
[EMAIL PROTECTED] /]# echo "-00-00 96:180:180" > /proc/acpi/alarm
[EMAIL PROTECTED] /]# cat /proc/acpi/alarm
0007-12-02 **:**:**
[EMAIL PROTECTED] /]#
After applying this patch:
[EMAIL PROTECTED] ~]# echo "2007-12-00 00:00:00" > /proc/acpi/alarm
[EMAIL PROTECTED] ~]# cat /proc/acpi/alarm
2007-12-00 00:00:00
[EMAIL PROTECTED] ~]# echo "-00-00 96:180:180" > /proc/acpi/alarm
[EMAIL PROTECTED] ~]# cat /proc/acpi/alarm
0007-12-04 03:03:00
[EMAIL PROTECTED] ~]#
Signed-off by Yi Yang <[EMAIL PROTECTED]>
diff --git a/drivers/acpi/sleep/proc.c b/drivers/acpi/sleep/proc.c
index fce78fb..f8df521 100644
--- a/drivers/acpi/sleep/proc.c
+++ b/drivers/acpi/sleep/proc.c
@@ -256,27 +256,6 @@ acpi_system_write_alarm(struct file *file,
if ((result = get_date_field(, )))
goto end;
- if (sec > 59) {
- min += 1;
- sec -= 60;
- }
- if (min > 59) {
- hr += 1;
- min -= 60;
- }
- if (hr > 23) {
- day += 1;
- hr -= 24;
- }
- if (day > 31) {
- mo += 1;
- day -= 31;
- }
- if (mo > 12) {
- yr += 1;
- mo -= 12;
- }
-
spin_lock_irq(_lock);
rtc_control = CMOS_READ(RTC_CONTROL);
@@ -293,24 +272,24 @@ acpi_system_write_alarm(struct file *file,
spin_unlock_irq(_lock);
if (sec > 59) {
- min++;
- sec -= 60;
+ min += sec/60;
+ sec = sec%60;
}
if (min > 59) {
- hr++;
- min -= 60;
+ hr += min/60;
+ min = min%60;
}
if (hr > 23) {
- day++;
- hr -= 24;
+ day += hr/24;
+ hr = hr%24;
}
if (day > 31) {
- mo++;
- day -= 31;
+ mo += day/32;
+ day = day%32;
}
if (mo > 12) {
- yr++;
- mo -= 12;
+ yr += mo/13;
+ mo = mo%13;
}
spin_lock_irq(_lock);
--
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/
[PATCH linux-acpi] Remove superfluous code and correct counting error in function acpi_system_write_alarm
In function acpi_system_write_alarm in file drivers/acpi/sleep/proc.c,
big sec, min, hr, mo, day and yr are counted twice to get reasonable
values, that is very superfluous, we can do that only once.
In additon, /proc/acpi/alarm can set a related value which can be
specified as years MM months DD days HH hours MM minutes SS
senconds, it isn't a date, so you can specify as +-00-00 96:00:00
, that means 3 days later, current code can't handle such a case.
This patch removes unnecessary code and does with the aforementioned
situation.
Before applying this patch:
[EMAIL PROTECTED] /]# cat /proc/acpi/alarm
2007-12-00 00:00:00
[EMAIL PROTECTED] /]# echo -00-00 96:180:180 /proc/acpi/alarm
[EMAIL PROTECTED] /]# cat /proc/acpi/alarm
0007-12-02 **:**:**
[EMAIL PROTECTED] /]#
After applying this patch:
[EMAIL PROTECTED] ~]# echo 2007-12-00 00:00:00 /proc/acpi/alarm
[EMAIL PROTECTED] ~]# cat /proc/acpi/alarm
2007-12-00 00:00:00
[EMAIL PROTECTED] ~]# echo -00-00 96:180:180 /proc/acpi/alarm
[EMAIL PROTECTED] ~]# cat /proc/acpi/alarm
0007-12-04 03:03:00
[EMAIL PROTECTED] ~]#
Signed-off by Yi Yang [EMAIL PROTECTED]
diff --git a/drivers/acpi/sleep/proc.c b/drivers/acpi/sleep/proc.c
index fce78fb..f8df521 100644
--- a/drivers/acpi/sleep/proc.c
+++ b/drivers/acpi/sleep/proc.c
@@ -256,27 +256,6 @@ acpi_system_write_alarm(struct file *file,
if ((result = get_date_field(p, sec)))
goto end;
- if (sec 59) {
- min += 1;
- sec -= 60;
- }
- if (min 59) {
- hr += 1;
- min -= 60;
- }
- if (hr 23) {
- day += 1;
- hr -= 24;
- }
- if (day 31) {
- mo += 1;
- day -= 31;
- }
- if (mo 12) {
- yr += 1;
- mo -= 12;
- }
-
spin_lock_irq(rtc_lock);
rtc_control = CMOS_READ(RTC_CONTROL);
@@ -293,24 +272,24 @@ acpi_system_write_alarm(struct file *file,
spin_unlock_irq(rtc_lock);
if (sec 59) {
- min++;
- sec -= 60;
+ min += sec/60;
+ sec = sec%60;
}
if (min 59) {
- hr++;
- min -= 60;
+ hr += min/60;
+ min = min%60;
}
if (hr 23) {
- day++;
- hr -= 24;
+ day += hr/24;
+ hr = hr%24;
}
if (day 31) {
- mo++;
- day -= 31;
+ mo += day/32;
+ day = day%32;
}
if (mo 12) {
- yr++;
- mo -= 12;
+ yr += mo/13;
+ mo = mo%13;
}
spin_lock_irq(rtc_lock);
--
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/
[PATCH linux-acpi] Fix /proc/acpi/alarm set error
/proc/acpi/alarm can't be set correctly, here is a sample: [EMAIL PROTECTED] /]# echo "2006 09" > /proc/acpi/alarm [EMAIL PROTECTED] /]# cat /proc/acpi/alarm 2007-12-09 09:09:09 [EMAIL PROTECTED] /]# echo "2006 04" > /proc/acpi/alarm [EMAIL PROTECTED] /]# cat /proc/acpi/alarm 2007-12-04 04:04:04 [EMAIL PROTECTED] /]# Obviously, it is wrong, it should consider it as an invalid input. This patch'll fix this issue, after applying this patch, the result is: [EMAIL PROTECTED] /]# echo "2008 09" > /proc/acpi/alarm -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# Signed-off by Yi Yang <[EMAIL PROTECTED]> diff --git a/drivers/acpi/sleep/proc.c b/drivers/acpi/sleep/proc.c index 1538355..fce78fb 100644 --- a/drivers/acpi/sleep/proc.c +++ b/drivers/acpi/sleep/proc.c @@ -178,6 +178,9 @@ static int get_date_field(char **p, u32 * value) * Try to find delimeter, only to insert null. The end of the * string won't have one, but is still valid. */ + if (*p == NULL) + return result; + next = strpbrk(*p, "- :"); if (next) *next++ = '\0'; @@ -190,6 +193,8 @@ static int get_date_field(char **p, u32 * value) if (next) *p = next; + else + *p = NULL; return result; } -- 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/
[PATCH linux-acpi] Fix /proc/acpi/alarm set error
/proc/acpi/alarm can't be set correctly, here is a sample: [EMAIL PROTECTED] /]# echo 2006 09 /proc/acpi/alarm [EMAIL PROTECTED] /]# cat /proc/acpi/alarm 2007-12-09 09:09:09 [EMAIL PROTECTED] /]# echo 2006 04 /proc/acpi/alarm [EMAIL PROTECTED] /]# cat /proc/acpi/alarm 2007-12-04 04:04:04 [EMAIL PROTECTED] /]# Obviously, it is wrong, it should consider it as an invalid input. This patch'll fix this issue, after applying this patch, the result is: [EMAIL PROTECTED] /]# echo 2008 09 /proc/acpi/alarm -bash: echo: write error: Invalid argument [EMAIL PROTECTED] /]# Signed-off by Yi Yang [EMAIL PROTECTED] diff --git a/drivers/acpi/sleep/proc.c b/drivers/acpi/sleep/proc.c index 1538355..fce78fb 100644 --- a/drivers/acpi/sleep/proc.c +++ b/drivers/acpi/sleep/proc.c @@ -178,6 +178,9 @@ static int get_date_field(char **p, u32 * value) * Try to find delimeter, only to insert null. The end of the * string won't have one, but is still valid. */ + if (*p == NULL) + return result; + next = strpbrk(*p, - :); if (next) *next++ = '\0'; @@ -190,6 +193,8 @@ static int get_date_field(char **p, u32 * value) if (next) *p = next; + else + *p = NULL; return result; } -- 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/
[PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 3
Compared to try 2, there are some little changes according to Valdis and Frans'
feedbacks.
For SysRq, we just can get hot key list from Documentation/sysrq.txt
, but in the most of cases, the user can't access it by hand on
using SysRq to debug, so it is better for SysRq to provide an online
help for the users.
SysRq has already provided a similiar help before this patch, but it
is not so clear that the user doesn't know what happened and what
he/she should do.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
key help info for such a function as "Changing Loglevel", just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
This patch adds this online help function, it'll print the whole hot
key list and corresponding function descriptions, it can print the new
defined hot key without any changed needed.
The output is below on pressing an undefined hot key before this patch:
SysRq : HELP : loglevel0-8 reBoot Crashdump tErm Full kIll saK showMem
Nice powerOff showPc show-all-timers(Q) unRaw Sync showTasks Unmount
shoW-blocked-tasks
But after applying this patch, the output is:
SysRq :
this hot key isn't defined.
SysRq Help Information:
Hot KeyFunction Description
===
0-8 Changing Loglevel To 0-8
b Resetting
c Trigger a crashdump
e Terminate All Tasks
f Manual OOM execution
i Kill All Tasks
k SAK
m Show Memory
n Nice All RT Tasks
o Power Off
p Show Regs
q Show Pending Timers
r Keyboard mode set to XLATE
s Emergency Sync
t Show State
u Emergency Remount R/O
w Show Blocked State
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
sysrq.c | 38 ++
1 file changed, 26 insertions(+), 12 deletions(-)
--- a/drivers/char/sysrq.c 2007-10-16 18:32:58.0 +0800
+++ b/drivers/char/sysrq.c 2007-10-18 19:32:13.0 +0800
@@ -45,6 +45,16 @@ int __read_mostly __sysrq_enabled = 1;
static int __read_mostly sysrq_always_enabled;
+/*
+ * Hot key table SysRq supports
+ */
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
+};
+
int sysrq_on(void)
{
return __sysrq_enabled || sysrq_always_enabled;
@@ -410,7 +420,7 @@ void __handle_sysrq(int key, struct tty_
spin_lock_irqsave(_key_table_lock, flags);
orig_log_level = console_loglevel;
console_loglevel = 7;
- printk(KERN_INFO "SysRq : ");
+ printk(KERN_INFO "SysRq : \n");
op_p = __sysrq_get_key_op(key);
if (op_p) {
@@ -426,19 +436,23 @@ void __handle_sysrq(int key, struct tty_
printk("This sysrq operation is disabled.\n");
}
} else {
- printk("HELP : ");
- /* Only print the help msg once per handler */
+ /*
+* Print SysRq hot key list
+*/
+ printk(KERN_INFO "this hot key isn't defined.\n\n");
+ printk(KERN_INFO "SysRq Help Information:\n\n");
+ printk(KERN_INFO "Hot KeyFunction Description\n");
+ printk(KERN_INFO "===\n");
+ printk(KERN_INFO " 0-8 %s To 0-8\n",
+ sysrq_loglevel_op.action_msg);
for (i = 0; i < ARRAY_SIZE(sysrq_key_table); i++) {
- if (sysrq_key_table[i]) {
- int j;
+ if (sysrq_key_table[i] == _loglevel_op)
+ continue;
- for (j = 0; sysrq_key_table[i] !=
- sysrq_key_table[j]; j++)
- ;
- if (j != i)
- continue;
- printk("%s ", sysrq_key_table[i]->help_msg);
- }
+ if (sysrq_key_table[i])
+ printk(KERN_INFO " %c %s\n",
+ sysrq_hot_key_table[i],
+ sysrq_key_table[i]->action_msg);
}
printk("\n");
console_loglevel = orig_log_level;
-
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: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2
Frans Pop 写道: > Yi Yang wrote: > >> Hot KeyFunction Description >> === >>0 Changing Loglevel to this value >>1 Changing Loglevel to this value >>2 Changing Loglevel to this value >>3 Changing Loglevel to this value >>4 Changing Loglevel to this value >>5 Changing Loglevel to this value >>6 Changing Loglevel to this value >>7 Changing Loglevel to this value >>8 Changing Loglevel to this value >>9 Changing Loglevel to this value >> > > Wouldn't "Change Loglevel to this value" be more consistent with other > descriptions? > Or maybe even "Change Kernel Loglevel to "; the repeated "this > value" seems somewhat silly. > I think so, too, only one line should be enough. > >>b Resetting >> > > This is not very descriptive. Maybe "Immediate System Reboot"? > > >>c Trigger a crashdump >> > > Most descriptions are capitalized. So for consistency this should be > "Trigger a Crashdump". (Although my personal preference would be to drop > the capitalization for all descriptions.) > > >>p Show Regs >> > > Should "Regs" be expanded to "Registers"? > > >> r Keyboard mode set to XLATE >> > > For consistency: "Set Keyboard Mode to XLATE" > > Cheers, > Frans Pop > > These descriptions are from the callers who are from many subsystems, so I can't change it. - 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: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2
[EMAIL PROTECTED] 写道:
On Wed, 17 Oct 2007 23:22:58 +0800, Yi Yang said:
SysRq has already provided a similiar help before this patch, but it
is not so clear that the user doesn't know what happened and what
he/she should do.
The person is in one of two states:
1) He has been told "recreate the problem, hit alt-sysreq-cokebottle,
and send me the results". He has a mission, and the only feedback he
needs is (a) that he hit cokebottle and not pepsibottle, and (b) the
resulting output.
2) He's already read the file in Documentation/ and just needs a reminder.
In this case, the fact it's only 2 or 3 lines and doesn't scroll other stuff
out of sight is more important.
Screen scroll isn't a problem.
"SysRq : HELP : loglevel0-8 reBoot Crashdump tErm Full kIll saK showMem
Nice powerOff showPc show-all-timers(Q) unRaw Sync showTasks Unmount
shoW-blocked-tasks"
The above help information isn't very user-friendly.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
You're optimizing code that hopefully never gets executed, and even if
it does, you have the optimization *backwards*. If you're worried about
the efficiency, trim it down to output 3 lines - do you realize how many
instructions it takes in the VGA and fb drivers to actually *output* all
these lines? (Seriously - I had a 1.6Ghz P4 laptop, where scrolling the
screen with vga=791 actually ran so slowly that it horqued up the timer
initialization code. *That* was a fun bug to figure out..)
To remove a bad loop is just a plus fix. That loop is really inefficient.
key help info for such a function as "Changing Loglevel", just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
And ironically enough, you then output the same exact text for all levels.
Yes, only one line for them is better.
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
The lists of 'Not defined' tends to scroll the screen away. The old code
instead focuses on listing the things you *can* do. If I'm looking at the
help output, I don't care that 'g' is not defined. I need to be reminded
that 'p' is 'showPc' and D is show-all-locks.
You're right, "Not defined" is meaningless for the common users. I'll
submit a new revision
to fix your concerns.
Thank you very much.
-
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: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2
[EMAIL PROTECTED] 写道:
On Wed, 17 Oct 2007 23:22:58 +0800, Yi Yang said:
SysRq has already provided a similiar help before this patch, but it
is not so clear that the user doesn't know what happened and what
he/she should do.
The person is in one of two states:
1) He has been told recreate the problem, hit alt-sysreq-cokebottle,
and send me the results. He has a mission, and the only feedback he
needs is (a) that he hit cokebottle and not pepsibottle, and (b) the
resulting output.
2) He's already read the file in Documentation/ and just needs a reminder.
In this case, the fact it's only 2 or 3 lines and doesn't scroll other stuff
out of sight is more important.
Screen scroll isn't a problem.
SysRq : HELP : loglevel0-8 reBoot Crashdump tErm Full kIll saK showMem
Nice powerOff showPc show-all-timers(Q) unRaw Sync showTasks Unmount
shoW-blocked-tasks
The above help information isn't very user-friendly.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
You're optimizing code that hopefully never gets executed, and even if
it does, you have the optimization *backwards*. If you're worried about
the efficiency, trim it down to output 3 lines - do you realize how many
instructions it takes in the VGA and fb drivers to actually *output* all
these lines? (Seriously - I had a 1.6Ghz P4 laptop, where scrolling the
screen with vga=791 actually ran so slowly that it horqued up the timer
initialization code. *That* was a fun bug to figure out..)
To remove a bad loop is just a plus fix. That loop is really inefficient.
key help info for such a function as Changing Loglevel, just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
And ironically enough, you then output the same exact text for all levels.
Yes, only one line for them is better.
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
The lists of 'Not defined' tends to scroll the screen away. The old code
instead focuses on listing the things you *can* do. If I'm looking at the
help output, I don't care that 'g' is not defined. I need to be reminded
that 'p' is 'showPc' and D is show-all-locks.
You're right, Not defined is meaningless for the common users. I'll
submit a new revision
to fix your concerns.
Thank you very much.
-
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/
[PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 3
Compared to try 2, there are some little changes according to Valdis and Frans'
feedbacks.
For SysRq, we just can get hot key list from Documentation/sysrq.txt
, but in the most of cases, the user can't access it by hand on
using SysRq to debug, so it is better for SysRq to provide an online
help for the users.
SysRq has already provided a similiar help before this patch, but it
is not so clear that the user doesn't know what happened and what
he/she should do.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
key help info for such a function as Changing Loglevel, just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
This patch adds this online help function, it'll print the whole hot
key list and corresponding function descriptions, it can print the new
defined hot key without any changed needed.
The output is below on pressing an undefined hot key before this patch:
SysRq : HELP : loglevel0-8 reBoot Crashdump tErm Full kIll saK showMem
Nice powerOff showPc show-all-timers(Q) unRaw Sync showTasks Unmount
shoW-blocked-tasks
But after applying this patch, the output is:
SysRq :
this hot key isn't defined.
SysRq Help Information:
Hot KeyFunction Description
===
0-8 Changing Loglevel To 0-8
b Resetting
c Trigger a crashdump
e Terminate All Tasks
f Manual OOM execution
i Kill All Tasks
k SAK
m Show Memory
n Nice All RT Tasks
o Power Off
p Show Regs
q Show Pending Timers
r Keyboard mode set to XLATE
s Emergency Sync
t Show State
u Emergency Remount R/O
w Show Blocked State
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
sysrq.c | 38 ++
1 file changed, 26 insertions(+), 12 deletions(-)
--- a/drivers/char/sysrq.c 2007-10-16 18:32:58.0 +0800
+++ b/drivers/char/sysrq.c 2007-10-18 19:32:13.0 +0800
@@ -45,6 +45,16 @@ int __read_mostly __sysrq_enabled = 1;
static int __read_mostly sysrq_always_enabled;
+/*
+ * Hot key table SysRq supports
+ */
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
+};
+
int sysrq_on(void)
{
return __sysrq_enabled || sysrq_always_enabled;
@@ -410,7 +420,7 @@ void __handle_sysrq(int key, struct tty_
spin_lock_irqsave(sysrq_key_table_lock, flags);
orig_log_level = console_loglevel;
console_loglevel = 7;
- printk(KERN_INFO SysRq : );
+ printk(KERN_INFO SysRq : \n);
op_p = __sysrq_get_key_op(key);
if (op_p) {
@@ -426,19 +436,23 @@ void __handle_sysrq(int key, struct tty_
printk(This sysrq operation is disabled.\n);
}
} else {
- printk(HELP : );
- /* Only print the help msg once per handler */
+ /*
+* Print SysRq hot key list
+*/
+ printk(KERN_INFO this hot key isn't defined.\n\n);
+ printk(KERN_INFO SysRq Help Information:\n\n);
+ printk(KERN_INFO Hot KeyFunction Description\n);
+ printk(KERN_INFO ===\n);
+ printk(KERN_INFO 0-8 %s To 0-8\n,
+ sysrq_loglevel_op.action_msg);
for (i = 0; i ARRAY_SIZE(sysrq_key_table); i++) {
- if (sysrq_key_table[i]) {
- int j;
+ if (sysrq_key_table[i] == sysrq_loglevel_op)
+ continue;
- for (j = 0; sysrq_key_table[i] !=
- sysrq_key_table[j]; j++)
- ;
- if (j != i)
- continue;
- printk(%s , sysrq_key_table[i]-help_msg);
- }
+ if (sysrq_key_table[i])
+ printk(KERN_INFO%c %s\n,
+ sysrq_hot_key_table[i],
+ sysrq_key_table[i]-action_msg);
}
printk(\n);
console_loglevel = orig_log_level;
-
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: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2
Frans Pop 写道: Yi Yang wrote: Hot KeyFunction Description === 0 Changing Loglevel to this value 1 Changing Loglevel to this value 2 Changing Loglevel to this value 3 Changing Loglevel to this value 4 Changing Loglevel to this value 5 Changing Loglevel to this value 6 Changing Loglevel to this value 7 Changing Loglevel to this value 8 Changing Loglevel to this value 9 Changing Loglevel to this value Wouldn't Change Loglevel to this value be more consistent with other descriptions? Or maybe even Change Kernel Loglevel to value; the repeated this value seems somewhat silly. I think so, too, only one line should be enough. b Resetting This is not very descriptive. Maybe Immediate System Reboot? c Trigger a crashdump Most descriptions are capitalized. So for consistency this should be Trigger a Crashdump. (Although my personal preference would be to drop the capitalization for all descriptions.) p Show Regs Should Regs be expanded to Registers? r Keyboard mode set to XLATE For consistency: Set Keyboard Mode to XLATE Cheers, Frans Pop These descriptions are from the callers who are from many subsystems, so I can't change it. - 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/
[PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2
Compared with last patch, this has a little change according to
Andreas and Pavel's feedbacks.
For SysRq, we just can get hot key list from Documentation/sysrq.txt
, but in the most of cases, the user can't access it by hand on
using SysRq to debug, so it is better for SysRq to provide an online
help for the users.
SysRq has already provided a similiar help before this patch, but it
is not so clear that the user doesn't know what happened and what
he/she should do.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
key help info for such a function as "Changing Loglevel", just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
This patch adds this online help function, it'll print the whole hot
key list and corresponding function descriptions, it can print the new
defined hot key without any changed needed.
The output is below on pressing an undefined hot key before this patch:
SysRq : HELP : loglevel0-8 reBoot Crashdump tErm Full kIll saK showMem
Nice powerOff showPc show-all-timers(Q) unRaw Sync showTasks Unmount
shoW-blocked-tasks
But after applying this patch, the output is:
SysRq :
this hot key isn't defined.
SysRq Help Information:
Hot KeyFunction Description
===
0 Changing Loglevel to this value
1 Changing Loglevel to this value
2 Changing Loglevel to this value
3 Changing Loglevel to this value
4 Changing Loglevel to this value
5 Changing Loglevel to this value
6 Changing Loglevel to this value
7 Changing Loglevel to this value
8 Changing Loglevel to this value
9 Changing Loglevel to this value
a Not defined
b Resetting
c Trigger a crashdump
d Not defined
e Terminate All Tasks
f Manual OOM execution
g Not defined
h Not defined
i Kill All Tasks
j Not defined
k SAK
l Not defined
m Show Memory
n Nice All RT Tasks
o Power Off
p Show Regs
q Show Pending Timers
r Keyboard mode set to XLATE
s Emergency Sync
t Show State
u Emergency Remount R/O
v Not defined
w Show Blocked State
x Not defined
y Not defined
z Not defined
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
sysrq.c | 38 ++
1 file changed, 26 insertions(+), 12 deletions(-)
--- a/drivers/char/sysrq.c 2007-10-16 18:32:58.0 +0800
+++ b/drivers/char/sysrq.c 2007-10-17 17:52:45.0 +0800
@@ -45,6 +45,16 @@ int __read_mostly __sysrq_enabled = 1;
static int __read_mostly sysrq_always_enabled;
+/*
+ * Hot key table SysRq supports
+ */
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
+};
+
int sysrq_on(void)
{
return __sysrq_enabled || sysrq_always_enabled;
@@ -81,7 +91,7 @@ static void sysrq_handle_loglevel(int ke
static struct sysrq_key_op sysrq_loglevel_op = {
.handler= sysrq_handle_loglevel,
.help_msg = "loglevel0-8",
- .action_msg = "Changing Loglevel",
+ .action_msg = "Changing Loglevel to this value",
.enable_mask= SYSRQ_ENABLE_LOG,
};
@@ -410,7 +420,7 @@ void __handle_sysrq(int key, struct tty_
spin_lock_irqsave(_key_table_lock, flags);
orig_log_level = console_loglevel;
console_loglevel = 7;
- printk(KERN_INFO "SysRq : ");
+ printk(KERN_INFO "SysRq : \n");
op_p = __sysrq_get_key_op(key);
if (op_p) {
@@ -426,18 +436,22 @@ void __handle_sysrq(int key, struct tty_
printk("This sysrq operation is disabled.\n");
}
} else {
- printk("HELP : ");
- /* Only print the help msg once per handler */
+ /*
+* Print SysRq hot key list
+*/
+ printk(KERN_INFO "this hot key isn't defined.\n\n");
+ printk(KERN_INFO "SysRq Help Information:\n\n");
+ printk(KERN_INFO "Hot KeyFunction Description\n");
+ printk(KERN_INFO "===\n");
for (i = 0; i < ARRAY_SIZE(sysrq_key_table); i++) {
if (sysrq_key_table[i]) {
- int j;
-
- for (j = 0; sysrq_key_table[i] !=
-
Re: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key
Pavel Machek 写道:
Hi!
For SysRq, we just can get hot key list from Documentation/sysrq.txt
, but in the most of cases, the user can't access it by hand on
using SysRq to debug, so it is better for SysRq to provide an online
help for the users.
SysRq has already provided a similiar help before this patch, but it
is not so definite that the user doesn't know what happened and how
to do on pressing an undefined hot key.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
key help info for such a function as "Changing Loglevel", just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
This patch add this online help function, it'll print thw whole hot
key list and corresponding function descriptions, it can print the new
defined hot key without any changed needed.
The output is the below on pressing an undefined hot key:
SysRq : <6>this hot key isn't defined.
SysRq Help Information:
Hot Key Function Description
===
ALT+SysRq+0Changing Loglevel to this value
ALT+SysRq+1Changing Loglevel to this value
ALT+SysRq+2Changing Loglevel to this value
ALT+SysRq+3Changing Loglevel to this value
ALT+SysRq+4Changing Loglevel to this value
ALT+SysRq+5Changing Loglevel to this value
ALT+SysRq+6Changing Loglevel to this value
ALT+SysRq+7Changing Loglevel to this value
ALT+SysRq+8Changing Loglevel to this value
ALT+SysRq+9Changing Loglevel to this value
ALT+SysRq+aNot defined
ALT+SysRq+bResetting
ALT+SysRq+cTrigger a crashdump
ALT+SysRq+dNot defined
ALT+SysRq+eTerminate All Tasks
ALT+SysRq+fManual OOM execution
ALT+SysRq+gNot defined
ALT+SysRq+hNot defined
ALT+SysRq+iKill All Tasks
ALT+SysRq+jNot defined
ALT+SysRq+kSAK
ALT+SysRq+lNot defined
ALT+SysRq+mShow Memory
ALT+SysRq+nNice All RT Tasks
ALT+SysRq+oPower Off
ALT+SysRq+pShow Regs
ALT+SysRq+qShow Pending Timers
ALT+SysRq+rKeyboard mode set to XLATE
ALT+SysRq+sEmergency Sync
ALT+SysRq+tShow State
ALT+SysRq+uEmergency Remount R/O
ALT+SysRq+vNot defined
ALT+SysRq+wShow Blocked State
ALT+SysRq+xNot defined
ALT+SysRq+yNot defined
ALT+SysRq+zNot defined
This will scroll out of users view on small terminals.
Plus it is untrue. On sparc, it is stop+x, not sysrq+x.
You're right, it it better to remove ALT+SysRq
+/*
+ * Hot key table SysRq supports
+ */
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
+};
Plus this is practically guaranteed to get out of sync with
reality. If you did this to make sysrq printing faster... then that is
very bad optimization.
Pavel
This char array is just for output, thees are jsut the keys sysrq has
defined, in the current
design of sysrq, to add a new key is not easy unless you use an existed
but unused key,
so this has no problem.
SysRq is just for debugging or emergent handling, so this array is
impossible to result in
any bad effects, it is just for convenience.
-
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: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key
Andreas Schwab 写道: Yi Yang <[EMAIL PROTECTED]> writes: For SysRq, we just can get hot key list from Documentation/sysrq.txt , but in the most of cases, the user can't access it by hand on using SysRq to debug, so it is better for SysRq to provide an online help for the users. SysRq has already provided a similiar help before this patch, but it is not so definite that the user doesn't know what happened and how to do on pressing an undefined hot key. In addition, that funtion has a big loop with another big loop embedded which is very inefficient, it is intended to skip some hot key help info for such a function as "Changing Loglevel", just print a help info for this, that is very unnecessary. In fact, the key '0' - '8' have different results the user should know. This patch add this online help function, it'll print thw whole hot key list and corresponding function descriptions, it can print the new defined hot key without any changed needed. The output is the below on pressing an undefined hot key: SysRq : <6>this hot key isn't defined. ^^^ This is misplaced. printk(KERN_INFO "SysRq : \n") can fix it. @@ -410,7 +420,7 @@ void __handle_sysrq(int key, struct tty_ spin_lock_irqsave(_key_table_lock, flags); orig_log_level = console_loglevel; console_loglevel = 7; - printk(KERN_INFO "SysRq : "); + printk(KERN_INFO "\nSysRq : "); It does not make any sense to put the printk level just before the newline, since the newline resets it to the default level. Thank you, I'll remove \n. Andreas. - 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: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key
Andreas Schwab 写道: Yi Yang [EMAIL PROTECTED] writes: For SysRq, we just can get hot key list from Documentation/sysrq.txt , but in the most of cases, the user can't access it by hand on using SysRq to debug, so it is better for SysRq to provide an online help for the users. SysRq has already provided a similiar help before this patch, but it is not so definite that the user doesn't know what happened and how to do on pressing an undefined hot key. In addition, that funtion has a big loop with another big loop embedded which is very inefficient, it is intended to skip some hot key help info for such a function as Changing Loglevel, just print a help info for this, that is very unnecessary. In fact, the key '0' - '8' have different results the user should know. This patch add this online help function, it'll print thw whole hot key list and corresponding function descriptions, it can print the new defined hot key without any changed needed. The output is the below on pressing an undefined hot key: SysRq : 6this hot key isn't defined. ^^^ This is misplaced. printk(KERN_INFO SysRq : \n) can fix it. @@ -410,7 +420,7 @@ void __handle_sysrq(int key, struct tty_ spin_lock_irqsave(sysrq_key_table_lock, flags); orig_log_level = console_loglevel; console_loglevel = 7; - printk(KERN_INFO SysRq : ); + printk(KERN_INFO \nSysRq : ); It does not make any sense to put the printk level just before the newline, since the newline resets it to the default level. Thank you, I'll remove \n. Andreas. - 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: [PATCH 2.6.23] SysRq: print hotkey info while pressing undef key
Pavel Machek 写道:
Hi!
For SysRq, we just can get hot key list from Documentation/sysrq.txt
, but in the most of cases, the user can't access it by hand on
using SysRq to debug, so it is better for SysRq to provide an online
help for the users.
SysRq has already provided a similiar help before this patch, but it
is not so definite that the user doesn't know what happened and how
to do on pressing an undefined hot key.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
key help info for such a function as Changing Loglevel, just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
This patch add this online help function, it'll print thw whole hot
key list and corresponding function descriptions, it can print the new
defined hot key without any changed needed.
The output is the below on pressing an undefined hot key:
SysRq : 6this hot key isn't defined.
SysRq Help Information:
Hot Key Function Description
===
ALT+SysRq+0Changing Loglevel to this value
ALT+SysRq+1Changing Loglevel to this value
ALT+SysRq+2Changing Loglevel to this value
ALT+SysRq+3Changing Loglevel to this value
ALT+SysRq+4Changing Loglevel to this value
ALT+SysRq+5Changing Loglevel to this value
ALT+SysRq+6Changing Loglevel to this value
ALT+SysRq+7Changing Loglevel to this value
ALT+SysRq+8Changing Loglevel to this value
ALT+SysRq+9Changing Loglevel to this value
ALT+SysRq+aNot defined
ALT+SysRq+bResetting
ALT+SysRq+cTrigger a crashdump
ALT+SysRq+dNot defined
ALT+SysRq+eTerminate All Tasks
ALT+SysRq+fManual OOM execution
ALT+SysRq+gNot defined
ALT+SysRq+hNot defined
ALT+SysRq+iKill All Tasks
ALT+SysRq+jNot defined
ALT+SysRq+kSAK
ALT+SysRq+lNot defined
ALT+SysRq+mShow Memory
ALT+SysRq+nNice All RT Tasks
ALT+SysRq+oPower Off
ALT+SysRq+pShow Regs
ALT+SysRq+qShow Pending Timers
ALT+SysRq+rKeyboard mode set to XLATE
ALT+SysRq+sEmergency Sync
ALT+SysRq+tShow State
ALT+SysRq+uEmergency Remount R/O
ALT+SysRq+vNot defined
ALT+SysRq+wShow Blocked State
ALT+SysRq+xNot defined
ALT+SysRq+yNot defined
ALT+SysRq+zNot defined
This will scroll out of users view on small terminals.
Plus it is untrue. On sparc, it is stop+x, not sysrq+x.
You're right, it it better to remove ALT+SysRq
+/*
+ * Hot key table SysRq supports
+ */
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
+};
Plus this is practically guaranteed to get out of sync with
reality. If you did this to make sysrq printing faster... then that is
very bad optimization.
Pavel
This char array is just for output, thees are jsut the keys sysrq has
defined, in the current
design of sysrq, to add a new key is not easy unless you use an existed
but unused key,
so this has no problem.
SysRq is just for debugging or emergent handling, so this array is
impossible to result in
any bad effects, it is just for convenience.
-
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/
[PATCH 2.6.23] SysRq: print hotkey info while pressing undef key, try 2
Compared with last patch, this has a little change according to
Andreas and Pavel's feedbacks.
For SysRq, we just can get hot key list from Documentation/sysrq.txt
, but in the most of cases, the user can't access it by hand on
using SysRq to debug, so it is better for SysRq to provide an online
help for the users.
SysRq has already provided a similiar help before this patch, but it
is not so clear that the user doesn't know what happened and what
he/she should do.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
key help info for such a function as Changing Loglevel, just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
This patch adds this online help function, it'll print the whole hot
key list and corresponding function descriptions, it can print the new
defined hot key without any changed needed.
The output is below on pressing an undefined hot key before this patch:
SysRq : HELP : loglevel0-8 reBoot Crashdump tErm Full kIll saK showMem
Nice powerOff showPc show-all-timers(Q) unRaw Sync showTasks Unmount
shoW-blocked-tasks
But after applying this patch, the output is:
SysRq :
this hot key isn't defined.
SysRq Help Information:
Hot KeyFunction Description
===
0 Changing Loglevel to this value
1 Changing Loglevel to this value
2 Changing Loglevel to this value
3 Changing Loglevel to this value
4 Changing Loglevel to this value
5 Changing Loglevel to this value
6 Changing Loglevel to this value
7 Changing Loglevel to this value
8 Changing Loglevel to this value
9 Changing Loglevel to this value
a Not defined
b Resetting
c Trigger a crashdump
d Not defined
e Terminate All Tasks
f Manual OOM execution
g Not defined
h Not defined
i Kill All Tasks
j Not defined
k SAK
l Not defined
m Show Memory
n Nice All RT Tasks
o Power Off
p Show Regs
q Show Pending Timers
r Keyboard mode set to XLATE
s Emergency Sync
t Show State
u Emergency Remount R/O
v Not defined
w Show Blocked State
x Not defined
y Not defined
z Not defined
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
sysrq.c | 38 ++
1 file changed, 26 insertions(+), 12 deletions(-)
--- a/drivers/char/sysrq.c 2007-10-16 18:32:58.0 +0800
+++ b/drivers/char/sysrq.c 2007-10-17 17:52:45.0 +0800
@@ -45,6 +45,16 @@ int __read_mostly __sysrq_enabled = 1;
static int __read_mostly sysrq_always_enabled;
+/*
+ * Hot key table SysRq supports
+ */
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
+};
+
int sysrq_on(void)
{
return __sysrq_enabled || sysrq_always_enabled;
@@ -81,7 +91,7 @@ static void sysrq_handle_loglevel(int ke
static struct sysrq_key_op sysrq_loglevel_op = {
.handler= sysrq_handle_loglevel,
.help_msg = loglevel0-8,
- .action_msg = Changing Loglevel,
+ .action_msg = Changing Loglevel to this value,
.enable_mask= SYSRQ_ENABLE_LOG,
};
@@ -410,7 +420,7 @@ void __handle_sysrq(int key, struct tty_
spin_lock_irqsave(sysrq_key_table_lock, flags);
orig_log_level = console_loglevel;
console_loglevel = 7;
- printk(KERN_INFO SysRq : );
+ printk(KERN_INFO SysRq : \n);
op_p = __sysrq_get_key_op(key);
if (op_p) {
@@ -426,18 +436,22 @@ void __handle_sysrq(int key, struct tty_
printk(This sysrq operation is disabled.\n);
}
} else {
- printk(HELP : );
- /* Only print the help msg once per handler */
+ /*
+* Print SysRq hot key list
+*/
+ printk(KERN_INFO this hot key isn't defined.\n\n);
+ printk(KERN_INFO SysRq Help Information:\n\n);
+ printk(KERN_INFO Hot KeyFunction Description\n);
+ printk(KERN_INFO ===\n);
for (i = 0; i ARRAY_SIZE(sysrq_key_table); i++) {
if (sysrq_key_table[i]) {
- int j;
-
- for (j = 0; sysrq_key_table[i] !=
- sysrq_key_table[j]; j++)
- ;
- if (j != i)
- continue
[PATCH 2.6.23] SysRq: print hotkey info while pressing undef key
For SysRq, we just can get hot key list from Documentation/sysrq.txt
, but in the most of cases, the user can't access it by hand on
using SysRq to debug, so it is better for SysRq to provide an online
help for the users.
SysRq has already provided a similiar help before this patch, but it
is not so definite that the user doesn't know what happened and how
to do on pressing an undefined hot key.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
key help info for such a function as "Changing Loglevel", just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
This patch add this online help function, it'll print thw whole hot
key list and corresponding function descriptions, it can print the new
defined hot key without any changed needed.
The output is the below on pressing an undefined hot key:
SysRq : <6>this hot key isn't defined.
SysRq Help Information:
Hot Key Function Description
===
ALT+SysRq+0Changing Loglevel to this value
ALT+SysRq+1Changing Loglevel to this value
ALT+SysRq+2Changing Loglevel to this value
ALT+SysRq+3Changing Loglevel to this value
ALT+SysRq+4Changing Loglevel to this value
ALT+SysRq+5Changing Loglevel to this value
ALT+SysRq+6Changing Loglevel to this value
ALT+SysRq+7Changing Loglevel to this value
ALT+SysRq+8Changing Loglevel to this value
ALT+SysRq+9Changing Loglevel to this value
ALT+SysRq+aNot defined
ALT+SysRq+bResetting
ALT+SysRq+cTrigger a crashdump
ALT+SysRq+dNot defined
ALT+SysRq+eTerminate All Tasks
ALT+SysRq+fManual OOM execution
ALT+SysRq+gNot defined
ALT+SysRq+hNot defined
ALT+SysRq+iKill All Tasks
ALT+SysRq+jNot defined
ALT+SysRq+kSAK
ALT+SysRq+lNot defined
ALT+SysRq+mShow Memory
ALT+SysRq+nNice All RT Tasks
ALT+SysRq+oPower Off
ALT+SysRq+pShow Regs
ALT+SysRq+qShow Pending Timers
ALT+SysRq+rKeyboard mode set to XLATE
ALT+SysRq+sEmergency Sync
ALT+SysRq+tShow State
ALT+SysRq+uEmergency Remount R/O
ALT+SysRq+vNot defined
ALT+SysRq+wShow Blocked State
ALT+SysRq+xNot defined
ALT+SysRq+yNot defined
ALT+SysRq+zNot defined
Signed-off-by: Yi Yang <[EMAIL PROTECTED]>
---
sysrq.c | 38 ++
1 file changed, 26 insertions(+), 12 deletions(-)
--- a/drivers/char/sysrq.c 2007-10-16 18:32:58.0 +0800
+++ b/drivers/char/sysrq.c 2007-10-16 23:21:59.0 +0800
@@ -45,6 +45,16 @@ int __read_mostly __sysrq_enabled = 1;
static int __read_mostly sysrq_always_enabled;
+/*
+ * Hot key table SysRq supports
+ */
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
+};
+
int sysrq_on(void)
{
return __sysrq_enabled || sysrq_always_enabled;
@@ -81,7 +91,7 @@ static void sysrq_handle_loglevel(int ke
static struct sysrq_key_op sysrq_loglevel_op = {
.handler= sysrq_handle_loglevel,
.help_msg = "loglevel0-8",
- .action_msg = "Changing Loglevel",
+ .action_msg = "Changing Loglevel to this value",
.enable_mask= SYSRQ_ENABLE_LOG,
};
@@ -410,7 +420,7 @@ void __handle_sysrq(int key, struct tty_
spin_lock_irqsave(_key_table_lock, flags);
orig_log_level = console_loglevel;
console_loglevel = 7;
- printk(KERN_INFO "SysRq : ");
+ printk(KERN_INFO "\nSysRq : ");
op_p = __sysrq_get_key_op(key);
if (op_p) {
@@ -426,18 +436,22 @@ void __handle_sysrq(int key, struct tty_
printk("This sysrq operation is disabled.\n");
}
} else {
- printk("HELP : ");
- /* Only print the help msg once per handler */
+ /*
+* Print SysRq hot key list
+*/
+ printk(KERN_INFO "this hot key isn't defined.\n");
+ printk(KERN_INFO "\nSysRq Help Information:\n");
+ printk(KERN_INFO "\n Hot Key Function Description\n");
+ printk(KERN_INFO "===\n");
for (i = 0; i < ARRAY_SIZE(sysrq_key_table); i++) {
if (sysrq_key_table[i]) {
- int j;
-
- for (j = 0; sysrq_key_table[i] !=
- sysrq_key_table[j]; j++)
- ;
- if (j != i)
-
[PATCH 2.6.23] SysRq: print hotkey info while pressing undef key
For SysRq, we just can get hot key list from Documentation/sysrq.txt
, but in the most of cases, the user can't access it by hand on
using SysRq to debug, so it is better for SysRq to provide an online
help for the users.
SysRq has already provided a similiar help before this patch, but it
is not so definite that the user doesn't know what happened and how
to do on pressing an undefined hot key.
In addition, that funtion has a big loop with another big loop
embedded which is very inefficient, it is intended to skip some hot
key help info for such a function as Changing Loglevel, just print
a help info for this, that is very unnecessary. In fact, the key '0'
- '8' have different results the user should know.
This patch add this online help function, it'll print thw whole hot
key list and corresponding function descriptions, it can print the new
defined hot key without any changed needed.
The output is the below on pressing an undefined hot key:
SysRq : 6this hot key isn't defined.
SysRq Help Information:
Hot Key Function Description
===
ALT+SysRq+0Changing Loglevel to this value
ALT+SysRq+1Changing Loglevel to this value
ALT+SysRq+2Changing Loglevel to this value
ALT+SysRq+3Changing Loglevel to this value
ALT+SysRq+4Changing Loglevel to this value
ALT+SysRq+5Changing Loglevel to this value
ALT+SysRq+6Changing Loglevel to this value
ALT+SysRq+7Changing Loglevel to this value
ALT+SysRq+8Changing Loglevel to this value
ALT+SysRq+9Changing Loglevel to this value
ALT+SysRq+aNot defined
ALT+SysRq+bResetting
ALT+SysRq+cTrigger a crashdump
ALT+SysRq+dNot defined
ALT+SysRq+eTerminate All Tasks
ALT+SysRq+fManual OOM execution
ALT+SysRq+gNot defined
ALT+SysRq+hNot defined
ALT+SysRq+iKill All Tasks
ALT+SysRq+jNot defined
ALT+SysRq+kSAK
ALT+SysRq+lNot defined
ALT+SysRq+mShow Memory
ALT+SysRq+nNice All RT Tasks
ALT+SysRq+oPower Off
ALT+SysRq+pShow Regs
ALT+SysRq+qShow Pending Timers
ALT+SysRq+rKeyboard mode set to XLATE
ALT+SysRq+sEmergency Sync
ALT+SysRq+tShow State
ALT+SysRq+uEmergency Remount R/O
ALT+SysRq+vNot defined
ALT+SysRq+wShow Blocked State
ALT+SysRq+xNot defined
ALT+SysRq+yNot defined
ALT+SysRq+zNot defined
Signed-off-by: Yi Yang [EMAIL PROTECTED]
---
sysrq.c | 38 ++
1 file changed, 26 insertions(+), 12 deletions(-)
--- a/drivers/char/sysrq.c 2007-10-16 18:32:58.0 +0800
+++ b/drivers/char/sysrq.c 2007-10-16 23:21:59.0 +0800
@@ -45,6 +45,16 @@ int __read_mostly __sysrq_enabled = 1;
static int __read_mostly sysrq_always_enabled;
+/*
+ * Hot key table SysRq supports
+ */
+static char __read_mostly sysrq_hot_key_table[36] = {
+ '0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
+ 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j',
+ 'k', 'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't',
+ 'u', 'v', 'w', 'x', 'y', 'z'
+};
+
int sysrq_on(void)
{
return __sysrq_enabled || sysrq_always_enabled;
@@ -81,7 +91,7 @@ static void sysrq_handle_loglevel(int ke
static struct sysrq_key_op sysrq_loglevel_op = {
.handler= sysrq_handle_loglevel,
.help_msg = loglevel0-8,
- .action_msg = Changing Loglevel,
+ .action_msg = Changing Loglevel to this value,
.enable_mask= SYSRQ_ENABLE_LOG,
};
@@ -410,7 +420,7 @@ void __handle_sysrq(int key, struct tty_
spin_lock_irqsave(sysrq_key_table_lock, flags);
orig_log_level = console_loglevel;
console_loglevel = 7;
- printk(KERN_INFO SysRq : );
+ printk(KERN_INFO \nSysRq : );
op_p = __sysrq_get_key_op(key);
if (op_p) {
@@ -426,18 +436,22 @@ void __handle_sysrq(int key, struct tty_
printk(This sysrq operation is disabled.\n);
}
} else {
- printk(HELP : );
- /* Only print the help msg once per handler */
+ /*
+* Print SysRq hot key list
+*/
+ printk(KERN_INFO this hot key isn't defined.\n);
+ printk(KERN_INFO \nSysRq Help Information:\n);
+ printk(KERN_INFO \n Hot Key Function Description\n);
+ printk(KERN_INFO ===\n);
for (i = 0; i ARRAY_SIZE(sysrq_key_table); i++) {
if (sysrq_key_table[i]) {
- int j;
-
- for (j = 0; sysrq_key_table[i] !=
- sysrq_key_table[j]; j++)
- ;
- if (j != i)
- continue;
- printk(%s , sysrq_key_table[i]-help_msg);
+ printk(KERN_INFO ALT+SysRq+%c
