| Message ID | 87e6761eb699c7912e2064dea222f5ac7fd04a6b.1581338640.git.jstancek@redhat.com |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | syscalls/setrlimit06: lower RLIMIT_CPU parameters | expand |
Hi Jan, It looks good to me and thanks a lot for your improvement. :-) Reviewed-by: Xiao Yang <ice_yangxiao@163.com> Best Regards, Xiao Yang On 2/10/20 8:47 PM, Jan Stancek wrote: > Lower the parameters so that test completes faster where possible. > > This also increases alarm timer slightly, which in combination with > lower RLIMIT_CPU aims to avoid false positives in environments with > high steal time, where it can take multiple of wall clock seconds > to spend single second on a cpu. > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > --- > testcases/kernel/syscalls/setrlimit/setrlimit06.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit06.c b/testcases/kernel/syscalls/setrlimit/setrlimit06.c > index 726b26841583..3e5bf1d4253d 100644 > --- a/testcases/kernel/syscalls/setrlimit/setrlimit06.c > +++ b/testcases/kernel/syscalls/setrlimit/setrlimit06.c > @@ -59,8 +59,8 @@ static void verify_setrlimit(void) > pid = SAFE_FORK(); > if (!pid) { > struct rlimit rlim = { > - .rlim_cur = 2, > - .rlim_max = 3, > + .rlim_cur = 1, > + .rlim_max = 2, > }; > > TEST(setrlimit(RLIMIT_CPU, &rlim)); > @@ -70,7 +70,7 @@ static void verify_setrlimit(void) > exit(1); > } > > - alarm(10); > + alarm(20); > > while (1); > }
On Mon, Feb 10, 2020 at 8:47 PM Jan Stancek <jstancek@redhat.com> wrote: > Lower the parameters so that test completes faster where possible. > > This also increases alarm timer slightly, which in combination with > lower RLIMIT_CPU aims to avoid false positives in environments with > high steal time, where it can take multiple of wall clock seconds > to spend single second on a cpu. > This patch could reduce the test failure possibility, but I'm afraid it can't fix the problem radically, because with `stress -c 20' to overload an s390x system(2cpus) in the background then setrlimit06(patched) still easily gets failed: setrlimit06.c:98: FAIL: Got only SIGXCPU after reaching both limit Another way I can think of is to raise the priority before its running, not sure if that will disturb the original test but from my test, it always gets a pass even with too much overload. --- a/testcases/kernel/syscalls/setrlimit/setrlimit06.c +++ b/testcases/kernel/syscalls/setrlimit/setrlimit06.c @@ -25,6 +25,7 @@ #include <sys/wait.h> #include <stdlib.h> #include <sys/mman.h> +#include <sys/resource.h> #include "tst_test.h" @@ -37,6 +38,8 @@ static void sighandler(int sig) static void setup(void) { + setpriority(PRIO_PROCESS, 0, -20); + SAFE_SIGNAL(SIGXCPU, sighandler); end = SAFE_MMAP(NULL, sizeof(int), PROT_READ | PROT_WRITE, @@ -110,6 +113,7 @@ static void verify_setrlimit(void) } static struct tst_test test = { + .needs_root = 1, .test_all = verify_setrlimit, .setup = setup, .cleanup = cleanup,
----- Original Message ----- > On Mon, Feb 10, 2020 at 8:47 PM Jan Stancek <jstancek@redhat.com> wrote: > > > Lower the parameters so that test completes faster where possible. > > > > This also increases alarm timer slightly, which in combination with > > lower RLIMIT_CPU aims to avoid false positives in environments with > > high steal time, where it can take multiple of wall clock seconds > > to spend single second on a cpu. > > > > This patch could reduce the test failure possibility, but I'm afraid it > can't fix the problem radically, because with `stress -c 20' to overload an > s390x system(2cpus) in the background then setrlimit06(patched) still > easily gets failed: > setrlimit06.c:98: FAIL: Got only SIGXCPU after reaching both limit > > Another way I can think of is to raise the priority before its running, not > sure if that will disturb the original test but from my test, it always > gets a pass even with too much overload. Is this in addition to my patch? Because on its own I don't see how this will help when load is coming from different guests. > > --- a/testcases/kernel/syscalls/setrlimit/setrlimit06.c > +++ b/testcases/kernel/syscalls/setrlimit/setrlimit06.c > @@ -25,6 +25,7 @@ > #include <sys/wait.h> > #include <stdlib.h> > #include <sys/mman.h> > +#include <sys/resource.h> > > #include "tst_test.h" > > @@ -37,6 +38,8 @@ static void sighandler(int sig) > > static void setup(void) > { > + setpriority(PRIO_PROCESS, 0, -20); > + > SAFE_SIGNAL(SIGXCPU, sighandler); > > end = SAFE_MMAP(NULL, sizeof(int), PROT_READ | PROT_WRITE, > @@ -110,6 +113,7 @@ static void verify_setrlimit(void) > } > > static struct tst_test test = { > + .needs_root = 1, > .test_all = verify_setrlimit, > .setup = setup, > .cleanup = cleanup, > > -- > Regards, > Li Wang >
On Tue, Feb 11, 2020 at 6:52 PM Jan Stancek <jstancek@redhat.com> wrote: > > > ----- Original Message ----- > > On Mon, Feb 10, 2020 at 8:47 PM Jan Stancek <jstancek@redhat.com> wrote: > > > > > Lower the parameters so that test completes faster where possible. > > > > > > This also increases alarm timer slightly, which in combination with > > > lower RLIMIT_CPU aims to avoid false positives in environments with > > > high steal time, where it can take multiple of wall clock seconds > > > to spend single second on a cpu. > > > > > > > This patch could reduce the test failure possibility, but I'm afraid it > > can't fix the problem radically, because with `stress -c 20' to overload > an > > s390x system(2cpus) in the background then setrlimit06(patched) still > > easily gets failed: > > setrlimit06.c:98: FAIL: Got only SIGXCPU after reaching both limit > > > > Another way I can think of is to raise the priority before its running, > not > > sure if that will disturb the original test but from my test, it always > > gets a pass even with too much overload. > > Is this in addition to my patch? Because on its own I don't see how this > will help when load is coming from different guests. > Yes, this is only solving for itself loads. Besides the high steal time, that's another reason I guess it causes the same failure, so do you think it makes sense to merge two methods together?
----- Original Message ----- > On Tue, Feb 11, 2020 at 6:52 PM Jan Stancek <jstancek@redhat.com> wrote: > > > > > > > ----- Original Message ----- > > > On Mon, Feb 10, 2020 at 8:47 PM Jan Stancek <jstancek@redhat.com> wrote: > > > > > > > Lower the parameters so that test completes faster where possible. > > > > > > > > This also increases alarm timer slightly, which in combination with > > > > lower RLIMIT_CPU aims to avoid false positives in environments with > > > > high steal time, where it can take multiple of wall clock seconds > > > > to spend single second on a cpu. > > > > > > > > > > This patch could reduce the test failure possibility, but I'm afraid it > > > can't fix the problem radically, because with `stress -c 20' to overload > > an > > > s390x system(2cpus) in the background then setrlimit06(patched) still > > > easily gets failed: > > > setrlimit06.c:98: FAIL: Got only SIGXCPU after reaching both limit > > > > > > Another way I can think of is to raise the priority before its running, > > not > > > sure if that will disturb the original test but from my test, it always > > > gets a pass even with too much overload. > > > > Is this in addition to my patch? Because on its own I don't see how this > > will help when load is coming from different guests. > > > > Yes, this is only solving for itself loads. Besides the high steal time, > that's another reason I guess it causes the same failure, so do you think > it makes sense to merge two methods together? For now I'd go with just original patch. Until there is parallel test execution, there shouldn't be any local load during this test.
On Tue, Feb 11, 2020 at 8:10 PM Jan Stancek <jstancek@redhat.com> wrote: > > > ----- Original Message ----- > > On Tue, Feb 11, 2020 at 6:52 PM Jan Stancek <jstancek@redhat.com> wrote: > > > > > > > > > > > ----- Original Message ----- > > > > On Mon, Feb 10, 2020 at 8:47 PM Jan Stancek <jstancek@redhat.com> > wrote: > > > > > > > > > Lower the parameters so that test completes faster where possible. > > > > > > > > > > This also increases alarm timer slightly, which in combination with > > > > > lower RLIMIT_CPU aims to avoid false positives in environments with > > > > > high steal time, where it can take multiple of wall clock seconds > > > > > to spend single second on a cpu. > > > > > > > > > > > > > This patch could reduce the test failure possibility, but I'm afraid > it > > > > can't fix the problem radically, because with `stress -c 20' to > overload > > > an > > > > s390x system(2cpus) in the background then setrlimit06(patched) still > > > > easily gets failed: > > > > setrlimit06.c:98: FAIL: Got only SIGXCPU after reaching both > limit > > > > > > > > Another way I can think of is to raise the priority before its > running, > > > not > > > > sure if that will disturb the original test but from my test, it > always > > > > gets a pass even with too much overload. > > > > > > Is this in addition to my patch? Because on its own I don't see how > this > > > will help when load is coming from different guests. > > > > > > > Yes, this is only solving for itself loads. Besides the high steal time, > > that's another reason I guess it causes the same failure, so do you think > > it makes sense to merge two methods together? > > For now I'd go with just original patch. Until there is parallel test > execution, > there shouldn't be any local load during this test. > Ok sure. Let's apply the original first, then keep watching the status in the next testing.
----- Original Message ----- > Ok sure. Let's apply the original first, then keep watching the status in > the next testing. Pushed.
diff --git a/testcases/kernel/syscalls/setrlimit/setrlimit06.c b/testcases/kernel/syscalls/setrlimit/setrlimit06.c index 726b26841583..3e5bf1d4253d 100644 --- a/testcases/kernel/syscalls/setrlimit/setrlimit06.c +++ b/testcases/kernel/syscalls/setrlimit/setrlimit06.c @@ -59,8 +59,8 @@ static void verify_setrlimit(void) pid = SAFE_FORK(); if (!pid) { struct rlimit rlim = { - .rlim_cur = 2, - .rlim_max = 3, + .rlim_cur = 1, + .rlim_max = 2, }; TEST(setrlimit(RLIMIT_CPU, &rlim)); @@ -70,7 +70,7 @@ static void verify_setrlimit(void) exit(1); } - alarm(10); + alarm(20); while (1); }
Lower the parameters so that test completes faster where possible. This also increases alarm timer slightly, which in combination with lower RLIMIT_CPU aims to avoid false positives in environments with high steal time, where it can take multiple of wall clock seconds to spend single second on a cpu. Signed-off-by: Jan Stancek <jstancek@redhat.com> --- testcases/kernel/syscalls/setrlimit/setrlimit06.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)