Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Monday, December 17, 2012 10:46:07 AM Sivaram Nair wrote: > On Mon, Dec 17, 2012 at 08:56:45AM +0100, Rafael J. Wysocki wrote: > > On Monday, December 17, 2012 09:38:15 AM Sivaram Nair wrote: > > > On Sat, Dec 15, 2012 at 01:03:02AM +0100, Rafael J. Wysocki wrote: > > > > On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote: > > > > > cpuidle_state->power_usage is signed; so change the corresponding > > > > > sysfs > > > > > ops to output signed value instead of unsigned. > > > > > > > > What's actually wrong with printing it as an unsigned int? > > > > > > power_usage could have negative values (for example cpuidle/driver.c > > > inits this value to -1, -2 etc. when drv->power_specified is not set) and > > > these shows up badly in the sysfs output. > > > > Does "badly" mean "as big positive numbers"? > > Yes (sorry for not being clearer). > > > > > Should we actually print them at all in those case? Perhaps it'll be > > better to > > make the file appear empty then? > > May be, but why is power_usage signed in the first place? Please read the comment in driver.c:set_power_states() for the answer. :-) > I also noticed > Daniel Lezcano's patches that reduces the scope of this variable. So, > perhaps we can just ignore this change. OK Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Mon, Dec 17, 2012 at 08:56:45AM +0100, Rafael J. Wysocki wrote: > On Monday, December 17, 2012 09:38:15 AM Sivaram Nair wrote: > > On Sat, Dec 15, 2012 at 01:03:02AM +0100, Rafael J. Wysocki wrote: > > > On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote: > > > > cpuidle_state->power_usage is signed; so change the corresponding sysfs > > > > ops to output signed value instead of unsigned. > > > > > > What's actually wrong with printing it as an unsigned int? > > > > power_usage could have negative values (for example cpuidle/driver.c > > inits this value to -1, -2 etc. when drv->power_specified is not set) and > > these shows up badly in the sysfs output. > > Does "badly" mean "as big positive numbers"? Yes (sorry for not being clearer). > > Should we actually print them at all in those case? Perhaps it'll be better > to > make the file appear empty then? May be, but why is power_usage signed in the first place? I also noticed Daniel Lezcano's patches that reduces the scope of this variable. So, perhaps we can just ignore this change. -Sivaram -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Mon, Dec 17, 2012 at 08:56:45AM +0100, Rafael J. Wysocki wrote: On Monday, December 17, 2012 09:38:15 AM Sivaram Nair wrote: On Sat, Dec 15, 2012 at 01:03:02AM +0100, Rafael J. Wysocki wrote: On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote: cpuidle_state-power_usage is signed; so change the corresponding sysfs ops to output signed value instead of unsigned. What's actually wrong with printing it as an unsigned int? power_usage could have negative values (for example cpuidle/driver.c inits this value to -1, -2 etc. when drv-power_specified is not set) and these shows up badly in the sysfs output. Does badly mean as big positive numbers? Yes (sorry for not being clearer). Should we actually print them at all in those case? Perhaps it'll be better to make the file appear empty then? May be, but why is power_usage signed in the first place? I also noticed Daniel Lezcano's patches that reduces the scope of this variable. So, perhaps we can just ignore this change. -Sivaram -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Monday, December 17, 2012 10:46:07 AM Sivaram Nair wrote: On Mon, Dec 17, 2012 at 08:56:45AM +0100, Rafael J. Wysocki wrote: On Monday, December 17, 2012 09:38:15 AM Sivaram Nair wrote: On Sat, Dec 15, 2012 at 01:03:02AM +0100, Rafael J. Wysocki wrote: On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote: cpuidle_state-power_usage is signed; so change the corresponding sysfs ops to output signed value instead of unsigned. What's actually wrong with printing it as an unsigned int? power_usage could have negative values (for example cpuidle/driver.c inits this value to -1, -2 etc. when drv-power_specified is not set) and these shows up badly in the sysfs output. Does badly mean as big positive numbers? Yes (sorry for not being clearer). Should we actually print them at all in those case? Perhaps it'll be better to make the file appear empty then? May be, but why is power_usage signed in the first place? Please read the comment in driver.c:set_power_states() for the answer. :-) I also noticed Daniel Lezcano's patches that reduces the scope of this variable. So, perhaps we can just ignore this change. OK Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Monday, December 17, 2012 09:38:15 AM Sivaram Nair wrote: > On Sat, Dec 15, 2012 at 01:03:02AM +0100, Rafael J. Wysocki wrote: > > On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote: > > > cpuidle_state->power_usage is signed; so change the corresponding sysfs > > > ops to output signed value instead of unsigned. > > > > What's actually wrong with printing it as an unsigned int? > > power_usage could have negative values (for example cpuidle/driver.c > inits this value to -1, -2 etc. when drv->power_specified is not set) and > these shows up badly in the sysfs output. Does "badly" mean "as big positive numbers"? Should we actually print them at all in those case? Perhaps it'll be better to make the file appear empty then? Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Sat, Dec 15, 2012 at 01:03:02AM +0100, Rafael J. Wysocki wrote: > On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote: > > cpuidle_state->power_usage is signed; so change the corresponding sysfs > > ops to output signed value instead of unsigned. > > What's actually wrong with printing it as an unsigned int? power_usage could have negative values (for example cpuidle/driver.c inits this value to -1, -2 etc. when drv->power_specified is not set) and these shows up badly in the sysfs output. regards, Sivaram -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Sat, Dec 15, 2012 at 01:03:02AM +0100, Rafael J. Wysocki wrote: On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote: cpuidle_state-power_usage is signed; so change the corresponding sysfs ops to output signed value instead of unsigned. What's actually wrong with printing it as an unsigned int? power_usage could have negative values (for example cpuidle/driver.c inits this value to -1, -2 etc. when drv-power_specified is not set) and these shows up badly in the sysfs output. regards, Sivaram -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Monday, December 17, 2012 09:38:15 AM Sivaram Nair wrote: On Sat, Dec 15, 2012 at 01:03:02AM +0100, Rafael J. Wysocki wrote: On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote: cpuidle_state-power_usage is signed; so change the corresponding sysfs ops to output signed value instead of unsigned. What's actually wrong with printing it as an unsigned int? power_usage could have negative values (for example cpuidle/driver.c inits this value to -1, -2 etc. when drv-power_specified is not set) and these shows up badly in the sysfs output. Does badly mean as big positive numbers? Should we actually print them at all in those case? Perhaps it'll be better to make the file appear empty then? Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote:
> cpuidle_state->power_usage is signed; so change the corresponding sysfs
> ops to output signed value instead of unsigned.
What's actually wrong with printing it as an unsigned int?
Rafael
> Signed-off-by: Sivaram Nair
> ---
> drivers/cpuidle/sysfs.c |9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
> index 3409429..2fc79cd 100644
> --- a/drivers/cpuidle/sysfs.c
> +++ b/drivers/cpuidle/sysfs.c
> @@ -232,6 +232,13 @@ static struct cpuidle_state_attr attr_##_name =
> __ATTR(_name, 0644, show, store)
> static ssize_t show_state_##_name(struct cpuidle_state *state, \
>struct cpuidle_state_usage *state_usage, char *buf) \
> { \
> + return sprintf(buf, "%d\n", state->_name);\
> +}
> +
> +#define define_show_state_u_function(_name) \
> +static ssize_t show_state_##_name(struct cpuidle_state *state, \
> + struct cpuidle_state_usage *state_usage, char *buf) \
> +{ \
> return sprintf(buf, "%u\n", state->_name);\
> }
>
> @@ -270,7 +277,7 @@ static ssize_t show_state_##_name(struct cpuidle_state
> *state, \
> return sprintf(buf, "%s\n", state->_name);\
> }
>
> -define_show_state_function(exit_latency)
> +define_show_state_u_function(exit_latency)
> define_show_state_function(power_usage)
> define_show_state_ull_function(usage)
> define_show_state_ull_function(time)
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] cpuidle: fix sysfs output for power_usage
cpuidle_state->power_usage is signed; so change the corresponding sysfs
ops to output signed value instead of unsigned.
Signed-off-by: Sivaram Nair
---
drivers/cpuidle/sysfs.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 3409429..2fc79cd 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -232,6 +232,13 @@ static struct cpuidle_state_attr attr_##_name =
__ATTR(_name, 0644, show, store)
static ssize_t show_state_##_name(struct cpuidle_state *state, \
struct cpuidle_state_usage *state_usage, char *buf) \
{ \
+ return sprintf(buf, "%d\n", state->_name);\
+}
+
+#define define_show_state_u_function(_name) \
+static ssize_t show_state_##_name(struct cpuidle_state *state, \
+struct cpuidle_state_usage *state_usage, char *buf) \
+{ \
return sprintf(buf, "%u\n", state->_name);\
}
@@ -270,7 +277,7 @@ static ssize_t show_state_##_name(struct cpuidle_state
*state, \
return sprintf(buf, "%s\n", state->_name);\
}
-define_show_state_function(exit_latency)
+define_show_state_u_function(exit_latency)
define_show_state_function(power_usage)
define_show_state_ull_function(usage)
define_show_state_ull_function(time)
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] cpuidle: fix sysfs output for power_usage
cpuidle_state-power_usage is signed; so change the corresponding sysfs
ops to output signed value instead of unsigned.
Signed-off-by: Sivaram Nair sivar...@nvidia.com
---
drivers/cpuidle/sysfs.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 3409429..2fc79cd 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -232,6 +232,13 @@ static struct cpuidle_state_attr attr_##_name =
__ATTR(_name, 0644, show, store)
static ssize_t show_state_##_name(struct cpuidle_state *state, \
struct cpuidle_state_usage *state_usage, char *buf) \
{ \
+ return sprintf(buf, %d\n, state-_name);\
+}
+
+#define define_show_state_u_function(_name) \
+static ssize_t show_state_##_name(struct cpuidle_state *state, \
+struct cpuidle_state_usage *state_usage, char *buf) \
+{ \
return sprintf(buf, %u\n, state-_name);\
}
@@ -270,7 +277,7 @@ static ssize_t show_state_##_name(struct cpuidle_state
*state, \
return sprintf(buf, %s\n, state-_name);\
}
-define_show_state_function(exit_latency)
+define_show_state_u_function(exit_latency)
define_show_state_function(power_usage)
define_show_state_ull_function(usage)
define_show_state_ull_function(time)
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] cpuidle: fix sysfs output for power_usage
On Friday, December 14, 2012 03:17:37 PM Sivaram Nair wrote:
cpuidle_state-power_usage is signed; so change the corresponding sysfs
ops to output signed value instead of unsigned.
What's actually wrong with printing it as an unsigned int?
Rafael
Signed-off-by: Sivaram Nair sivar...@nvidia.com
---
drivers/cpuidle/sysfs.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/sysfs.c b/drivers/cpuidle/sysfs.c
index 3409429..2fc79cd 100644
--- a/drivers/cpuidle/sysfs.c
+++ b/drivers/cpuidle/sysfs.c
@@ -232,6 +232,13 @@ static struct cpuidle_state_attr attr_##_name =
__ATTR(_name, 0644, show, store)
static ssize_t show_state_##_name(struct cpuidle_state *state, \
struct cpuidle_state_usage *state_usage, char *buf) \
{ \
+ return sprintf(buf, %d\n, state-_name);\
+}
+
+#define define_show_state_u_function(_name) \
+static ssize_t show_state_##_name(struct cpuidle_state *state, \
+ struct cpuidle_state_usage *state_usage, char *buf) \
+{ \
return sprintf(buf, %u\n, state-_name);\
}
@@ -270,7 +277,7 @@ static ssize_t show_state_##_name(struct cpuidle_state
*state, \
return sprintf(buf, %s\n, state-_name);\
}
-define_show_state_function(exit_latency)
+define_show_state_u_function(exit_latency)
define_show_state_function(power_usage)
define_show_state_ull_function(usage)
define_show_state_ull_function(time)
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
