Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically
On Wednesday 07 May 2014 03:34 PM, Mohammad Merajul Islam Molla wrote: Hello Sanjay, One suggestion - its probably better to precede the output lines with cpu no. As currently output from child processes are intermixed as below and difficult to co-relate. jiffies are : 1 usecs found 4 cpu(s) duration: 120 secs, #sleep: 1200, delay: 10 us duration: 120 secs, #sleep: 1200, delay: 10 us duration: 120 secs, #sleep: 1200, delay: 10 us duration: 120 secs, #sleep: 1200, delay: 10 us counter value 4072718528 test duration: 120.057526 secs deviation 0.000479 counter value 3914063589 test duration: 120.057335 secs counter value 4015937716 test duration: 120.057430 secs deviation 0.000479 deviation 0.000478 counter value 411037603 test duration: 120.062716 secs deviation 0.000523 thanks Meraj, done -- Thanks, -Meraj On Fri, May 2, 2014 at 11:23 PM, Sanjay Singh Rawat sanjay.ra...@linaro.org mailto:sanjay.ra...@linaro.org wrote: On Wednesday 30 April 2014 02:04 PM, Mohammad Merajul Islam Molla wrote: Hello Sanjay, As far I know, if option argument is 0, the parent will wait for Looks like waiting is done if childs exit. I checked for the offlined CPU case, if there are no child processes, waitpid returns -1. Setting errno as no child processes specified child pid to terminate, its not for immediate return as in case of WNOHANG. This is probably the intended use of the code (author will be able to confirm). Changing 0 to WNOHANG macro will change the meaning of the code. Also, from header file - #define WNOHANG 0x0001 sorry i referred wrong document WNOHANG is defined as 1, not zero. Which makes me think of two cases below - 1. If the real intended purpose is to not to wait infinitely, replacing [...] -- sanjay ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically
Hello Sanjay, You are welcome. Earlier I submitted some patches in powedebug and powertop tools. Please feel free to merge if you find any useful. http://lists.linaro.org/pipermail/linaro-dev/2014-April/017117.html http://lists.linaro.org/pipermail/linaro-dev/2014-May/017141.html http://lists.linaro.org/pipermail/linaro-dev/2014-May/017144.html http://lists.linaro.org/pipermail/linaro-dev/2014-May/017146.html -- Thanks, -Meraj Mohammad Merajul Islam Molla (Meraj) Mobile Lab 1, Mobile Development Team Samsung RD Institute Bangladesh (SRBD) Phone: +880-1754380207 Skype: mmm2177 On Mon, May 12, 2014 at 4:27 PM, Sanjay Singh Rawat sanjay.ra...@linaro.org wrote: On Wednesday 07 May 2014 03:34 PM, Mohammad Merajul Islam Molla wrote: Hello Sanjay, One suggestion - its probably better to precede the output lines with cpu no. As currently output from child processes are intermixed as below and difficult to co-relate. jiffies are : 1 usecs found 4 cpu(s) duration: 120 secs, #sleep: 1200, delay: 10 us duration: 120 secs, #sleep: 1200, delay: 10 us duration: 120 secs, #sleep: 1200, delay: 10 us duration: 120 secs, #sleep: 1200, delay: 10 us counter value 4072718528 test duration: 120.057526 secs deviation 0.000479 counter value 3914063589 test duration: 120.057335 secs counter value 4015937716 test duration: 120.057430 secs deviation 0.000479 deviation 0.000478 counter value 411037603 test duration: 120.062716 secs deviation 0.000523 thanks Meraj, done -- Thanks, -Meraj On Fri, May 2, 2014 at 11:23 PM, Sanjay Singh Rawat sanjay.ra...@linaro.org mailto:sanjay.ra...@linaro.org wrote: On Wednesday 30 April 2014 02:04 PM, Mohammad Merajul Islam Molla wrote: Hello Sanjay, As far I know, if option argument is 0, the parent will wait for Looks like waiting is done if childs exit. I checked for the offlined CPU case, if there are no child processes, waitpid returns -1. Setting errno as no child processes specified child pid to terminate, its not for immediate return as in case of WNOHANG. This is probably the intended use of the code (author will be able to confirm). Changing 0 to WNOHANG macro will change the meaning of the code. Also, from header file - #define WNOHANG 0x0001 sorry i referred wrong document WNOHANG is defined as 1, not zero. Which makes me think of two cases below - 1. If the real intended purpose is to not to wait infinitely, replacing [...] -- sanjay ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically
Hello Sanjay, One suggestion - its probably better to precede the output lines with cpu no. As currently output from child processes are intermixed as below and difficult to co-relate. jiffies are : 1 usecs found 4 cpu(s) duration: 120 secs, #sleep: 1200, delay: 10 us duration: 120 secs, #sleep: 1200, delay: 10 us duration: 120 secs, #sleep: 1200, delay: 10 us duration: 120 secs, #sleep: 1200, delay: 10 us counter value 4072718528 test duration: 120.057526 secs deviation 0.000479 counter value 3914063589 test duration: 120.057335 secs counter value 4015937716 test duration: 120.057430 secs deviation 0.000479 deviation 0.000478 counter value 411037603 test duration: 120.062716 secs deviation 0.000523 -- Thanks, -Meraj On Fri, May 2, 2014 at 11:23 PM, Sanjay Singh Rawat sanjay.ra...@linaro.org wrote: On Wednesday 30 April 2014 02:04 PM, Mohammad Merajul Islam Molla wrote: Hello Sanjay, As far I know, if option argument is 0, the parent will wait for Looks like waiting is done if childs exit. I checked for the offlined CPU case, if there are no child processes, waitpid returns -1. Setting errno as no child processes specified child pid to terminate, its not for immediate return as in case of WNOHANG. This is probably the intended use of the code (author will be able to confirm). Changing 0 to WNOHANG macro will change the meaning of the code. Also, from header file - #define WNOHANG 0x0001 sorry i referred wrong document WNOHANG is defined as 1, not zero. Which makes me think of two cases below - 1. If the real intended purpose is to not to wait infinitely, replacing [...] ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically
On 04/30/2014 10:34 AM, Mohammad Merajul Islam Molla wrote:
Hello Sanjay,
As far I know, if option argument is 0, the parent will wait for
specified child pid to terminate, its not for immediate return as in
case of WNOHANG. This is probably the intended use of the code (author
will be able to confirm). Changing 0 to WNOHANG macro will change the
meaning of the code.
Right.
Also, from header file -
#define WNOHANG 0x0001
WNOHANG is defined as 1, not zero.
Which makes me think of two cases below -
1. If the real intended purpose is to not to wait infinitely, replacing
0 with WNOHANG macro will do.
2. But if the real intended purpose is to wait infinitely, then the
solution I proposed earlier should be real fix, i.e.,
if (pids[i] != 0)
waitpid(pids[i], status, 0); [ and keeping options
argument as 0]
--
There is an alternative if you want to simplify the code.
Instead of using nrcpus and waitpid, use *wait*.
for (;;) {
pid_t pid = wait(status);
if (pid 0 errno == ECHILD)
break;
if (status != 0)
fprintf(stderr, blabla \n);
}
Thanks,
-Meraj
On Wed, Apr 30, 2014 at 1:25 PM, Sanjay Singh Rawat
sanjay.ra...@linaro.org mailto:sanjay.ra...@linaro.org wrote:
On Wednesday 30 April 2014 12:14 PM, Mohammad Merajul Islam Molla wrote:
Hello,
I would like to share two observations -
1. Is it necessary to initialize nrcpus = 2 anymore?
thanks, ack
2. Another problem may happen in the code below where waitpid is
called -
for (i = 0; i nrcpus; i++) {
int status;
waitpid(pids[i], status, 0);
if (status != 0) {
fprintf(stderr, test for cpu %d has
failed\n, i);
ret = 1;
}
}
Since for offline cpus, no child process is created, now
these cpus
pid[i]'s will be zero (due to calloc). This will change the
meaning of
waitpid function as man page says -
pid 0 -meaning wait for any child process whose
process group
ID is equal to that of the calling process.
I think a check should be added before waitpid call -
if (pids[i] != 0)
waitpid(pids[i], status, 0);
Here Parent will not wait for child infinitely if status is not
visible, the option argument is 0(NOHANG). I will add the macro for
readability. thanks
--
Thanks,
-Meraj
On Wed, Apr 30, 2014 at 11:08 AM, Sanjay Singh Rawat
sanjay.ra...@linaro.org mailto:sanjay.ra...@linaro.org
mailto:sanjay.rawat@linaro.__org
mailto:sanjay.ra...@linaro.org wrote:
currently percpu process array is set to 2, which results
in segfault
Signed-off-by: Sanjay Singh Rawat sanjay.ra...@linaro.org
mailto:sanjay.ra...@linaro.org
mailto:sanjay.rawat@linaro.__org
mailto:sanjay.ra...@linaro.org
---
cpuidle/cpuidle_killer.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/cpuidle/cpuidle_killer.c
b/cpuidle/cpuidle_killer.c
index 5e7320f..09009ef 100644
--- a/cpuidle/cpuidle_killer.c
+++ b/cpuidle/cpuidle_killer.c
@@ -104,7 +104,7 @@ int main(int argc, char *argv[])
{
int ret, i, nrcpus = 2;
int nrsleeps, delay;
- pid_t pids[nrcpus];
+ pid_t *pids;
struct timex timex = { 0 };
if (adjtimex(timex) 0) {
@@ -121,6 +121,11 @@ int main(int argc, char *argv[])
}
fprintf(stderr, found %d cpu(s)\n, nrcpus);
+ pids = (pid_t *) calloc(nrcpus, sizeof(pid_t));
+ if (pids == NULL) {
+ fprintf(stderr, error: calloc failed\n);
+ return 1;
+ }
for (i = 0; i nrcpus; i++) {
--
1.7.10.4
_
linaro-dev mailing list
linaro-dev@lists.linaro.org mailto:linaro-dev@lists.linaro.org
mailto:linaro-dev@lists.__linaro.org
mailto:linaro-dev@lists.linaro.org
http://lists.linaro.org/__mailman/listinfo/linaro-dev
http://lists.linaro.org/mailman/listinfo/linaro-dev
--
sanjay
--
http://www.linaro.org/ Linaro.org │ Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro Facebook |
http://twitter.com/#!/linaroorg Twitter |
http://www.linaro.org/linaro-blog/
Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically
On Wednesday 30 April 2014 02:04 PM, Mohammad Merajul Islam Molla wrote: Hello Sanjay, As far I know, if option argument is 0, the parent will wait for Looks like waiting is done if childs exit. I checked for the offlined CPU case, if there are no child processes, waitpid returns -1. Setting errno as no child processes specified child pid to terminate, its not for immediate return as in case of WNOHANG. This is probably the intended use of the code (author will be able to confirm). Changing 0 to WNOHANG macro will change the meaning of the code. Also, from header file - #define WNOHANG 0x0001 sorry i referred wrong document WNOHANG is defined as 1, not zero. Which makes me think of two cases below - 1. If the real intended purpose is to not to wait infinitely, replacing [...] ___ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically
Hello,
I would like to share two observations -
1. Is it necessary to initialize nrcpus = 2 anymore?
2. Another problem may happen in the code below where waitpid is called -
for (i = 0; i nrcpus; i++) {
int status;
waitpid(pids[i], status, 0);
if (status != 0) {
fprintf(stderr, test for cpu %d has failed\n, i);
ret = 1;
}
}
Since for offline cpus, no child process is created, now these cpus
pid[i]'s will be zero (due to calloc). This will change the meaning of
waitpid function as man page says -
pid 0 -meaning wait for any child process whose process group ID
is equal to that of the calling process.
I think a check should be added before waitpid call -
if (pids[i] != 0)
waitpid(pids[i], status, 0);
--
Thanks,
-Meraj
On Wed, Apr 30, 2014 at 11:08 AM, Sanjay Singh Rawat
sanjay.ra...@linaro.org wrote:
currently percpu process array is set to 2, which results in segfault
Signed-off-by: Sanjay Singh Rawat sanjay.ra...@linaro.org
---
cpuidle/cpuidle_killer.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/cpuidle/cpuidle_killer.c b/cpuidle/cpuidle_killer.c
index 5e7320f..09009ef 100644
--- a/cpuidle/cpuidle_killer.c
+++ b/cpuidle/cpuidle_killer.c
@@ -104,7 +104,7 @@ int main(int argc, char *argv[])
{
int ret, i, nrcpus = 2;
int nrsleeps, delay;
- pid_t pids[nrcpus];
+ pid_t *pids;
struct timex timex = { 0 };
if (adjtimex(timex) 0) {
@@ -121,6 +121,11 @@ int main(int argc, char *argv[])
}
fprintf(stderr, found %d cpu(s)\n, nrcpus);
+ pids = (pid_t *) calloc(nrcpus, sizeof(pid_t));
+ if (pids == NULL) {
+ fprintf(stderr, error: calloc failed\n);
+ return 1;
+ }
for (i = 0; i nrcpus; i++) {
--
1.7.10.4
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically
On Wednesday 30 April 2014 12:14 PM, Mohammad Merajul Islam Molla wrote:
Hello,
I would like to share two observations -
1. Is it necessary to initialize nrcpus = 2 anymore?
thanks, ack
2. Another problem may happen in the code below where waitpid is called -
for (i = 0; i nrcpus; i++) {
int status;
waitpid(pids[i], status, 0);
if (status != 0) {
fprintf(stderr, test for cpu %d has failed\n, i);
ret = 1;
}
}
Since for offline cpus, no child process is created, now these cpus
pid[i]'s will be zero (due to calloc). This will change the meaning of
waitpid function as man page says -
pid 0 -meaning wait for any child process whose process group
ID is equal to that of the calling process.
I think a check should be added before waitpid call -
if (pids[i] != 0)
waitpid(pids[i], status, 0);
Here Parent will not wait for child infinitely if status is not visible,
the option argument is 0(NOHANG). I will add the macro for readability.
thanks
--
Thanks,
-Meraj
On Wed, Apr 30, 2014 at 11:08 AM, Sanjay Singh Rawat
sanjay.ra...@linaro.org mailto:sanjay.ra...@linaro.org wrote:
currently percpu process array is set to 2, which results in segfault
Signed-off-by: Sanjay Singh Rawat sanjay.ra...@linaro.org
mailto:sanjay.ra...@linaro.org
---
cpuidle/cpuidle_killer.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/cpuidle/cpuidle_killer.c b/cpuidle/cpuidle_killer.c
index 5e7320f..09009ef 100644
--- a/cpuidle/cpuidle_killer.c
+++ b/cpuidle/cpuidle_killer.c
@@ -104,7 +104,7 @@ int main(int argc, char *argv[])
{
int ret, i, nrcpus = 2;
int nrsleeps, delay;
- pid_t pids[nrcpus];
+ pid_t *pids;
struct timex timex = { 0 };
if (adjtimex(timex) 0) {
@@ -121,6 +121,11 @@ int main(int argc, char *argv[])
}
fprintf(stderr, found %d cpu(s)\n, nrcpus);
+ pids = (pid_t *) calloc(nrcpus, sizeof(pid_t));
+ if (pids == NULL) {
+ fprintf(stderr, error: calloc failed\n);
+ return 1;
+ }
for (i = 0; i nrcpus; i++) {
--
1.7.10.4
___
linaro-dev mailing list
linaro-dev@lists.linaro.org mailto:linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
--
sanjay
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically
Hello Sanjay,
As far I know, if option argument is 0, the parent will wait for
specified child pid to terminate, its not for immediate return as in case
of WNOHANG. This is probably the intended use of the code (author will be
able to confirm). Changing 0 to WNOHANG macro will change the meaning of
the code.
Also, from header file -
#define WNOHANG 0x0001
WNOHANG is defined as 1, not zero.
Which makes me think of two cases below -
1. If the real intended purpose is to not to wait infinitely, replacing 0
with WNOHANG macro will do.
2. But if the real intended purpose is to wait infinitely, then the
solution I proposed earlier should be real fix, i.e.,
if (pids[i] != 0)
waitpid(pids[i], status, 0); [ and keeping options argument
as 0]
--
Thanks,
-Meraj
On Wed, Apr 30, 2014 at 1:25 PM, Sanjay Singh Rawat sanjay.ra...@linaro.org
wrote:
On Wednesday 30 April 2014 12:14 PM, Mohammad Merajul Islam Molla wrote:
Hello,
I would like to share two observations -
1. Is it necessary to initialize nrcpus = 2 anymore?
thanks, ack
2. Another problem may happen in the code below where waitpid is called -
for (i = 0; i nrcpus; i++) {
int status;
waitpid(pids[i], status, 0);
if (status != 0) {
fprintf(stderr, test for cpu %d has failed\n,
i);
ret = 1;
}
}
Since for offline cpus, no child process is created, now these cpus
pid[i]'s will be zero (due to calloc). This will change the meaning of
waitpid function as man page says -
pid 0 -meaning wait for any child process whose process group
ID is equal to that of the calling process.
I think a check should be added before waitpid call -
if (pids[i] != 0)
waitpid(pids[i], status, 0);
Here Parent will not wait for child infinitely if status is not visible,
the option argument is 0(NOHANG). I will add the macro for readability.
thanks
--
Thanks,
-Meraj
On Wed, Apr 30, 2014 at 11:08 AM, Sanjay Singh Rawat
sanjay.ra...@linaro.org mailto:sanjay.ra...@linaro.org wrote:
currently percpu process array is set to 2, which results in segfault
Signed-off-by: Sanjay Singh Rawat sanjay.ra...@linaro.org
mailto:sanjay.ra...@linaro.org
---
cpuidle/cpuidle_killer.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/cpuidle/cpuidle_killer.c b/cpuidle/cpuidle_killer.c
index 5e7320f..09009ef 100644
--- a/cpuidle/cpuidle_killer.c
+++ b/cpuidle/cpuidle_killer.c
@@ -104,7 +104,7 @@ int main(int argc, char *argv[])
{
int ret, i, nrcpus = 2;
int nrsleeps, delay;
- pid_t pids[nrcpus];
+ pid_t *pids;
struct timex timex = { 0 };
if (adjtimex(timex) 0) {
@@ -121,6 +121,11 @@ int main(int argc, char *argv[])
}
fprintf(stderr, found %d cpu(s)\n, nrcpus);
+ pids = (pid_t *) calloc(nrcpus, sizeof(pid_t));
+ if (pids == NULL) {
+ fprintf(stderr, error: calloc failed\n);
+ return 1;
+ }
for (i = 0; i nrcpus; i++) {
--
1.7.10.4
___
linaro-dev mailing list
linaro-dev@lists.linaro.org mailto:linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
--
sanjay
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
[PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically
currently percpu process array is set to 2, which results in segfault
Signed-off-by: Sanjay Singh Rawat sanjay.ra...@linaro.org
---
cpuidle/cpuidle_killer.c |7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/cpuidle/cpuidle_killer.c b/cpuidle/cpuidle_killer.c
index 5e7320f..09009ef 100644
--- a/cpuidle/cpuidle_killer.c
+++ b/cpuidle/cpuidle_killer.c
@@ -104,7 +104,7 @@ int main(int argc, char *argv[])
{
int ret, i, nrcpus = 2;
int nrsleeps, delay;
- pid_t pids[nrcpus];
+ pid_t *pids;
struct timex timex = { 0 };
if (adjtimex(timex) 0) {
@@ -121,6 +121,11 @@ int main(int argc, char *argv[])
}
fprintf(stderr, found %d cpu(s)\n, nrcpus);
+ pids = (pid_t *) calloc(nrcpus, sizeof(pid_t));
+ if (pids == NULL) {
+ fprintf(stderr, error: calloc failed\n);
+ return 1;
+ }
for (i = 0; i nrcpus; i++) {
--
1.7.10.4
___
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev
