[PATCH i-g-t v2] lib/igt_core.c : only disable low mem killer once

[PATCH i-g-t v2] lib/igt_core.c : only disable low mem killer once

[tim.gore]

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

The call to low_mem_killer_disable(true) was being done
from within function oom_adjust_for_doom. However,
oom_adjust_for_doom gets called from 3 places. We only
want the call to low_mem_killer_disable(true) to happen
during common_init, so call it from here instead of from
oom_adjust_for_doom.

v2:Thomas Wood pointed out that the initial call to disable
the low_mem_killer does not get made when we are just
listing subtests; so I have qualified the call from the
exit handler, which re-enables the low_mem_killer, with
if (!igt_only_list_subtests()).
For belt and braces I have also made low_mem_killer_disable
idempotent, so multiple calls to disable or re-enable are
safe.

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

diff --git a/lib/igt_core.c b/lib/igt_core.c
index 7d04f2c..15f3922 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -396,6 +396,7 @@ static void low_mem_killer_disable(bool disable)
 	/* 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
@@ -408,7 +409,7 @@ static void low_mem_killer_disable(bool disable)
 	/* make sure the file can be read/written - by default it is write-only */
 	chmod(adj_fname, S_IRUSR | S_IWUSR);
 
-	if (disable) {
+	if (disable && !is_disabled) {
 		/* read the current oom adj parameters for lowmemorykiller */
 		fd = open(adj_fname, O_RDWR);
 		igt_assert(fd != -1);
@@ -421,13 +422,15 @@ static void low_mem_killer_disable(bool disable)
 		igt_assert_eq(write(fd, no_lowmem_killer, sizeof(no_lowmem_killer)),
 			      sizeof(no_lowmem_killer));
 		close(fd);
-	} else {
+		is_disabled = true;
+	} else if (is_disabled) {
 		/* just re-enstate the original settings */
 		fd = open(adj_fname, O_WRONLY);
 		igt_assert(fd != -1);
 		igt_assert_eq(write(fd, prev_adj_scores, adj_scores_len),
 			      adj_scores_len);
 		close(fd);
+		is_disabled = false;
 	}
 
 	/* re-enstate the file permissions */
@@ -437,7 +440,10 @@ static void low_mem_killer_disable(bool disable)
 bool igt_exit_called;
 static void common_exit_handler(int sig)
 {
-	low_mem_killer_disable(false);
+	if (!igt_only_list_subtests())
+	{
+		low_mem_killer_disable(false);
+	}
 
 	/* When not killed by a signal check that igt_exit() has been properly
 	 * called. */
@@ -490,7 +496,6 @@ static void oom_adjust_for_doom(void)
 	igt_assert(write(fd, always_kill, sizeof(always_kill)) == sizeof(always_kill));
 	close(fd);
 
-	low_mem_killer_disable(true);
 }
 
 static int common_init(int *argc, char **argv,
@@ -653,6 +658,7 @@ out:
 		print_version();
 
 		oom_adjust_for_doom();
+		low_mem_killer_disable(true);
 	}
 
 	/* install exit handler, to ensure we clean up */
-- 
2.3.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t v2] lib/igt_core.c : only disable low mem killer once

[Thomas Wood]

On 27 April 2015 at 16:17,  <tim.gore@intel.com> wrote:
> From: Tim Gore <tim.gore@intel.com>
>
> The call to low_mem_killer_disable(true) was being done
> from within function oom_adjust_for_doom. However,
> oom_adjust_for_doom gets called from 3 places. We only
> want the call to low_mem_killer_disable(true) to happen
> during common_init, so call it from here instead of from
> oom_adjust_for_doom.
>
> v2:Thomas Wood pointed out that the initial call to disable
> the low_mem_killer does not get made when we are just
> listing subtests; so I have qualified the call from the
> exit handler, which re-enables the low_mem_killer, with
> if (!igt_only_list_subtests()).
> For belt and braces I have also made low_mem_killer_disable
> idempotent, so multiple calls to disable or re-enable are
> safe.
>
> Signed-off-by: Tim Gore <tim.gore@intel.com>
> ---
>  lib/igt_core.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 7d04f2c..15f3922 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -396,6 +396,7 @@ static void low_mem_killer_disable(bool disable)
>         /* 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
> @@ -408,7 +409,7 @@ static void low_mem_killer_disable(bool disable)
>         /* make sure the file can be read/written - by default it is write-only */
>         chmod(adj_fname, S_IRUSR | S_IWUSR);
>
> -       if (disable) {
> +       if (disable && !is_disabled) {
>                 /* read the current oom adj parameters for lowmemorykiller */
>                 fd = open(adj_fname, O_RDWR);
>                 igt_assert(fd != -1);
> @@ -421,13 +422,15 @@ static void low_mem_killer_disable(bool disable)
>                 igt_assert_eq(write(fd, no_lowmem_killer, sizeof(no_lowmem_killer)),
>                               sizeof(no_lowmem_killer));
>                 close(fd);
> -       } else {
> +               is_disabled = true;
> +       } else if (is_disabled) {
>                 /* just re-enstate the original settings */
>                 fd = open(adj_fname, O_WRONLY);
>                 igt_assert(fd != -1);
>                 igt_assert_eq(write(fd, prev_adj_scores, adj_scores_len),
>                               adj_scores_len);
>                 close(fd);
> +               is_disabled = false;
>         }
>
>         /* re-enstate the file permissions */
> @@ -437,7 +440,10 @@ static void low_mem_killer_disable(bool disable)
>  bool igt_exit_called;
>  static void common_exit_handler(int sig)
>  {
> -       low_mem_killer_disable(false);
> +       if (!igt_only_list_subtests())
> +       {

I've pushed this patch with just a small style fix to put the opening
brace here on the line above.


> +               low_mem_killer_disable(false);
> +       }
>
>         /* When not killed by a signal check that igt_exit() has been properly
>          * called. */
> @@ -490,7 +496,6 @@ static void oom_adjust_for_doom(void)
>         igt_assert(write(fd, always_kill, sizeof(always_kill)) == sizeof(always_kill));
>         close(fd);
>
> -       low_mem_killer_disable(true);
>  }
>
>  static int common_init(int *argc, char **argv,
> @@ -653,6 +658,7 @@ out:
>                 print_version();
>
>                 oom_adjust_for_doom();
> +               low_mem_killer_disable(true);
>         }
>
>         /* install exit handler, to ensure we clean up */
> --
> 2.3.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx