public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Disable Android low memory killer
@ 2014-09-26  9:27 tim.gore
  2014-09-26  9:27 ` [PATCH 1/2] lib/igt_core: make single/simple tests use igt_exit tim.gore
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: tim.gore @ 2014-09-26  9:27 UTC (permalink / raw)
  To: intel-gfx

From: Tim Gore <tim.gore@intel.com>

For some tests that put pressure on memory, the Android
lowmemorykiller needs to be disabled for the test to run to
completion. The first patch is a simple bit of preparation
to ensure that all (well written) "simple" tests exit via a
call to igt_exit, in the same way as tests with subtests do.
This is to make sure we can clean up by re-enabling the
lowmemorykiller.
The second patch is to disable the Android lowmemorykiller
during the common initialisation code (in oom_adjust_for_doom
to be exact) and to re-enstate it in igt_exit.

Tim Gore (2):
  lib/igt_core: make single/simple tests use igt_exit
  lib/igt_core.c: disable lowmemorykiller during tests

 lib/igt_core.c         | 77 +++++++++++++++++++++++++++++++++++++++++++++++---
 lib/igt_core.h         |  2 +-
 tests/igt_simulation.c |  2 +-
 3 files changed, 75 insertions(+), 6 deletions(-)

-- 
2.0.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] lib/igt_core: make single/simple tests use igt_exit
  2014-09-26  9:27 [PATCH 0/2] Disable Android low memory killer tim.gore
@ 2014-09-26  9:27 ` tim.gore
  2014-09-29 13:15   ` Daniel Vetter
  2014-09-26  9:27 ` [PATCH 2/2] lib/igt_core.c: disable lowmemorykiller during tests tim.gore
  2014-09-26  9:49 ` [PATCH 0/2] Disable Android low memory killer Chris Wilson
  2 siblings, 1 reply; 11+ messages in thread
From: tim.gore @ 2014-09-26  9:27 UTC (permalink / raw)
  To: intel-gfx

From: Tim Gore <tim.gore@intel.com>

Currently tests that use igt_simple_main will simply call
"exit()" if they pass, making it difficult to ensure that
any required cleanup is done. At present this is not an
issue, but it will be when I submit a patch to turn off the
lowmemorykiller for all tests.

Signed-off-by: Tim Gore <tim.gore@intel.com>
---
 lib/igt_core.c         | 9 +++++----
 lib/igt_core.h         | 2 +-
 tests/igt_simulation.c | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 5d41468..9344815 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -641,7 +641,7 @@ bool igt_only_list_subtests(void)
 static bool skipped_one = false;
 static bool succeeded_one = false;
 static bool failed_one = false;
-static int igt_exitcode;
+static int igt_exitcode = IGT_EXIT_SUCCESS;
 
 static void exit_subtest(const char *) __attribute__((noreturn));
 static void exit_subtest(const char *result)
@@ -692,7 +692,8 @@ void igt_skip(const char *f, ...)
 		assert(in_fixture);
 		__igt_fixture_end();
 	} else {
-		exit(IGT_EXIT_SKIP);
+		igt_exitcode = IGT_EXIT_SKIP;
+		igt_exit();
 	}
 }
 
@@ -786,7 +787,7 @@ void igt_fail(int exitcode)
 			__igt_fixture_end();
 		}
 
-		exit(exitcode);
+		igt_exit();
 	}
 }
 
@@ -857,7 +858,7 @@ void igt_exit(void)
 		exit(IGT_EXIT_SUCCESS);
 
 	if (!test_with_subtests)
-		exit(IGT_EXIT_SUCCESS);
+		exit(igt_exitcode);
 
 	/* Calling this without calling one of the above is a failure */
 	assert(skipped_one || succeeded_one || failed_one);
diff --git a/lib/igt_core.h b/lib/igt_core.h
index d74cbf9..974a2fb 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -188,7 +188,7 @@ void igt_simple_init_parse_opts(int argc, char **argv,
 	int main(int argc, char **argv) { \
 		igt_simple_init(argc, argv); \
 		igt_tokencat(__real_main, __LINE__)(); \
-		exit(0); \
+		igt_exit(); \
 	} \
 	static void igt_tokencat(__real_main, __LINE__)(void) \
 
diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c
index 13b2da9..e588959 100644
--- a/tests/igt_simulation.c
+++ b/tests/igt_simulation.c
@@ -65,7 +65,7 @@ static int do_fork(void)
 
 			igt_skip_on_simulation();
 
-			exit(0);
+			igt_exit();
 		} else {
 			if (list_subtests)
 				igt_subtest_init(2, argv_list);
-- 
2.0.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] lib/igt_core.c: disable lowmemorykiller during tests
  2014-09-26  9:27 [PATCH 0/2] Disable Android low memory killer tim.gore
  2014-09-26  9:27 ` [PATCH 1/2] lib/igt_core: make single/simple tests use igt_exit tim.gore
@ 2014-09-26  9:27 ` tim.gore
  2014-09-26  9:49 ` [PATCH 0/2] Disable Android low memory killer Chris Wilson
  2 siblings, 0 replies; 11+ messages in thread
From: tim.gore @ 2014-09-26  9:27 UTC (permalink / raw)
  To: intel-gfx

From: Tim Gore <tim.gore@intel.com>

Several IGT tests put a lot of pressure on memory and
when running these tests on Android they tend to get
killed by the lowmemorykiller. The lowmemorykiller really
is not usefull in this context and is just preventing the
test from doing its job. So this commit disables the
lowmemorykiller by writing "9999" to its oom adj parameter
which means it will never "select" any process to kill.
The normal linux oom killer is still there to protect
the kernel.
NOTE: if a test is not well behaved, it could exit
without re-enabling the lowmemorykiller. This should
be mostly benign, especially since running these tests
requires the Android system to be stopped, and restarting
the Android system automatically re-enstates the default
lowmemorykiller oom adj parameters.

Signed-off-by: Tim Gore <tim.gore@intel.com>
---
 lib/igt_core.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 9344815..9ef39ab 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -326,6 +326,67 @@ static void print_usage(const char *help_str, bool output_on_stderr)
 		fprintf(f, "%s\n", help_str);
 }
 
+
+/* Some of the IGT tests put quite a lot of pressure on memory and when
+ * running on Android they are sometimes killed by the Android low memory killer.
+ * The low memory killer really isn't usefull in this context and has no
+ * interaction with the gpu driver that we are testing, so the following
+ * function is used to disable it by modifying one of its module parameters.
+ * We still have the normal linux oom killer to protect the kernel.
+ * Apparently it is also possible for the lowmemorykiller to get included
+ * in some linux distributions; so rather than check for Android we directly
+ * check for the existence of the module parameter we want to adjust.
+ */
+void low_mem_killer_disable(bool disable)
+{
+	static const char* adj_fname="/sys/module/lowmemorykiller/parameters/adj";
+	static const char no_lowmem_killer[] = "9999";
+	int fd;
+	struct stat buf;
+	/* The following must persist across invocations */
+	static char prev_adj_scores[256];
+	static int adj_scores_len = 0;
+	static bool is_disabled = false;
+
+	/* capture the permissions bits for the lowmemkiller adj pseudo-file.
+	   Bail out if the stat fails; it probably means that there is no
+	   lowmemorykiller, but in any case we're doomed. */
+	if (stat (adj_fname, &buf))
+	{
+		igt_assert(errno == ENOENT);
+		return;
+	}
+	/* make sure the file can be read/written - by default it is write-only */
+	chmod (adj_fname, S_IRUSR | S_IWUSR);
+
+	if (disable && (!is_disabled))
+	{
+		/* read the current oom adj parameters for lowmemorykiller */
+		fd = open(adj_fname, O_RDWR);
+		igt_assert(fd != -1);
+		adj_scores_len = read(fd, (void*)prev_adj_scores, 255);
+
+		/* writing 9999 to this module parameter effectively diables the
+		 * low memory killer. This is not a real file, so we dont need to
+		 * seek to the start or truncate it */
+		write(fd, no_lowmem_killer, sizeof(no_lowmem_killer));
+		close(fd);
+		is_disabled = true;
+	}
+	else if ((!disable) && is_disabled)
+	{
+		/* just re-enstate the original settings */
+		fd = open(adj_fname, O_WRONLY);
+		igt_assert(fd != -1);
+		write(fd, prev_adj_scores, adj_scores_len);
+		close(fd);
+		is_disabled = false;
+	}
+
+	/* re-enstate the file permissions */
+	chmod (adj_fname, buf.st_mode);
+}
+
 static void oom_adjust_for_doom(void)
 {
 	int fd;
@@ -334,6 +395,10 @@ static void oom_adjust_for_doom(void)
 	fd = open("/proc/self/oom_score_adj", O_WRONLY);
 	igt_assert(fd != -1);
 	igt_assert(write(fd, always_kill, sizeof(always_kill)) == sizeof(always_kill));
+	close(fd);
+
+	/* disable the low memory killer (not oom) if present. */
+	low_mem_killer_disable(true);
 }
 
 static int common_init(int argc, char **argv,
@@ -848,6 +913,9 @@ void igt_exit(void)
 {
 	igt_exit_called = true;
 
+	/* re-enstate the low mem killer if it exists */
+	low_mem_killer_disable(false);
+
 	if (run_single_subtest && !run_single_subtest_found) {
 		igt_warn("Unknown subtest: %s\n", run_single_subtest);
 		exit(IGT_EXIT_INVALID);
-- 
2.0.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Disable Android low memory killer
  2014-09-26  9:27 [PATCH 0/2] Disable Android low memory killer tim.gore
  2014-09-26  9:27 ` [PATCH 1/2] lib/igt_core: make single/simple tests use igt_exit tim.gore
  2014-09-26  9:27 ` [PATCH 2/2] lib/igt_core.c: disable lowmemorykiller during tests tim.gore
@ 2014-09-26  9:49 ` Chris Wilson
  2014-09-26 10:08   ` Gore, Tim
  2 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2014-09-26  9:49 UTC (permalink / raw)
  To: tim.gore; +Cc: intel-gfx

On Fri, Sep 26, 2014 at 10:27:22AM +0100, tim.gore@intel.com wrote:
> From: Tim Gore <tim.gore@intel.com>
> 
> For some tests that put pressure on memory, the Android
> lowmemorykiller needs to be disabled for the test to run to
> completion. The first patch is a simple bit of preparation
> to ensure that all (well written) "simple" tests exit via a
> call to igt_exit, in the same way as tests with subtests do.
> This is to make sure we can clean up by re-enabling the
> lowmemorykiller.
> The second patch is to disable the Android lowmemorykiller
> during the common initialisation code (in oom_adjust_for_doom
> to be exact) and to re-enstate it in igt_exit.

Isn't this just a huge red flag that the kernel is incompatible with our
UMA allocation strategy and that by masking the issue we will never be
motivated to fix it?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Disable Android low memory killer
  2014-09-26  9:49 ` [PATCH 0/2] Disable Android low memory killer Chris Wilson
@ 2014-09-26 10:08   ` Gore, Tim
  2014-09-26 10:20     ` Chris Wilson
  2014-09-26 10:29     ` Chris Wilson
  0 siblings, 2 replies; 11+ messages in thread
From: Gore, Tim @ 2014-09-26 10:08 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Friday, September 26, 2014 10:50 AM
> To: Gore, Tim
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 0/2] Disable Android low memory killer
> 
> On Fri, Sep 26, 2014 at 10:27:22AM +0100, tim.gore@intel.com wrote:
> > From: Tim Gore <tim.gore@intel.com>
> >
> > For some tests that put pressure on memory, the Android
> > lowmemorykiller needs to be disabled for the test to run to
> > completion. The first patch is a simple bit of preparation to ensure
> > that all (well written) "simple" tests exit via a call to igt_exit, in
> > the same way as tests with subtests do.
> > This is to make sure we can clean up by re-enabling the
> > lowmemorykiller.
> > The second patch is to disable the Android lowmemorykiller during the
> > common initialisation code (in oom_adjust_for_doom to be exact) and to
> > re-enstate it in igt_exit.
> 
> Isn't this just a huge red flag that the kernel is incompatible with our UMA
> allocation strategy and that by masking the issue we will never be motivated
> to fix it?
> -Chris
> 
I don't think so. This is really just about the Android low memory killer having
Different goals to kswapd. Kswapd tries to keep a certain amount of free memory
so that the kernel can run smoothly. On Android the lowmemorykiller attempts
to maintain somewhat higher levels of free memory by killing off processes,
because the user is not expected to ever close anything and expects new
applications to open quickly. So if you put the memory under pressure the
Android low memory killer will inevitably look for something to kill, and if
your test is the only thing running its toast. The linux oom killer is still there,
but is never needed on Android because the lowmemorykiller gets there first.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Disable Android low memory killer
  2014-09-26 10:08   ` Gore, Tim
@ 2014-09-26 10:20     ` Chris Wilson
  2014-09-26 10:29     ` Chris Wilson
  1 sibling, 0 replies; 11+ messages in thread
From: Chris Wilson @ 2014-09-26 10:20 UTC (permalink / raw)
  To: Gore, Tim; +Cc: intel-gfx@lists.freedesktop.org

On Fri, Sep 26, 2014 at 10:08:54AM +0000, Gore, Tim wrote:
> 
> 
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > Sent: Friday, September 26, 2014 10:50 AM
> > To: Gore, Tim
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 0/2] Disable Android low memory killer
> > 
> > On Fri, Sep 26, 2014 at 10:27:22AM +0100, tim.gore@intel.com wrote:
> > > From: Tim Gore <tim.gore@intel.com>
> > >
> > > For some tests that put pressure on memory, the Android
> > > lowmemorykiller needs to be disabled for the test to run to
> > > completion. The first patch is a simple bit of preparation to ensure
> > > that all (well written) "simple" tests exit via a call to igt_exit, in
> > > the same way as tests with subtests do.
> > > This is to make sure we can clean up by re-enabling the
> > > lowmemorykiller.
> > > The second patch is to disable the Android lowmemorykiller during the
> > > common initialisation code (in oom_adjust_for_doom to be exact) and to
> > > re-enstate it in igt_exit.
> > 
> > Isn't this just a huge red flag that the kernel is incompatible with our UMA
> > allocation strategy and that by masking the issue we will never be motivated
> > to fix it?
> > -Chris
> > 
> I don't think so. This is really just about the Android low memory killer having
> Different goals to kswapd. Kswapd tries to keep a certain amount of free memory
> so that the kernel can run smoothly. On Android the lowmemorykiller attempts
> to maintain somewhat higher levels of free memory by killing off processes,
> because the user is not expected to ever close anything and expects new
> applications to open quickly. So if you put the memory under pressure the
> Android low memory killer will inevitably look for something to kill, and if
> your test is the only thing running its toast. The linux oom killer is still there,
> but is never needed on Android because the lowmemorykiller gets there first.

There's the issue that the lowmemorykiller doesn't take into account
that, often quite large amounts, of i915 memory is purgeable (should be
regarded as free by the shrinker). That can and does affect ordinary userspace
clients (for example if a popular desktop distribution enables lowmemkiller...).

The simplest solution would perhaps to be to create a new category of
"late" shrinker for lowmemkiller so that it runs after all the in-kernel
caches are cleaned but before actual swapping.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Disable Android low memory killer
  2014-09-26 10:08   ` Gore, Tim
  2014-09-26 10:20     ` Chris Wilson
@ 2014-09-26 10:29     ` Chris Wilson
  2014-09-26 10:46       ` Gore, Tim
  1 sibling, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2014-09-26 10:29 UTC (permalink / raw)
  To: Gore, Tim; +Cc: intel-gfx@lists.freedesktop.org

On Fri, Sep 26, 2014 at 10:08:54AM +0000, Gore, Tim wrote:
> I don't think so. This is really just about the Android low memory killer having
> Different goals to kswapd. Kswapd tries to keep a certain amount of free memory
> so that the kernel can run smoothly. On Android the lowmemorykiller attempts
> to maintain somewhat higher levels of free memory by killing off processes,
> because the user is not expected to ever close anything and expects new
> applications to open quickly. So if you put the memory under pressure the
> Android low memory killer will inevitably look for something to kill, and if
> your test is the only thing running its toast. The linux oom killer is still there,
> but is never needed on Android because the lowmemorykiller gets there first.

Though I think the interaction between lowmemkiller and i915 is broken,
I do agree that we need to run our swap tests and that to do so we need
to disable lowmemkiller.

I would prefer it if only the swap-thrash tests disabled the
lowmemkiller though, as I think we still need to test integration
behaviour and if lowmemkiller starts killing tests that we think should
be well within the limits, that is likely to be our bug.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Disable Android low memory killer
  2014-09-26 10:29     ` Chris Wilson
@ 2014-09-26 10:46       ` Gore, Tim
  2014-09-26 11:14         ` Chris Wilson
  0 siblings, 1 reply; 11+ messages in thread
From: Gore, Tim @ 2014-09-26 10:46 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Friday, September 26, 2014 11:30 AM
> To: Gore, Tim
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 0/2] Disable Android low memory killer
> 
> On Fri, Sep 26, 2014 at 10:08:54AM +0000, Gore, Tim wrote:
> > I don't think so. This is really just about the Android low memory
> > killer having Different goals to kswapd. Kswapd tries to keep a
> > certain amount of free memory so that the kernel can run smoothly. On
> > Android the lowmemorykiller attempts to maintain somewhat higher
> > levels of free memory by killing off processes, because the user is
> > not expected to ever close anything and expects new applications to
> > open quickly. So if you put the memory under pressure the Android low
> > memory killer will inevitably look for something to kill, and if your
> > test is the only thing running its toast. The linux oom killer is still there, but
> is never needed on Android because the lowmemorykiller gets there first.
> 
> Though I think the interaction between lowmemkiller and i915 is broken, I do
> agree that we need to run our swap tests and that to do so we need to
> disable lowmemkiller.
> 
> I would prefer it if only the swap-thrash tests disabled the lowmemkiller
> though, as I think we still need to test integration behaviour and if
> lowmemkiller starts killing tests that we think should be well within the limits,
> that is likely to be our bug.
> -Chris
> 
We could do it on a test by test basis if people prefer. It just puts the responsibility
on test writers to know when they might trigger the lowmemorykiller. The trouble
is that the kernel is pretty lazy when comes to freeing up memory. You may think
there should be plenty "free", but that doesn't mean those pages are on the free list.
Once kswapd has achieved its high water mark its done. But the lowmemorykiller
always looks for more than the high water mark (its threshold is based on the
high water mark, so is always higher). If you have enough file backed pages you're ok,
but igt tests don't tend to do much file reading.
As for modifying the low memory killer to be more linux friendly, that's a whole
new level of debate!!

  Tim

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Disable Android low memory killer
  2014-09-26 10:46       ` Gore, Tim
@ 2014-09-26 11:14         ` Chris Wilson
  2014-09-29 13:19           ` Daniel Vetter
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Wilson @ 2014-09-26 11:14 UTC (permalink / raw)
  To: Gore, Tim; +Cc: intel-gfx@lists.freedesktop.org

On Fri, Sep 26, 2014 at 10:46:48AM +0000, Gore, Tim wrote:
> 
> 
> > -----Original Message-----
> > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > Sent: Friday, September 26, 2014 11:30 AM
> > To: Gore, Tim
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH 0/2] Disable Android low memory killer
> > 
> > On Fri, Sep 26, 2014 at 10:08:54AM +0000, Gore, Tim wrote:
> > > I don't think so. This is really just about the Android low memory
> > > killer having Different goals to kswapd. Kswapd tries to keep a
> > > certain amount of free memory so that the kernel can run smoothly. On
> > > Android the lowmemorykiller attempts to maintain somewhat higher
> > > levels of free memory by killing off processes, because the user is
> > > not expected to ever close anything and expects new applications to
> > > open quickly. So if you put the memory under pressure the Android low
> > > memory killer will inevitably look for something to kill, and if your
> > > test is the only thing running its toast. The linux oom killer is still there, but
> > is never needed on Android because the lowmemorykiller gets there first.
> > 
> > Though I think the interaction between lowmemkiller and i915 is broken, I do
> > agree that we need to run our swap tests and that to do so we need to
> > disable lowmemkiller.
> > 
> > I would prefer it if only the swap-thrash tests disabled the lowmemkiller
> > though, as I think we still need to test integration behaviour and if
> > lowmemkiller starts killing tests that we think should be well within the limits,
> > that is likely to be our bug.
> > -Chris
> > 
> We could do it on a test by test basis if people prefer. It just puts the responsibility
> on test writers to know when they might trigger the lowmemorykiller. The trouble
> is that the kernel is pretty lazy when comes to freeing up memory. You may think
> there should be plenty "free", but that doesn't mean those pages are on the free list.
> Once kswapd has achieved its high water mark its done. But the lowmemorykiller
> always looks for more than the high water mark (its threshold is based on the
> high water mark, so is always higher). If you have enough file backed pages you're ok,
> but igt tests don't tend to do much file reading.

The principle is that mempressure tests call intel_check_memory() first
to see if it valid to run. One of its side-effects is kick the kernel
into freeing up all of its caches. It seems reasonable that we could
disable lowmemkiller here if the test declares that it wants to use
swap. The trick I leave up to you is how to reenable lowmemkiller
automatically when the test completes...

Or since swap tests are already special, there is no problem in having
  igt_swapping_start();
  ...
  igt_swapping_end();
bracketing them.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] lib/igt_core: make single/simple tests use igt_exit
  2014-09-26  9:27 ` [PATCH 1/2] lib/igt_core: make single/simple tests use igt_exit tim.gore
@ 2014-09-29 13:15   ` Daniel Vetter
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2014-09-29 13:15 UTC (permalink / raw)
  To: tim.gore; +Cc: intel-gfx

On Fri, Sep 26, 2014 at 10:27:23AM +0100, tim.gore@intel.com wrote:
> From: Tim Gore <tim.gore@intel.com>
> 
> Currently tests that use igt_simple_main will simply call
> "exit()" if they pass, making it difficult to ensure that
> any required cleanup is done. At present this is not an
> issue, but it will be when I submit a patch to turn off the
> lowmemorykiller for all tests.
> 
> Signed-off-by: Tim Gore <tim.gore@intel.com>

igt_install_exit_handler and you're done. Sorry if I was a bit unclear
with what I've meant exactly. Btw there's some cute docs for all this
stuff:

http://people.freedesktop.org/~danvet/igt/intel-gpu-tools-i-g-t-core.html

Worth reading once in a while, at least the overview sections ;-)

Cheers, Daniel

> ---
>  lib/igt_core.c         | 9 +++++----
>  lib/igt_core.h         | 2 +-
>  tests/igt_simulation.c | 2 +-
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 5d41468..9344815 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -641,7 +641,7 @@ bool igt_only_list_subtests(void)
>  static bool skipped_one = false;
>  static bool succeeded_one = false;
>  static bool failed_one = false;
> -static int igt_exitcode;
> +static int igt_exitcode = IGT_EXIT_SUCCESS;
>  
>  static void exit_subtest(const char *) __attribute__((noreturn));
>  static void exit_subtest(const char *result)
> @@ -692,7 +692,8 @@ void igt_skip(const char *f, ...)
>  		assert(in_fixture);
>  		__igt_fixture_end();
>  	} else {
> -		exit(IGT_EXIT_SKIP);
> +		igt_exitcode = IGT_EXIT_SKIP;
> +		igt_exit();
>  	}
>  }
>  
> @@ -786,7 +787,7 @@ void igt_fail(int exitcode)
>  			__igt_fixture_end();
>  		}
>  
> -		exit(exitcode);
> +		igt_exit();
>  	}
>  }
>  
> @@ -857,7 +858,7 @@ void igt_exit(void)
>  		exit(IGT_EXIT_SUCCESS);
>  
>  	if (!test_with_subtests)
> -		exit(IGT_EXIT_SUCCESS);
> +		exit(igt_exitcode);
>  
>  	/* Calling this without calling one of the above is a failure */
>  	assert(skipped_one || succeeded_one || failed_one);
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index d74cbf9..974a2fb 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -188,7 +188,7 @@ void igt_simple_init_parse_opts(int argc, char **argv,
>  	int main(int argc, char **argv) { \
>  		igt_simple_init(argc, argv); \
>  		igt_tokencat(__real_main, __LINE__)(); \
> -		exit(0); \
> +		igt_exit(); \
>  	} \
>  	static void igt_tokencat(__real_main, __LINE__)(void) \
>  
> diff --git a/tests/igt_simulation.c b/tests/igt_simulation.c
> index 13b2da9..e588959 100644
> --- a/tests/igt_simulation.c
> +++ b/tests/igt_simulation.c
> @@ -65,7 +65,7 @@ static int do_fork(void)
>  
>  			igt_skip_on_simulation();
>  
> -			exit(0);
> +			igt_exit();
>  		} else {
>  			if (list_subtests)
>  				igt_subtest_init(2, argv_list);
> -- 
> 2.0.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Disable Android low memory killer
  2014-09-26 11:14         ` Chris Wilson
@ 2014-09-29 13:19           ` Daniel Vetter
  0 siblings, 0 replies; 11+ messages in thread
From: Daniel Vetter @ 2014-09-29 13:19 UTC (permalink / raw)
  To: Chris Wilson, Gore, Tim, intel-gfx@lists.freedesktop.org

On Fri, Sep 26, 2014 at 12:14:49PM +0100, Chris Wilson wrote:
> On Fri, Sep 26, 2014 at 10:46:48AM +0000, Gore, Tim wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> > > Sent: Friday, September 26, 2014 11:30 AM
> > > To: Gore, Tim
> > > Cc: intel-gfx@lists.freedesktop.org
> > > Subject: Re: [Intel-gfx] [PATCH 0/2] Disable Android low memory killer
> > > 
> > > On Fri, Sep 26, 2014 at 10:08:54AM +0000, Gore, Tim wrote:
> > > > I don't think so. This is really just about the Android low memory
> > > > killer having Different goals to kswapd. Kswapd tries to keep a
> > > > certain amount of free memory so that the kernel can run smoothly. On
> > > > Android the lowmemorykiller attempts to maintain somewhat higher
> > > > levels of free memory by killing off processes, because the user is
> > > > not expected to ever close anything and expects new applications to
> > > > open quickly. So if you put the memory under pressure the Android low
> > > > memory killer will inevitably look for something to kill, and if your
> > > > test is the only thing running its toast. The linux oom killer is still there, but
> > > is never needed on Android because the lowmemorykiller gets there first.
> > > 
> > > Though I think the interaction between lowmemkiller and i915 is broken, I do
> > > agree that we need to run our swap tests and that to do so we need to
> > > disable lowmemkiller.
> > > 
> > > I would prefer it if only the swap-thrash tests disabled the lowmemkiller
> > > though, as I think we still need to test integration behaviour and if
> > > lowmemkiller starts killing tests that we think should be well within the limits,
> > > that is likely to be our bug.
> > > -Chris
> > > 
> > We could do it on a test by test basis if people prefer. It just puts the responsibility
> > on test writers to know when they might trigger the lowmemorykiller. The trouble
> > is that the kernel is pretty lazy when comes to freeing up memory. You may think
> > there should be plenty "free", but that doesn't mean those pages are on the free list.
> > Once kswapd has achieved its high water mark its done. But the lowmemorykiller
> > always looks for more than the high water mark (its threshold is based on the
> > high water mark, so is always higher). If you have enough file backed pages you're ok,
> > but igt tests don't tend to do much file reading.
> 
> The principle is that mempressure tests call intel_check_memory() first
> to see if it valid to run. One of its side-effects is kick the kernel
> into freeing up all of its caches. It seems reasonable that we could
> disable lowmemkiller here if the test declares that it wants to use
> swap. The trick I leave up to you is how to reenable lowmemkiller
> automatically when the test completes...
> 
> Or since swap tests are already special, there is no problem in having
>   igt_swapping_start();
>   ...
>   igt_swapping_end();
> bracketing them.

Iiui corrrectly then this isn't good enough, and the lowmemory killer will
go beserk even for the tests that exercise purgeable behaviour.

Imo this stuff is just fundamentally misdesigned, but if our Android gfx
people are ok with the state of affairs and don't want to fix the
lowmemory killer for i915 then I'm ok with merging this hack. And then it
also makes sense to have it unconditionally for all tests. We just need to
add a big comment explaining that if someone ever fixes up the lowmemory
killer we need to drop this again.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2014-09-29 13:19 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-26  9:27 [PATCH 0/2] Disable Android low memory killer tim.gore
2014-09-26  9:27 ` [PATCH 1/2] lib/igt_core: make single/simple tests use igt_exit tim.gore
2014-09-29 13:15   ` Daniel Vetter
2014-09-26  9:27 ` [PATCH 2/2] lib/igt_core.c: disable lowmemorykiller during tests tim.gore
2014-09-26  9:49 ` [PATCH 0/2] Disable Android low memory killer Chris Wilson
2014-09-26 10:08   ` Gore, Tim
2014-09-26 10:20     ` Chris Wilson
2014-09-26 10:29     ` Chris Wilson
2014-09-26 10:46       ` Gore, Tim
2014-09-26 11:14         ` Chris Wilson
2014-09-29 13:19           ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox