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         0x00000001
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/> Blog


_______________________________________________
linaro-dev mailing list
linaro-dev@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-dev

Reply via email to