| Message ID | 20190319093858.584-1-liwang@redhat.com |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC,1/3] min_free_kbytes: Fix child exit status check conditions | expand |
----- Original Message ----- > From: Vipin K Parashar <vipin@linux.vnet.ibm.com> > > Fixes: #349 > > min_free_kbytes test has badly formed if conditions in mem_tune() > for child exit status check. Can you please elaborate what's wrong about it? > This is causing test to declare as FAILED > despite that not being the case. Why? Shouldn't mmap succeed with overcommit == 1? > Fix child exit status check conditions. > > ---ERROR LOG--- > <..snip..> > mem.c:839: INFO: set overcommit_memory to 1 > mem.c:839: INFO: set min_free_kbytes to 11580 > memfree is 6974720 kB before eatup mem > memfree is 15488 kB after eatup mem > min_free_kbytes.c:135: FAIL: child unexpectedly failed: 0 > mem.c:839: INFO: set min_free_kbytes to 23160 > memfree is 7104128 kB before eatup mem > memfree is 26560 kB after eatup mem > min_free_kbytes.c:135: FAIL: child unexpectedly failed: 0 > mem.c:839: INFO: set min_free_kbytes to 145812 > memfree is 7101504 kB before eatup mem > memfree is 215872 kB after eatup mem > min_free_kbytes.c:135: FAIL: child unexpectedly failed: 0 > min_free_kbytes.c:81: PASS: min_free_kbytes test pass > mem.c:839: INFO: set min_free_kbytes to 11580 > --------------- > > Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com> > Signed-off-by: Li Wang <liwang@redhat.com> > Cc: Jan Stancek <jstancek@redhat.com> > --- > .../kernel/mem/tunable/min_free_kbytes.c | 40 +++++-------------- > 1 file changed, 11 insertions(+), 29 deletions(-) > > diff --git a/testcases/kernel/mem/tunable/min_free_kbytes.c > b/testcases/kernel/mem/tunable/min_free_kbytes.c > index f114dc493..d2378a700 100644 > --- a/testcases/kernel/mem/tunable/min_free_kbytes.c > +++ b/testcases/kernel/mem/tunable/min_free_kbytes.c > @@ -119,39 +119,21 @@ static void test_tune(unsigned long overcommit_policy) > > SAFE_WAITPID(pid[i], &status, WUNTRACED | WCONTINUED); > > - if (overcommit_policy == 2) { > - if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) > - tst_res(TFAIL, > - "child unexpectedly failed: %d", > - status); > - } else if (overcommit_policy == 1) { > - if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL) > + if (WIFEXITED(status) && WEXITSTATUS(status) != 0) { > + tst_res(TFAIL, > + "child unexpectedly failed: %d", status); > + } else if (WIFSIGNALED(status) && WTERMSIG(status) != SIGKILL) { > #if __WORDSIZE == 32 > - { > - if (total_mem < 3145728UL) > + if (total_mem < 3145728UL) > #endif > - tst_res(TFAIL, > - "child unexpectedly failed: %d", > - status); > + tst_res(TFAIL, > + "child unexpectedly failed: %d", status); > #if __WORDSIZE == 32 > - /* in 32-bit system, a process allocate about 3Gb memory at most */ > - else > - tst_res(TINFO, "Child can't allocate " > - ">3Gb memory in 32bit system"); > - } > + /* in 32-bit system, a process allocate about 3Gb memory at most */ > + else > + tst_res(TINFO, "Child can't allocate " > + ">3Gb memory in 32bit system"); > #endif > - } else { > - if (WIFEXITED(status)) { > - if (WEXITSTATUS(status) != 0) { > - tst_res(TFAIL, "child unexpectedly " > - "failed: %d", status); > - } > - } else if (!WIFSIGNALED(status) || > - WTERMSIG(status) != SIGKILL) { > - tst_res(TFAIL, > - "child unexpectedly failed: %d", > - status); > - } > } > } > } > -- > 2.20.1 > >
@Vipin, is there any possible you can have a look Jan's question? On Wed, Mar 20, 2019 at 12:30 AM Jan Stancek <jstancek@redhat.com> wrote: > > > ----- Original Message ----- > > From: Vipin K Parashar <vipin@linux.vnet.ibm.com> > > > > Fixes: #349 > > > > min_free_kbytes test has badly formed if conditions in mem_tune() > > for child exit status check. > > Can you please elaborate what's wrong about it? > I‘m not sure if Vipin could help to explain this. I just nocited that this pull request was pending there more than half year, so I re-send this patch to ML for more discussion. https://github.com/linux-test-project/ltp/pull/350 > > This is causing test to declare as FAILED > > despite that not being the case. > > Why? Shouldn't mmap succeed with overcommit == 1? > I was unable to reproduce this. But AFAIK, 'overcommit == 1' cann't guarantee the MAP_FAILED(with ENOMEM) before oom. So I roughly made change in the original patch to catch up both oom and map_failed scenario for any 'overcommit'. Beside that, as I was pointed out in patch 3/3, this test has an obviously defect in design, I'm also not sure if MemFree + (1/10 *min_free_kbytes) is safty enough, but that's only what I can do for the case remedy. > > Fix child exit status check conditions. > > > > ---ERROR LOG--- > > <..snip..> > > mem.c:839: INFO: set overcommit_memory to 1 > > mem.c:839: INFO: set min_free_kbytes to 11580 > > memfree is 6974720 kB before eatup mem > > memfree is 15488 kB after eatup mem > > min_free_kbytes.c:135: FAIL: child unexpectedly failed: 0 > > mem.c:839: INFO: set min_free_kbytes to 23160 > > memfree is 7104128 kB before eatup mem > > memfree is 26560 kB after eatup mem > > min_free_kbytes.c:135: FAIL: child unexpectedly failed: 0 > > mem.c:839: INFO: set min_free_kbytes to 145812 > > memfree is 7101504 kB before eatup mem > > memfree is 215872 kB after eatup mem > > min_free_kbytes.c:135: FAIL: child unexpectedly failed: 0 > > min_free_kbytes.c:81: PASS: min_free_kbytes test pass > > mem.c:839: INFO: set min_free_kbytes to 11580 > > --------------- > > > > Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com> > > Signed-off-by: Li Wang <liwang@redhat.com> > > Cc: Jan Stancek <jstancek@redhat.com> > > --- > > .../kernel/mem/tunable/min_free_kbytes.c | 40 +++++-------------- > > 1 file changed, 11 insertions(+), 29 deletions(-) > > > > diff --git a/testcases/kernel/mem/tunable/min_free_kbytes.c > > b/testcases/kernel/mem/tunable/min_free_kbytes.c > > index f114dc493..d2378a700 100644 > > --- a/testcases/kernel/mem/tunable/min_free_kbytes.c > > +++ b/testcases/kernel/mem/tunable/min_free_kbytes.c > > @@ -119,39 +119,21 @@ static void test_tune(unsigned long > overcommit_policy) > > > > SAFE_WAITPID(pid[i], &status, WUNTRACED | WCONTINUED); > > > > - if (overcommit_policy == 2) { > > - if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) > > - tst_res(TFAIL, > > - "child unexpectedly failed: %d", > > - status); > > - } else if (overcommit_policy == 1) { > > - if (!WIFSIGNALED(status) || WTERMSIG(status) != > SIGKILL) > > + if (WIFEXITED(status) && WEXITSTATUS(status) != 0) { > > + tst_res(TFAIL, > > + "child unexpectedly failed: %d", status); > > + } else if (WIFSIGNALED(status) && WTERMSIG(status) != > SIGKILL) { > > #if __WORDSIZE == 32 > > - { > > - if (total_mem < 3145728UL) > > + if (total_mem < 3145728UL) > > #endif > > - tst_res(TFAIL, > > - "child unexpectedly > failed: %d", > > - status); > > + tst_res(TFAIL, > > + "child unexpectedly failed: %d", > status); > > #if __WORDSIZE == 32 > > - /* in 32-bit system, a process allocate > about 3Gb memory at most */ > > - else > > - tst_res(TINFO, "Child can't > allocate " > > - ">3Gb memory in 32bit > system"); > > - } > > + /* in 32-bit system, a process allocate about 3Gb > memory at most */ > > + else > > + tst_res(TINFO, "Child can't allocate " > > + ">3Gb memory in 32bit system"); > > #endif > > - } else { > > - if (WIFEXITED(status)) { > > - if (WEXITSTATUS(status) != 0) { > > - tst_res(TFAIL, "child unexpectedly > " > > - "failed: %d", status); > > - } > > - } else if (!WIFSIGNALED(status) || > > - WTERMSIG(status) != SIGKILL) { > > - tst_res(TFAIL, > > - "child unexpectedly failed: %d", > > - status); > > - } > > } > > } > > } > > -- > > 2.20.1 > > > > >
----- Original Message ----- > @Vipin, is there any possible you can have a look Jan's question? > > On Wed, Mar 20, 2019 at 12:30 AM Jan Stancek <jstancek@redhat.com> wrote: > > > > > > > ----- Original Message ----- > > > From: Vipin K Parashar <vipin@linux.vnet.ibm.com> > > > > > > Fixes: #349 > > > > > > min_free_kbytes test has badly formed if conditions in mem_tune() > > > for child exit status check. > > > > Can you please elaborate what's wrong about it? > > > > I‘m not sure if Vipin could help to explain this. I just nocited that this > pull request was pending there more than half year, so I re-send this patch > to ML for more discussion. > https://github.com/linux-test-project/ltp/pull/350 > > > > > This is causing test to declare as FAILED > > > despite that not being the case. > > > > Why? Shouldn't mmap succeed with overcommit == 1? > > > > I was unable to reproduce this. But AFAIK, 'overcommit == 1' cann't > guarantee the MAP_FAILED(with ENOMEM) before oom. With 'overcommit == 1' test always expects OOM. Test output below suggests that child ended normally, because mmap failed. > So I roughly made change > in the original patch to catch up both oom and map_failed scenario for any > 'overcommit'. > > Beside that, as I was pointed out in patch 3/3, this test has an obviously > defect in design, I'm also not sure if MemFree + (1/10 *min_free_kbytes) is > safty enough, but that's only what I can do for the case remedy. > > > > > Fix child exit status check conditions. > > > > > > ---ERROR LOG--- > > > <..snip..> > > > mem.c:839: INFO: set overcommit_memory to 1 > > > mem.c:839: INFO: set min_free_kbytes to 11580 > > > memfree is 6974720 kB before eatup mem > > > memfree is 15488 kB after eatup mem > > > min_free_kbytes.c:135: FAIL: child unexpectedly failed: 0 > > > mem.c:839: INFO: set min_free_kbytes to 23160 > > > memfree is 7104128 kB before eatup mem > > > memfree is 26560 kB after eatup mem > > > min_free_kbytes.c:135: FAIL: child unexpectedly failed: 0 > > > mem.c:839: INFO: set min_free_kbytes to 145812 > > > memfree is 7101504 kB before eatup mem > > > memfree is 215872 kB after eatup mem > > > min_free_kbytes.c:135: FAIL: child unexpectedly failed: 0 > > > min_free_kbytes.c:81: PASS: min_free_kbytes test pass > > > mem.c:839: INFO: set min_free_kbytes to 11580 > > > --------------- > > > > > > Signed-off-by: Vipin K Parashar <vipin@linux.vnet.ibm.com> > > > Signed-off-by: Li Wang <liwang@redhat.com> > > > Cc: Jan Stancek <jstancek@redhat.com> > > > --- > > > .../kernel/mem/tunable/min_free_kbytes.c | 40 +++++-------------- > > > 1 file changed, 11 insertions(+), 29 deletions(-) > > > > > > diff --git a/testcases/kernel/mem/tunable/min_free_kbytes.c > > > b/testcases/kernel/mem/tunable/min_free_kbytes.c > > > index f114dc493..d2378a700 100644 > > > --- a/testcases/kernel/mem/tunable/min_free_kbytes.c > > > +++ b/testcases/kernel/mem/tunable/min_free_kbytes.c > > > @@ -119,39 +119,21 @@ static void test_tune(unsigned long > > overcommit_policy) > > > > > > SAFE_WAITPID(pid[i], &status, WUNTRACED | WCONTINUED); > > > > > > - if (overcommit_policy == 2) { > > > - if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) > > > - tst_res(TFAIL, > > > - "child unexpectedly failed: %d", > > > - status); > > > - } else if (overcommit_policy == 1) { > > > - if (!WIFSIGNALED(status) || WTERMSIG(status) != > > SIGKILL) > > > + if (WIFEXITED(status) && WEXITSTATUS(status) != 0) { > > > + tst_res(TFAIL, > > > + "child unexpectedly failed: %d", status); > > > + } else if (WIFSIGNALED(status) && WTERMSIG(status) != > > SIGKILL) { > > > #if __WORDSIZE == 32 > > > - { > > > - if (total_mem < 3145728UL) > > > + if (total_mem < 3145728UL) > > > #endif > > > - tst_res(TFAIL, > > > - "child unexpectedly > > failed: %d", > > > - status); > > > + tst_res(TFAIL, > > > + "child unexpectedly failed: %d", > > status); > > > #if __WORDSIZE == 32 > > > - /* in 32-bit system, a process allocate > > about 3Gb memory at most */ > > > - else > > > - tst_res(TINFO, "Child can't > > allocate " > > > - ">3Gb memory in 32bit > > system"); > > > - } > > > + /* in 32-bit system, a process allocate about 3Gb > > memory at most */ > > > + else > > > + tst_res(TINFO, "Child can't allocate " > > > + ">3Gb memory in 32bit system"); > > > #endif > > > - } else { > > > - if (WIFEXITED(status)) { > > > - if (WEXITSTATUS(status) != 0) { > > > - tst_res(TFAIL, "child unexpectedly > > " > > > - "failed: %d", status); > > > - } > > > - } else if (!WIFSIGNALED(status) || > > > - WTERMSIG(status) != SIGKILL) { > > > - tst_res(TFAIL, > > > - "child unexpectedly failed: %d", > > > - status); > > > - } > > > } > > > } > > > } > > > -- > > > 2.20.1 > > > > > > > > > > > -- > Regards, > Li Wang >
diff --git a/testcases/kernel/mem/tunable/min_free_kbytes.c b/testcases/kernel/mem/tunable/min_free_kbytes.c index f114dc493..d2378a700 100644 --- a/testcases/kernel/mem/tunable/min_free_kbytes.c +++ b/testcases/kernel/mem/tunable/min_free_kbytes.c @@ -119,39 +119,21 @@ static void test_tune(unsigned long overcommit_policy) SAFE_WAITPID(pid[i], &status, WUNTRACED | WCONTINUED); - if (overcommit_policy == 2) { - if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) - tst_res(TFAIL, - "child unexpectedly failed: %d", - status); - } else if (overcommit_policy == 1) { - if (!WIFSIGNALED(status) || WTERMSIG(status) != SIGKILL) + if (WIFEXITED(status) && WEXITSTATUS(status) != 0) { + tst_res(TFAIL, + "child unexpectedly failed: %d", status); + } else if (WIFSIGNALED(status) && WTERMSIG(status) != SIGKILL) { #if __WORDSIZE == 32 - { - if (total_mem < 3145728UL) + if (total_mem < 3145728UL) #endif - tst_res(TFAIL, - "child unexpectedly failed: %d", - status); + tst_res(TFAIL, + "child unexpectedly failed: %d", status); #if __WORDSIZE == 32 - /* in 32-bit system, a process allocate about 3Gb memory at most */ - else - tst_res(TINFO, "Child can't allocate " - ">3Gb memory in 32bit system"); - } + /* in 32-bit system, a process allocate about 3Gb memory at most */ + else + tst_res(TINFO, "Child can't allocate " + ">3Gb memory in 32bit system"); #endif - } else { - if (WIFEXITED(status)) { - if (WEXITSTATUS(status) != 0) { - tst_res(TFAIL, "child unexpectedly " - "failed: %d", status); - } - } else if (!WIFSIGNALED(status) || - WTERMSIG(status) != SIGKILL) { - tst_res(TFAIL, - "child unexpectedly failed: %d", - status); - } } } }