Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1427320 > unrolled thread

[PATCH 1/3] staging: lowmemorykiller: change lowmem_adj to lowmem_score_adj

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] staging: lowmemorykiller: change lowmem_adj to lowmem_score_adj Ganesh Mahendran <opensource.ganesh@gmail.com> - 2016-06-21 07:40 +0200
    [PATCH 3/3] staging: lowmemorykiller: select the task with maximum rss to kill Ganesh Mahendran <opensource.ganesh@gmail.com> - 2016-06-21 07:40 +0200
      Re: [PATCH 3/3] staging: lowmemorykiller: select the task with  maximum rss to kill David Rientjes <rientjes@google.com> - 2016-06-21 23:20 +0200
        Re: [PATCH 3/3] staging: lowmemorykiller: select the task with  maximum rss to kill Ganesh Mahendran <opensource.ganesh@gmail.com> - 2016-06-22 06:50 +0200
    Re: [PATCH 1/3] staging: lowmemorykiller: change lowmem_adj to  lowmem_score_adj David Rientjes <rientjes@google.com> - 2016-06-21 22:40 +0200
      Re: [PATCH 1/3] staging: lowmemorykiller: change lowmem_adj to  lowmem_score_adj Ganesh Mahendran <opensource.ganesh@gmail.com> - 2016-06-22 06:10 +0200

#1427320 — [PATCH 1/3] staging: lowmemorykiller: change lowmem_adj to lowmem_score_adj

om_adj is deprecated, and in lowmemorykiller module, we use score adj
to do the comparing.
---
                oom_score_adj = p->signal->oom_score_adj;
                if (oom_score_adj < min_score_adj) {
                        task_unlock(p);
                        continue;
                }
---

This patch makes the variable name consistent with the usage.

Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
---
 drivers/staging/android/lowmemorykiller.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
index 24d2745..6da9260 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -7,9 +7,9 @@
  * /sys/module/lowmemorykiller/parameters/minfree. Both files take a comma
  * separated list of numbers in ascending order.
  *
- * For example, write "0,8" to /sys/module/lowmemorykiller/parameters/adj and
- * "1024,4096" to /sys/module/lowmemorykiller/parameters/minfree to kill
- * processes with a oom_score_adj value of 8 or higher when the free memory
+ * For example, write "0,470" to /sys/module/lowmemorykiller/parameters/score_adj
+ * and "1024,4096" to /sys/module/lowmemorykiller/parameters/minfree to kill
+ * processes with a oom_score_adj value of 470 or higher when the free memory
  * drops below 4096 pages and kill processes with a oom_score_adj value of 0 or
  * higher when the free memory drops below 1024 pages.
  *
@@ -44,14 +44,15 @@
 #include <linux/notifier.h>
 
 static u32 lowmem_debug_level = 1;
-static short lowmem_adj[6] = {
+
+static short lowmem_score_adj[6] = {
 	0,
-	1,
-	6,
-	12,
+	58,
+	352,
+	705,
 };
 
-static int lowmem_adj_size = 4;
+static int lowmem_score_adj_size = 4;
 static int lowmem_minfree[6] = {
 	3 * 512,	/* 6MB */
 	2 * 1024,	/* 8MB */
@@ -89,20 +90,20 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	int minfree = 0;
 	int selected_tasksize = 0;
 	short selected_oom_score_adj;
-	int array_size = ARRAY_SIZE(lowmem_adj);
+	int array_size = ARRAY_SIZE(lowmem_score_adj);
 	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
 	int other_file = global_page_state(NR_FILE_PAGES) -
 						global_page_state(NR_SHMEM) -
 						total_swapcache_pages();
 
-	if (lowmem_adj_size < array_size)
-		array_size = lowmem_adj_size;
+	if (lowmem_score_adj_size < array_size)
+		array_size = lowmem_score_adj_size;
 	if (lowmem_minfree_size < array_size)
 		array_size = lowmem_minfree_size;
 	for (i = 0; i < array_size; i++) {
 		minfree = lowmem_minfree[i];
 		if (other_free < minfree && other_file < minfree) {
-			min_score_adj = lowmem_adj[i];
+			min_score_adj = lowmem_score_adj[i];
 			break;
 		}
 	}
@@ -165,7 +166,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 		if (selected->mm)
 			task_set_lmk_waiting(selected);
 		task_unlock(selected);
-		lowmem_print(1, "Killing '%s' (%d), adj %hd,\n"
+		lowmem_print(1, "Killing '%s' (%d), score adj %hd,\n"
 				 "   to free %ldkB on behalf of '%s' (%d) because\n"
 				 "   cache %ldkB is below limit %ldkB for oom_score_adj %hd\n"
 				 "   Free memory is %ldkB above reserved\n",
@@ -205,7 +206,7 @@ device_initcall(lowmem_init);
  * bootargs behaviour is to continue using module_param here.
  */
 module_param_named(cost, lowmem_shrinker.seeks, int, S_IRUGO | S_IWUSR);
-module_param_array_named(adj, lowmem_adj, short, &lowmem_adj_size,
+module_param_array_named(score_adj, lowmem_score_adj, short, &lowmem_score_adj_size,
 			 S_IRUGO | S_IWUSR);
 module_param_array_named(minfree, lowmem_minfree, uint, &lowmem_minfree_size,
 			 S_IRUGO | S_IWUSR);
-- 
1.9.1

[toc] | [next] | [standalone]

#1427322 — [PATCH 3/3] staging: lowmemorykiller: select the task with maximum rss to kill

Current task selecting logic in LMK does not fully aware of the memory
pressure. It may select the task with maximum score adj, but with
least tasksize.

For example, if min_score_adj is 200, and there are 2 tasks in system:
   task a: score adj 500, tasksize 200M
   task b: score adj 1000, tasksize 1M
Current LMK logic will select *task b*. But now the system already have
much memory pressure.

We should select the task with maximum task from all the tasks which
score adj >= min_score_adj.

Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
---
 drivers/staging/android/lowmemorykiller.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
index 1d8de47..5fcfcfe 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -122,8 +122,6 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 		return 0;
 	}
 
-	selected_oom_score_adj = min_score_adj;
-
 	rcu_read_lock();
 	for_each_process(tsk) {
 		struct task_struct *p;
@@ -151,18 +149,19 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 		task_unlock(p);
 		if (tasksize <= 0)
 			continue;
-		if (selected) {
-			if (oom_score_adj < selected_oom_score_adj)
-				continue;
-			if (oom_score_adj == selected_oom_score_adj &&
-			    tasksize <= selected_tasksize)
-				continue;
+
+		/*
+		 * From the processes which score adj >= min_score_adj,
+		 * we select the one with the maximum tasksize.
+		 */
+		if (selected_tasksize < tasksize) {
+			selected = p;
+			selected_tasksize = tasksize;
+			selected_oom_score_adj = oom_score_adj;
+
+			lowmem_print(2, "select '%s' (%d), adj %hd, size %d, to kill\n",
+					 p->comm, p->pid, oom_score_adj, tasksize);
 		}
-		selected = p;
-		selected_tasksize = tasksize;
-		selected_oom_score_adj = oom_score_adj;
-		lowmem_print(2, "select '%s' (%d), adj %hd, size %d, to kill\n",
-			     p->comm, p->pid, oom_score_adj, tasksize);
 	}
 	if (selected) {
 		task_lock(selected);
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1428173 — Re: [PATCH 3/3] staging: lowmemorykiller: select the task with maximum rss to kill

On Tue, 21 Jun 2016, Ganesh Mahendran wrote:

> Current task selecting logic in LMK does not fully aware of the memory
> pressure. It may select the task with maximum score adj, but with
> least tasksize.
> 
> For example, if min_score_adj is 200, and there are 2 tasks in system:
>    task a: score adj 500, tasksize 200M
>    task b: score adj 1000, tasksize 1M
> Current LMK logic will select *task b*. But now the system already have
> much memory pressure.
> 
> We should select the task with maximum task from all the tasks which
> score adj >= min_score_adj.
> 

Unfortunately, I'm not sure that we can get away with this although I 
agree that it is a better result (kill a large process, avoid lowmem or 
oom for longer).

It changes the kill order for systems that have already fine-tuned their 
oom_score_adj settings and can regress because of this change.  If systems 
really want task b to be killed above, this breaks and they have no 
immediate way of fixing it.

[toc] | [prev] | [next] | [standalone]

#1428408 — Re: [PATCH 3/3] staging: lowmemorykiller: select the task with maximum rss to kill

Hi, David:

On Tue, Jun 21, 2016 at 01:14:36PM -0700, David Rientjes wrote:
> On Tue, 21 Jun 2016, Ganesh Mahendran wrote:
> 
> > Current task selecting logic in LMK does not fully aware of the memory
> > pressure. It may select the task with maximum score adj, but with
> > least tasksize.
> > 
> > For example, if min_score_adj is 200, and there are 2 tasks in system:
> >    task a: score adj 500, tasksize 200M
> >    task b: score adj 1000, tasksize 1M
> > Current LMK logic will select *task b*. But now the system already have
> > much memory pressure.
> > 
> > We should select the task with maximum task from all the tasks which
> > score adj >= min_score_adj.
> > 
> 
> Unfortunately, I'm not sure that we can get away with this although I 
> agree that it is a better result (kill a large process, avoid lowmem or 
> oom for longer).

Yes, from our testing with this patch applied, system works more smoothly,
and user have better experience.

> 
> It changes the kill order for systems that have already fine-tuned their 
> oom_score_adj settings and can regress because of this change.  If systems 
> really want task b to be killed above, this breaks and they have no 
> immediate way of fixing it.

I think the processes with score_adj >= min_score_adj are all acceptable
if we kill them. In android products, LMK does the main job to free memory
when system is hard to shrink file/anon pages. If LMK does not free enough memory,
the system will be very slow before OOM is triggered. During this period, user will
have bad experience.

So LMK need to work effectivly to let system running smoothly, as user experience
is very important for android system.

Thanks.

[toc] | [prev] | [next] | [standalone]

#1428140 — Re: [PATCH 1/3] staging: lowmemorykiller: change lowmem_adj to lowmem_score_adj

On Tue, 21 Jun 2016, Ganesh Mahendran wrote:

> om_adj is deprecated, and in lowmemorykiller module, we use score adj
> to do the comparing.
> ---
>                 oom_score_adj = p->signal->oom_score_adj;
>                 if (oom_score_adj < min_score_adj) {
>                         task_unlock(p);
>                         continue;
>                 }
> ---
> 
> This patch makes the variable name consistent with the usage.
> 

Umm, I don't think you can just remove a parameter to a module and replace 
it with something that has a different unit and not think that userspace 
will break as a result.

[toc] | [prev] | [next] | [standalone]

#1428399 — Re: [PATCH 1/3] staging: lowmemorykiller: change lowmem_adj to lowmem_score_adj

Hi, David:

On Tue, Jun 21, 2016 at 01:27:40PM -0700, David Rientjes wrote:
> On Tue, 21 Jun 2016, Ganesh Mahendran wrote:
> 
> > om_adj is deprecated, and in lowmemorykiller module, we use score adj
> > to do the comparing.
> > ---
> >                 oom_score_adj = p->signal->oom_score_adj;
> >                 if (oom_score_adj < min_score_adj) {
> >                         task_unlock(p);
> >                         continue;
> >                 }
> > ---
> > 
> > This patch makes the variable name consistent with the usage.
> > 
> 
> Umm, I don't think you can just remove a parameter to a module and replace 
> it with something that has a different unit and not think that userspace 
> will break as a result.

You are right, this change will break android AMS which will set the LMK
watermark via /sys/module/lowmemorykiller/parameters/adj.

Please help to review below change. Only make the varialbe name consistent
with the variable usage.

------
From 394872fc1993a04ae471b10d7f971d4544812ec4 Mon Sep 17 00:00:00 2001
From: Ganesh Mahendran <opensource.ganesh@gmail.com>
Date: Wed, 22 Jun 2016 10:53:13 +0800
Subject: [PATCH v2] staging: lowmemorykiller: make variable name consistent with
 the varialbe usage

LMK use oom_score_adj to do the comparing. But the variable name is
*_adj. This patch makes thevarialbe name consistent with the varialb
usage to avoid ambiguity.

*_adj -> *_score_adj

Signed-off-by: Ganesh Mahendran <opensource.ganesh@gmail.com>
---
v2:
  do not change user API - David
---
 drivers/staging/android/lowmemorykiller.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/android/lowmemorykiller.c b/drivers/staging/android/lowmemorykiller.c
index 24d2745..6568bbf 100644
--- a/drivers/staging/android/lowmemorykiller.c
+++ b/drivers/staging/android/lowmemorykiller.c
@@ -44,14 +44,15 @@
 #include <linux/notifier.h>
 
 static u32 lowmem_debug_level = 1;
-static short lowmem_adj[6] = {
+
+static short lowmem_score_adj[6] = {
 	0,
-	1,
-	6,
-	12,
+	58,
+	352,
+	705,
 };
 
-static int lowmem_adj_size = 4;
+static int lowmem_score_adj_size = 4;
 static int lowmem_minfree[6] = {
 	3 * 512,	/* 6MB */
 	2 * 1024,	/* 8MB */
@@ -89,20 +90,20 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 	int minfree = 0;
 	int selected_tasksize = 0;
 	short selected_oom_score_adj;
-	int array_size = ARRAY_SIZE(lowmem_adj);
+	int array_size = ARRAY_SIZE(lowmem_score_adj);
 	int other_free = global_page_state(NR_FREE_PAGES) - totalreserve_pages;
 	int other_file = global_page_state(NR_FILE_PAGES) -
 						global_page_state(NR_SHMEM) -
 						total_swapcache_pages();
 
-	if (lowmem_adj_size < array_size)
-		array_size = lowmem_adj_size;
+	if (lowmem_score_adj_size < array_size)
+		array_size = lowmem_score_adj_size;
 	if (lowmem_minfree_size < array_size)
 		array_size = lowmem_minfree_size;
 	for (i = 0; i < array_size; i++) {
 		minfree = lowmem_minfree[i];
 		if (other_free < minfree && other_file < minfree) {
-			min_score_adj = lowmem_adj[i];
+			min_score_adj = lowmem_score_adj[i];
 			break;
 		}
 	}
@@ -165,7 +166,7 @@ static unsigned long lowmem_scan(struct shrinker *s, struct shrink_control *sc)
 		if (selected->mm)
 			task_set_lmk_waiting(selected);
 		task_unlock(selected);
-		lowmem_print(1, "Killing '%s' (%d), adj %hd,\n"
+		lowmem_print(1, "Killing '%s' (%d), score adj %hd,\n"
 				 "   to free %ldkB on behalf of '%s' (%d) because\n"
 				 "   cache %ldkB is below limit %ldkB for oom_score_adj %hd\n"
 				 "   Free memory is %ldkB above reserved\n",
@@ -205,7 +206,7 @@ device_initcall(lowmem_init);
  * bootargs behaviour is to continue using module_param here.
  */
 module_param_named(cost, lowmem_shrinker.seeks, int, S_IRUGO | S_IWUSR);
-module_param_array_named(adj, lowmem_adj, short, &lowmem_adj_size,
+module_param_array_named(adj, lowmem_score_adj, short, &lowmem_score_adj_size,
 			 S_IRUGO | S_IWUSR);
 module_param_array_named(minfree, lowmem_minfree, uint, &lowmem_minfree_size,
 			 S_IRUGO | S_IWUSR);
-- 
1.9.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web