Re: [PATCH PMQA] cpuidle_killer: allocate per cpu process array dynamically

[Sanjay Singh Rawat]

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