| Message ID | 20180524124234.8437-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 24/05/18 05:42, Chris Wilson wrote: > Recently we discovered that we have a race between swapping and > suspend in our resume path (we might be trying to page in an object > after disabling the block devices). Let's try to exercise that by > exhausting all of system memory before suspend. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=106640 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> LGTM: Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com> > --- > lib/igt_core.c | 34 ++++++++++++++++++++-------------- > lib/igt_core.h | 1 + > tests/drv_suspend.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 63 insertions(+), 14 deletions(-) > > diff --git a/lib/igt_core.c b/lib/igt_core.c > index e292ca24c..804ce4578 100644 > +static void > +test_shrink(int fd, unsigned int mode) > +{ > + uint64_t *can_mlock; > + void *locked; > + uint64_t pin; > + > + gem_quiescent_gpu(fd); > + intel_purge_vm_caches(fd); > + > + can_mlock = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); > + igt_assert(can_mlock != MAP_FAILED); > + > + pin = intel_get_total_ram_mb() << 20; > + > + igt_debug("Locking %'"PRIu64" MiB\n", pin >> 20); > + locked = malloc(pin); > + igt_assert(locked); Shouldn't this^ be a require as well? > + > + /* Lock all the system memory, forcing the driver into swap and OOM */ > + > static void > test_forcewake(int fd, bool hibernate) > { > @@ -199,6 +238,9 @@ igt_main > igt_subtest("sysfs-reader") > test_sysfs_reader(false); > > + igt_subtest("shrink") > + test_shrink(fd, SUSPEND_STATE_MEM); I am assuming you plan to have tests for other suspend modes. Thanks, Antonio > + > igt_subtest("forcewake") > test_forcewake(fd, false); > >
Quoting Antonio Argenziano (2018-05-24 21:55:38) > > > On 24/05/18 05:42, Chris Wilson wrote: > > Recently we discovered that we have a race between swapping and > > suspend in our resume path (we might be trying to page in an object > > after disabling the block devices). Let's try to exercise that by > > exhausting all of system memory before suspend. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=106640 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> > > LGTM: > Reviewed-by: Antonio Argenziano <antonio.argenziano@intel.com> > > > --- > > lib/igt_core.c | 34 ++++++++++++++++++++-------------- > > lib/igt_core.h | 1 + > > tests/drv_suspend.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 63 insertions(+), 14 deletions(-) > > > > diff --git a/lib/igt_core.c b/lib/igt_core.c > > index e292ca24c..804ce4578 100644 > > > +static void > > +test_shrink(int fd, unsigned int mode) > > +{ > > + uint64_t *can_mlock; > > + void *locked; > > + uint64_t pin; > > + > > + gem_quiescent_gpu(fd); > > + intel_purge_vm_caches(fd); > > + > > + can_mlock = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); > > + igt_assert(can_mlock != MAP_FAILED); > > + > > + pin = intel_get_total_ram_mb() << 20; > > + > > + igt_debug("Locking %'"PRIu64" MiB\n", pin >> 20); > > + locked = malloc(pin); > > + igt_assert(locked); > > Shouldn't this^ be a require as well? It would be a weird system if it failed; overcommit turned off and populate on allocation. igt_require is reasonable as we haven't begun testing anything for real yet. > > + > > + /* Lock all the system memory, forcing the driver into swap and OOM */ > > > + > > static void > > test_forcewake(int fd, bool hibernate) > > { > > @@ -199,6 +238,9 @@ igt_main > > igt_subtest("sysfs-reader") > > test_sysfs_reader(false); > > > > + igt_subtest("shrink") > > + test_shrink(fd, SUSPEND_STATE_MEM); > > I am assuming you plan to have tests for other suspend modes. Not as much as you might imagine ;) Filling all of ram before hibernation is not as interesting. But using the test modes might be. -Chris
diff --git a/lib/igt_core.c b/lib/igt_core.c index e292ca24c..804ce4578 100644 --- a/lib/igt_core.c +++ b/lib/igt_core.c @@ -1756,20 +1756,7 @@ void igt_child_done(pid_t pid) test_children[i] = test_children[i + 1]; } -/** - * igt_waitchildren: - * - * Wait for all children forked with igt_fork. - * - * The magic here is that exit codes from children will be correctly propagated - * to the main thread, including the relevant exit code if a child thread failed. - * Of course if multiple children failed with different exit codes the resulting - * exit code will be non-deterministic. - * - * Note that igt_skip() will not be forwarded, feature tests need to be done - * before spawning threads with igt_fork(). - */ -void igt_waitchildren(void) +int __igt_waitchildren(void) { int err = 0; int count; @@ -1815,6 +1802,25 @@ void igt_waitchildren(void) } num_test_children = 0; + return err; +} + +/** + * igt_waitchildren: + * + * Wait for all children forked with igt_fork. + * + * The magic here is that exit codes from children will be correctly propagated + * to the main thread, including the relevant exit code if a child thread failed. + * Of course if multiple children failed with different exit codes the resulting + * exit code will be non-deterministic. + * + * Note that igt_skip() will not be forwarded, feature tests need to be done + * before spawning threads with igt_fork(). + */ +void igt_waitchildren(void) +{ + int err = __igt_waitchildren(); if (err) igt_fail(err); } diff --git a/lib/igt_core.h b/lib/igt_core.h index 3d7b787b2..6d4260403 100644 --- a/lib/igt_core.h +++ b/lib/igt_core.h @@ -742,6 +742,7 @@ bool __igt_fork(void); for (int child = 0; child < (num_children); child++) \ for (; __igt_fork(); exit(0)) void igt_child_done(pid_t pid); +int __igt_waitchildren(void); void igt_waitchildren(void); void igt_waitchildren_timeout(int seconds, const char *reason); diff --git a/tests/drv_suspend.c b/tests/drv_suspend.c index 2e39f20ae..8f71a81d1 100644 --- a/tests/drv_suspend.c +++ b/tests/drv_suspend.c @@ -160,6 +160,45 @@ test_sysfs_reader(bool hibernate) igt_stop_helper(&reader); } +static void +test_shrink(int fd, unsigned int mode) +{ + uint64_t *can_mlock; + void *locked; + uint64_t pin; + + gem_quiescent_gpu(fd); + intel_purge_vm_caches(fd); + + can_mlock = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); + igt_assert(can_mlock != MAP_FAILED); + + pin = intel_get_total_ram_mb() << 20; + + igt_debug("Locking %'"PRIu64" MiB\n", pin >> 20); + locked = malloc(pin); + igt_assert(locked); + + /* Lock all the system memory, forcing the driver into swap and OOM */ + igt_fork(child, 1) { + for (uint64_t bytes = 64 << 20; bytes <= pin; bytes += 64 << 20) + if (!mlock(locked, bytes)) + *can_mlock = bytes; + if (!mlock(locked, pin)) + *can_mlock = pin; + } + __igt_waitchildren(); + igt_require(*can_mlock); + mlock(locked, *can_mlock); + igt_info("Locked %'"PRIu64" MiB\n", *can_mlock >> 20); + munmap(can_mlock, 4096); + + intel_purge_vm_caches(fd); + igt_system_suspend_autoresume(mode, SUSPEND_TEST_NONE); + + free(locked); +} + static void test_forcewake(int fd, bool hibernate) { @@ -199,6 +238,9 @@ igt_main igt_subtest("sysfs-reader") test_sysfs_reader(false); + igt_subtest("shrink") + test_shrink(fd, SUSPEND_STATE_MEM); + igt_subtest("forcewake") test_forcewake(fd, false);
Recently we discovered that we have a race between swapping and suspend in our resume path (we might be trying to page in an object after disabling the block devices). Let's try to exercise that by exhausting all of system memory before suspend. References: https://bugs.freedesktop.org/show_bug.cgi?id=106640 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tomi Sarvela <tomi.p.sarvela@intel.com> --- lib/igt_core.c | 34 ++++++++++++++++++++-------------- lib/igt_core.h | 1 + tests/drv_suspend.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 14 deletions(-)