lowmemory android driver not needed?

Hi San,

Alan Cox pointed me at the /proc/<pid>/oom_adj file that controls the oom-killer score for any process as being more than sufficent to control the oom killer.

This makes me wonder why you wrote the android lowmemlorykiller driver?

What is that driver for that is not already present in the existing oom_* values for every process?

thanks,

greg k-h

Looping in Arve who wrote the low memory killer and can explain things in more detail.

Brian

[Greg KH <[email protected]>]

Quoted 6 lines

Hi San,

Alan Cox pointed me at the /proc/<pid>/oom_adj file that controls the oom-killer score for any process as being more than sufficent to control the oom killer.

Quoted 8 lines

This makes me wonder why you wrote the android lowmemlorykiller driver?

What is that driver for that is not already present in the existing oom_* values for every process?

thanks,

greg k-h

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

The oom killer does not kick in until all caches are emptied. Our user space code changes the oom_adj value of processes that are no longer in the foreground so that they killed first (the process saves its state but does not exit). To avoid excessive demand paging, the low memory killer will kill these processes when the memory available drops below a threshold.

Quoted 6 lines · Arve Hj?nnev?g

The oom killer does not kick in until all caches are emptied. Our user space code changes the oom_adj value of processes that are no longer in the foreground so that they killed first (the process saves its state but does not exit). To avoid excessive demand paging, the low memory killer will kill these processes when the memory available drops below a threshold.

That makes sense. Can you provide a bit of documentation that I can include in the driver so that people can actually use the thing? :)

Alan, does this sound like it should remain in the tree?

Quoted 1 line

Alan, does this sound like it should remain in the tree?

Maybe our oom killer should get a new tunable, telling it how aggressive it should be, instead?

Quoted 4 lines

Quoted 1 line

Alan, does this sound like it should remain in the tree?

Maybe our oom killer should get a new tunable, telling it how aggressive it should be, instead?

I was thinking that, and it would integrate better with the OLPC work (which IMHO is a nicer interface for some stuff)

You'd want two thresholds

The 'arghhhh....' point where you start killing stuff
The 'uh oh...' point where an OLPC style low memory notifier kicks in

(OLPC's model is a handle you can select/poll for 'memory getting low' so apps can respond to pressure by doing stuff like dumping caches)

The rest ought to follow naturally IFF you can find a clean efficient way to measure that pressure and quantify it as a number. Our default would be like now, the Android default might be to trigger earlier.. --
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 2 lines

Maybe our oom killer should get a new tunable, telling it how aggressive it should be, instead?

I was thinking that, and it would integrate better with the OLPC work (which IMHO is a nicer interface for some stuff)

You'd want two thresholds

The 'arghhhh....' point where you start killing stuff
The 'uh oh...' point where an OLPC style low memory notifier kicks in

Quoted 10 lines · Alan Cox

Quoted 2 lines

Maybe our oom killer should get a new tunable, telling it how aggressive it should be, instead?

I was thinking that, and it would integrate better with the OLPC work (which IMHO is a nicer interface for some stuff)

You'd want two thresholds

The 'arghhhh....' point where you start killing stuff
The 'uh oh...' point where an OLPC style low memory notifier kicks in

We actually use 6 different thresholds for killing processes. I don't know what all the classes are, processes with a higher oom_adj value can be killed with less impact to the user than processes with a lower oom_adj value. The first few classes only affect latency when switching apps, but later classes stop non critical background services and finally the foreground app. Another reason to not kill every process at the same threshold is that memory may not be free immediately when the process is killed.

Quoted 8 lines

We actually use 6 different thresholds for killing processes. I don't know what all the classes are, processes with a higher oom_adj value can be killed with less impact to the user than processes with a lower oom_adj value. The first few classes only affect latency when switching apps, but later classes stop non critical background services and finally the foreground app. Another reason to not kill every process at the same threshold is that memory may not be free immediately when the process is killed.

But the lowmemorykiller android module doesn't have anything to do with this, right?

Quoted 9 lines · Greg KH

Quoted 8 lines

We actually use 6 different thresholds for killing processes. I don't know what all the classes are, processes with a higher oom_adj value can be killed with less impact to the user than processes with a lower oom_adj value. The first few classes only affect latency when switching apps, but later classes stop non critical background services and finally the foreground app. Another reason to not kill every process at the same threshold is that memory may not be free immediately when the process is killed.

Quoted 3 lines

But the lowmemorykiller android module doesn't have anything to do with this, right?

It does. We write the thresholds to /sys/module/lowmemorykiller/parameters/adj and /sys/module/lowmemorykiller/parameters/minfree then set the oom_adj value per process. If the standard oom killer can be adjusted in a similar way, then we will not need the lowmemorykiller module.

Signed-off-by Arve Hjønnevåg <[email protected]>

Quoted 14 lines · Arve Hjønnevåg

Quoted 12 lines · Greg KH

Quoted 8 lines

We actually use 6 different thresholds for killing processes. I don't know what all the classes are, processes with a higher oom_adj value can be killed with less impact to the user than processes with a lower oom_adj value. The first few classes only affect latency when switching apps, but later classes stop non critical background services and finally the foreground app. Another reason to not kill every process at the same threshold is that memory may not be free immediately when the process is killed.

But the lowmemorykiller android module doesn't have anything to do with this, right?

Quoted 5 lines

It does. We write the thresholds to /sys/module/lowmemorykiller/parameters/adj and /sys/module/lowmemorykiller/parameters/minfree then set the oom_adj value per process. If the standard oom killer can be adjusted in a similar way, then we will not need the lowmemorykiller module.

Great, care to document this somewhere so people like me don't get confused?

Hi Greg,

Quoted 21 lines · Greg KH

Quoted 19 lines · Arve Hjønnevåg

Quoted 9 lines · Greg KH

Quoted 8 lines

We actually use 6 different thresholds for killing processes. I don't know what all the classes are, processes with a higher oom_adj value can be killed with less impact to the user than processes with a lower oom_adj value. The first few classes only affect latency when switching apps, but later classes stop non critical background services and finally the foreground app. Another reason to not kill every process at the same threshold is that memory may not be free immediately when the process is killed.

Quoted 3 lines

But the lowmemorykiller android module doesn't have anything to do with this, right?

It does. We write the thresholds to /sys/module/lowmemorykiller/parameters/adj and /sys/module/lowmemorykiller/parameters/minfree then set the oom_adj value per process. If the standard oom killer can be adjusted in a similar way, then we will not need the lowmemorykiller module.

Quoted 2 lines

Great, care to document this somewhere so people like me don't get confused?

And there is one more lowmem driver developed by Nokia for Nokia 8xx tablets it seems. CCed Tony Lindgren, Juha and Viktor.

http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=blob;f=security/lowmem.c;h=ae78a530af39703e335ad769f1e6f097f63ec6dd;hb=HEAD

Quoted 4 lines · Trilok Soni

And there is one more lowmem driver developed by Nokia for Nokia 8xx tablets it seems. CCed Tony Lindgren, Juha and Viktor.

http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=blob;f=security/lowmem.c;h=ae78a530af39703e335ad769f1e6f097f63ec6dd;hb=HEAD

As we can't stack LSMs, using the lsm interface for a simple memory driver seems pretty wasteful :)

Quoted 5 lines

Quoted 4 lines

And there is one more lowmem driver developed by Nokia for Nokia 8xx tablets it seems. CCed Tony Lindgren, Juha and Viktor.

http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=blob;f=security/lowmem.c;h=ae78a530af39703e335ad769f1e6f097f63ec6dd;hb=HEAD

Quoted 3 lines

As we can't stack LSMs, using the lsm interface for a simple memory driver seems pretty wasteful :)

The heuristics and tunables are more what ended up being useful with this module, which could trivially be abstracted out. The focus of the lowmem module was mostly giving userspace an opportunity to change its behaviour, and to try to save critical state. I don't know how well this would map to the Android use cases, though.

As far as I know, embedded guys strong want to lowmem notification mecanism.

At least, I and my mem_notify receive multiple contact from embedded and JavaVM developer.
(include sun javavm engineer)

In ideal, I think linux MM should care this requirement directly.

LSM and driver notifier is easy breakable because these component deeply depend on MM.

(eg, I developed /dev/mem_notify patch last year. but this patch don't work on 2.6.28 because split-lru patch series totally changed MM reclaim processing.)

Unfortunately, we don't have any consensus of memory notification requirement. various people have various requirement. so, if I can discuss it and we get consensus, I'm glad.

Quoted 2 lines

As far as I know, embedded guys strong want to lowmem notification mecanism.

I think the big server guys also want the same thing :)

Quoted 5 lines

At least, I and my mem_notify receive multiple contact from embedded and JavaVM developer.
(include sun javavm engineer)

In ideal, I think linux MM should care this requirement directly.

I agree.

Quoted 2 lines

LSM and driver notifier is easy breakable because these component deeply depend on MM.

Agreed.

Quoted 4 lines

(eg, I developed /dev/mem_notify patch last year. but this patch don't work on 2.6.28 because split-lru patch series totally changed MM reclaim processing.)

Quoted 3 lines

Unfortunately, we don't have any consensus of memory notification requirement. various people have various requirement. so, if I can discuss it and we get consensus, I'm glad.

Care to work on your mem_notify patch again and bring it up to date? That would be a good place to start working from, right?

I should rewrite memory notification patchset from scratch. the new version will construct on memcg infrastrcture.

Why?

last year, I received many feedback from lkml folks and my article reader. (I monthly write kernel patch introduction article to japanese web
magazine and receive some feedback periodically)

I learned many people want flexibility notification. (per workload, per user, et al)
eg. nokia lowmem driver have exception process defined by uid.

at top of last year, I thought memcg don't provide good infrastructure. the first version memcg is just pretty funny joke. if its config turn on, memory workload performance decrease ~30% although the user don't use memcg at runtime. then nobody use it.
but recently, KAMEZAWA hiroyuki (and Li zefan, Daisuke Nishimura et al) dramatically improvement memcg performance.
now, memcg overhead become less than 1%.

Then, I think any memory accounting activity should use this infrastructure. That's my homework.

last year, I received many feedback from lkml folks and my article reader. (I monthly write kernel patch introduction article to japanese web
magazine and receive some feedback periodically)

I learned many people want flexibility notification. (per workload, per user, et al)
eg. nokia lowmem driver have exception process defined by uid.

at top of last year, I thought memcg don't provide good infrastructure. the first version memcg is just pretty funny joke. if its config turn on, memory workload performance decrease ~30% although the user don't use memcg at runtime. then nobody use it.
but recently, KAMEZAWA hiroyuki (and Li zefan, Daisuke Nishimura et al) dramatically improvement memcg performance.
now, memcg overhead become less than 1%.

Then, I think any memory accounting activity should use this infrastructure. That's my homework.

Quoted 3 lines

Care to work on your mem_notify patch again and bring it up to date? That would be a good place to start working from, right?

Unfortunately No ;)

I should rewrite memory notification patchset from scratch. the new version will construct on memcg infrastrcture.

Why?

last year, I received many feedback from lkml folks and my article reader. (I monthly write kernel patch introduction article to japanese web magazine and receive some feedback periodically)

I learned many people want flexibility notification. (per workload, per user, et al)
eg. nokia lowmem driver have exception process defined by uid.

at top of last year, I thought memcg don't provide good infrastructure. the first version memcg is just pretty funny joke. if its config turn on, memory workload performance decrease ~30% although the user don't use memcg at runtime. then nobody use it.
but recently, KAMEZAWA hiroyuki (and Li zefan, Daisuke Nishimura et al) dramatically improvement memcg performance.
now, memcg overhead become less than 1%.

Then, I think any memory accounting activity should use this infrastructure. That's my homework.

Any updates on top of mem_notify v6 patches? Is there any WIP for mem_notify with memcg infrastructure as you have pointed out above?

Quoted 6 lines · Greg KH

Quoted 4 lines · Trilok Soni

And there is one more lowmem driver developed by Nokia for Nokia 8xx tablets it seems. CCed Tony Lindgren, Juha and Viktor.

http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=blob;f=security/lowmem.c;h=ae78a530af39703e335ad769f1e6f097f63ec6dd;hb=HEAD

Quoted 3 lines

As we can't stack LSMs, using the lsm interface for a simple memory driver seems pretty wasteful :)

The heuristics and tunables are more what ended up being useful with this module, which could trivially be abstracted out. The focus of the lowmem module was mostly giving userspace an opportunity to change its behaviour, and to try to save critical state. I don't know how well this would map to the Android use cases, though.

And there is one more lowmem driver developed by Nokia for Nokia 8xx tablets it seems. CCed Tony Lindgren, Juha and Viktor.

http://git.kernel.org/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=blob;f=security/lowmem.c;h=ae78a530af39703e335ad769f1e6f097f63ec6dd;hb=HEAD

325 static struct security_operations lowmem_security_ops = {

326         /* Use the capability functions for some of the hooks */
327         .ptrace_may_access = cap_ptrace_may_access,
328         .ptrace_traceme = cap_ptrace_traceme,
329         .capget = cap_capget,
330         .capset_check = cap_capset_check,
331         .capset_set = cap_capset_set,
332         .capable = cap_capable,

333
334 .bprm_apply_creds = cap_bprm_apply_creds,
335 .bprm_set_security = cap_bprm_set_security,
336

337         .task_post_setuid = cap_task_post_setuid,
338         .task_reparent_to_init = cap_task_reparent_to_init,
339         .vm_enough_memory = low_vm_enough_memory,

340 };

57 static ctl_table lowmem_table[] = {
58 {

59                 .ctl_name = VM_LOWMEM_DENY_PAGES,
60                 .procname = "lowmem_deny_watermark_pages",
61                 .data = &deny_pages,
62                 .maxlen = sizeof(long),
63                 .mode = 0644,
64                 .child = NULL,
65                 .proc_handler = &proc_dointvec,
66                 .strategy = &sysctl_intvec,

67 }, {

249 static int low_vm_enough_memory(struct mm_struct *mm, long pages) 250 {
251 unsigned long free, allowed;
252 int cap_sys_admin = 0, notify;
253
254 if (cap_capable(current, CAP_SYS_ADMIN) == 0)
255 cap_sys_admin = 1;
256
257 allowed = totalram_pages - hugetlb_total_pages();
258 allowed_pages = allowed;
259
260 /* We activate ourselves only after both parameters have been

261 * configured. */
262 if (deny_pages == 0 || notify_low_pages == 0 || notify_high_pages == 0)
263 return __vm_enough_memory(mm, pages, cap_sys_admin);
264
265 vm_acct_memory(pages);
266
267 /* Easily freed pages when under VM pressure or direct reclaim */
268 free = global_page_state(NR_FILE_PAGES);

269 free += nr_swap_pages;
270 free += global_page_state(NR_SLAB_RECLAIMABLE);
271
272 if (likely(free > notify_low_pages))
273 goto enough_memory;
274
275 /* No luck, lets make it more expensive and try again.. */
276 free += nr_free_pages();
277
278 if (free < deny_pages) {
279 int i;
280

281                 lowmem_free_pages = free;
282                 low_watermark_state(1);
283                 high_watermark_state(1);
284                 /* Memory allocations by root are always allowed */
285                 if (cap_sys_admin)

286 return 0;
287
288 /* OOM unkillable process is allowed to consume memory */
289 if (current->oomkilladj == OOM_DISABLE)
290 return 0;

292 /* uids from allowed_uids vector are also allowed no matter what */
293 for (i = 0; i < LOWMEM_MAX_UIDS && allowed_uids[i]; i++)
294 if (current->uid == allowed_uids[i])
295 return 0;

297 vm_unacct_memory(pages);
298 if (printk_ratelimit()) {
299 printk(MY_NAME ": denying memory allocation to process %d (%s> )\n",
300 current->pid, current->comm);
301 }
302 return -ENOMEM;
303 }
304
305 enough_memory:
306 /* See if we need to notify level 1 */
307 low_watermark_state(free < notify_low_pages);
308
309 /*

310          * In the level 2 notification case things are more complicated,
311          * as the level that we drop the state and send a notification
312          * should be lower than when it is first triggered. Having this
313          * on the same watermark level ends up bouncing back and forth
314          * when applications are being stupid.
315          */

316 notify = free < notify_high_pages;
317 if (notify || free - nr_decay_pages > notify_high_pages)
318 high_watermark_state(notify);
319
320 /* We have plenty of memory */
321 lowmem_free_pages = free;

322 return 0;
323 }

#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/mm.h>
#include <linux/oom.h>
#include <linux/sched.h>

static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask);

static struct shrinker lowmem_shrinker = {
.shrink = lowmem_shrink,
.seeks = DEFAULT_SEEKS * 16
};

static uint32_t lowmem_debug_level = 2;
static int lowmem_adj[6] = {

        0,
        1,
        6,
        12,
};

static int lowmem_adj_size = 4;

static size_t lowmem_minfree[6] = {
        3*512, // 6MB
        2*1024, // 8MB
        4*1024, // 16MB
        16*1024, // 64MB
};

static int lowmem_minfree_size = 4;

#define lowmem_print(level, x...) do { if(lowmem_debug_level >= (level)) printk(x); } while(0)

module_param_named(cost, lowmem_shrinker.seeks, int, S_IRUGO | S_IWUSR); module_param_array_named(adj, lowmem_adj, int, &lowmem_adj_size, S_IRUGO | S_IWUSR); module_param_array_named(minfree, lowmem_minfree, uint, &lowmem_minfree_size, S_IRUGO | S_IWUSR); module_param_named(debug_level, lowmem_debug_level, uint, S_IRUGO | S_IWUSR);

static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)

{
        struct task_struct *p;
        struct task_struct *selected = NULL;
        int rem = 0;
        int tasksize;
        int i;
        int min_adj = OOM_ADJUST_MAX + 1;
        int selected_tasksize = 0;
        int array_size = ARRAY_SIZE(lowmem_adj);
        int other_free = global_page_state(NR_FREE_PAGES) + global_page_state(NR_FILE_PAGES);

        if(lowmem_adj_size < array_size)
                array_size = lowmem_adj_size;
        if(lowmem_minfree_size < array_size)
                array_size = lowmem_minfree_size;
        for(i = 0; i < array_size; i++) {
                if(other_free < lowmem_minfree[i]) {
                        min_adj = lowmem_adj[i];
                        break;

                }
        }


        if(nr_to_scan > 0)
                lowmem_print(3, "lowmem_shrink %d, %x, ofree %d, ma %d\n", nr_to_scan, gfp_mask, other_fr> ee, min_adj);
        read_lock(&tasklist_lock);
        for_each_process(p) {

                if(p->oomkilladj >= 0 && p->mm) {
                        tasksize = get_mm_rss(p->mm);
                        if(nr_to_scan > 0 && tasksize > 0 && p->oomkilladj >= min_adj) {
                                if(selected == NULL ||
                                   p->oomkilladj > selected->oomkilladj ||
                                   (p->oomkilladj == selected->oomkilladj &&

                                    tasksize > selected_tasksize)) {
                                        selected = p;
                                        selected_tasksize = tasksize;
                                        lowmem_print(2, "select %d (%s), adj %d, size %d, to kill\n",
                                                     p->pid, p->comm, p->oomkilladj, tasksize);
                                }
                        }
                        rem += tasksize;

                }
        }
        if(selected != NULL) {
                lowmem_print(1, "send sigkill to %d (%s), adj %d, size %d\n",
                             selected->pid, selected->comm,
                             selected->oomkilladj, selected_tasksize);
                force_sig(SIGKILL, selected);
                rem -= selected_tasksize;
        }
        lowmem_print(4, "lowmem_shrink %d, %x, return %d\n", nr_to_scan, gfp_mask, rem);
        read_unlock(&tasklist_lock);
        return rem;
}

also quick review to lowmemorykiller.c

Quoted 4 lines

static struct shrinker lowmem_shrinker = {
.shrink = lowmem_shrink,
.seeks = DEFAULT_SEEKS * 16
};

why do you choice *16?

Quoted 3 lines

static uint32_t lowmem_debug_level = 2;
static int lowmem_adj[6] = {

why do you choice [6]?

Quoted 11 lines

static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)

{
        struct task_struct *p;
        struct task_struct *selected = NULL;
        int rem = 0;
        int tasksize;
        int i;
        int min_adj = OOM_ADJUST_MAX + 1;
        int selected_tasksize = 0;
        int array_size = ARRAY_SIZE(lowmem_adj);
        int other_free = global_page_state(NR_FREE_PAGES) + global_page_state(NR_FILE_PAGES);

I think you don't consider mlocked page (and/or other unevictable page). if much mlocked file page exist, other_free can become large value. then, this routine don't kill any process.

Quoted 2 lines

        read_lock(&tasklist_lock);
        for_each_process(p) {

Oops. too long locking.
shrink_slab() is freqentlly called function. I don't like costly operation.

Quoted 6 lines

                if(p->oomkilladj >= 0 && p->mm) {
                        tasksize = get_mm_rss(p->mm);
                        if(nr_to_scan > 0 && tasksize > 0 && p->oomkilladj >= min_adj) {
                                if(selected == NULL ||
                                   p->oomkilladj > selected->oomkilladj ||
                                   (p->oomkilladj == selected->oomkilladj &&

Quoted 8 lines

                                    tasksize > selected_tasksize)) {
                                        selected = p;
                                        selected_tasksize = tasksize;
                                        lowmem_print(2, "select %d (%s), adj %d, size %d, to kill\n",
                                                     p->pid, p->comm, p->oomkilladj, tasksize);
                                }
                        }
                        rem += tasksize;

this code mean,
shrinker->shrink(0, gfp_mask) indicate to recalculate total rss every time. it is too CPU and battery wasting.

Quoted 7 lines · KOSAKI Motohiro

also quick review to lowmemorykiller.c

Quoted 4 lines

static struct shrinker lowmem_shrinker = {
.shrink = lowmem_shrink,
.seeks = DEFAULT_SEEKS * 16
};

Quoted 1 line

why do you choice *16?

To indicate that it is more expensive to restart a killed process than a regular cache page.

Quoted 4 lines

Quoted 2 lines

static uint32_t lowmem_debug_level = 2;
static int lowmem_adj[6] = {

why do you choice [6]?

We use six levels.

Quoted 12 lines

Quoted 11 lines

static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)

{
        struct task_struct *p;
        struct task_struct *selected = NULL;
        int rem = 0;
        int tasksize;
        int i;
        int min_adj = OOM_ADJUST_MAX + 1;
        int selected_tasksize = 0;
        int array_size = ARRAY_SIZE(lowmem_adj);
        int other_free = global_page_state(NR_FREE_PAGES) + global_page_state(NR_FILE_PAGES);

Quoted 3 lines

I think you don't consider mlocked page (and/or other unevictable page). if much mlocked file page exist, other_free can become large value. then, this routine don't kill any process.

Yes, this is a problem we have seen, but I did not find counter for pinned cache pages. I may be able to use NR_UNEVICTABLE if CONFIG_UNEVICTABLE_LRU is enabled now though.

Another problem we run into is that allocations of contiguous pages can cause kswapd to empty all the caches. This will not trigger this lowmemorykiller or the regular oom killer.

Quoted 21 lines

Quoted 2 lines

        read_lock(&tasklist_lock);
        for_each_process(p) {

Oops. too long locking.
shrink_slab() is freqentlly called function. I don't like costly operation.

Quoted 6 lines

                if(p->oomkilladj >= 0 && p->mm) {
                        tasksize = get_mm_rss(p->mm);
                        if(nr_to_scan > 0 && tasksize > 0 && p->oomkilladj >= min_adj) {
                                if(selected == NULL ||
                                   p->oomkilladj > selected->oomkilladj ||
                                   (p->oomkilladj == selected->oomkilladj &&

Quoted 8 lines

                                    tasksize > selected_tasksize)) {
                                        selected = p;
                                        selected_tasksize = tasksize;
                                        lowmem_print(2, "select %d (%s), adj %d, size %d, to kill\n",
                                                     p->pid, p->comm, p->oomkilladj, tasksize);
                                }
                        }
                        rem += tasksize;

Quoted 4 lines

this code mean,
shrinker->shrink(0, gfp_mask) indicate to recalculate total rss every time. it is too CPU and battery wasting.

How about this:
----

drivers/misc/lowmemorykiller.c +0 -0

index 3715d56..3f4e41a 100644
@@ -71,6 +71,12 @@ static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
        }
        if(nr_to_scan > 0)

        read_lock(&tasklist_lock);
        for_each_process(p) {
                if(p->oomkilladj >= 0 && p->mm) {

drivers/misc/lowmemorykiller.c +0 -1

@@ -86,7 +92,6 @@ static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
                                                     p->pid, p->comm,
 p->oomkilladj, tasksize);
                                }
                        }
-                       rem += tasksize;
                }
        }

        if(selected != NULL) {

Quoted 2 lines

To indicate that it is more expensive to restart a killed process than a regular cache page.

I think you already know this answer isn't actual answer.
anybody know >1 mean expensive. a reviewer want to know why DEFAULT_SEEKS * 16 is best, not *10 nor *20.

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

static uint32_t lowmem_debug_level = 2;
static int lowmem_adj[6] = {

why do you choice [6]?

We use six levels.

if you don't consider other user and other usage case, this file can't merge forever.

The top problem is, this file stay on non proper place. then, MM folks don't review at all.

I think this patch need to receive MM folks review.

Quoted 3 lines

Another problem we run into is that allocations of contiguous pages can cause kswapd to empty all the caches. This will not trigger this lowmemorykiller or the regular oom killer.

strange usage.
you shouldn't use high order allocation on embedded machine.

Quoted 9 lines

How about this:
----

drivers/misc/lowmemorykiller.c +0 -0

index 3715d56..3f4e41a 100644
@@ -71,6 +71,12 @@ static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
        }
        if(nr_to_scan > 0)

drivers/misc/lowmemorykiller.c static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask) · line 82

Quoted 10 lines

drivers/misc/lowmemorykiller.c +6 -0

                lowmem_print(3, "lowmem_shrink %d, %x, ofree %d, ma
 %d\n", nr_to_scan, gfp_mask, other_free, min_adj);
+       rem = global_page_state(NR_ACTIVE_ANON) +
 global_page_state(NR_ACTIVE_FILE);
+       if (!nr_to_scan || min_adj == OOM_ADJUST_MAX + 1) {
+               lowmem_print(5, "lowmem_shrink %d, %x, return %d\n",
 nr_to_scan, gfp_mask, rem);
+               return rem;
+       }
+

I don't understand your intention.
if file server machine has tons NR_ACTIVE_FILE, this logic kill many process.

Do I misunderstand anything?

Quoted 9 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

static uint32_t lowmem_debug_level = 2;
static int lowmem_adj[6] = {

why do you choice [6]?

We use six levels.

if you don't consider other user and other usage case, this file can't merge forever.

I never expected it to be merged. I wrote it to allow us to ship a product.

Quoted 7 lines · On Wed, 28 Jan 2009 18:51:07 -0800 Arve Hjønnevåg <[email protected]> wrote:

Quoted 7 lines

Quoted 6 lines

Quoted 4 lines

Quoted 2 lines

static uint32_t lowmem_debug_level = 2;
static int lowmem_adj[6] = {

why do you choice [6]?

We use six levels.

Quoted 5 lines

Quoted 2 lines

if you don't consider other user and other usage case, this file can't merge forever.

I never expected it to be merged. I wrote it to allow us to ship a product.

Then, please write "DON'T MERGE ME" on the top of patch description. we can adjust our viewpoints.

Quoted 4 lines

Quoted 2 lines

I never expected it to be merged. I wrote it to allow us to ship a product.

Then, please write "DON'T MERGE ME" on the top of patch description. we can adjust our viewpoints.

The code will live in the drivers/staging/ directory for now and not get merged into the "main" portion of the kernel tree until everyone can agree on it.

But for now, it is useful and seems to work for a few million devices out there, so we can't just ignore it :)

Quoted 9 lines

Quoted 4 lines

Quoted 2 lines

I never expected it to be merged. I wrote it to allow us to ship a product.

Then, please write "DON'T MERGE ME" on the top of patch description. we can adjust our viewpoints.

The code will live in the drivers/staging/ directory for now and not get merged into the "main" portion of the kernel tree until everyone can agree on it.

Quoted 2 lines

But for now, it is useful and seems to work for a few million devices out there, so we can't just ignore it :)

No.
if author don't hope review and merge, we don't have any reason to reviewing.

Quoted 12 lines

Quoted 11 lines

Quoted 4 lines

Quoted 2 lines

I never expected it to be merged. I wrote it to allow us to ship a product.

Then, please write "DON'T MERGE ME" on the top of patch description. we can adjust our viewpoints.

The code will live in the drivers/staging/ directory for now and not get merged into the "main" portion of the kernel tree until everyone can agree on it.

But for now, it is useful and seems to work for a few million devices out there, so we can't just ignore it :)

Quoted 2 lines

No.
if author don't hope review and merge, we don't have any reason to reviewing.

I don't think you understand the goal/model for the drivers/staging/ subdirectories. This is where "drivers" and other stand-alone chunks of code live while they are not yet up to the real mergable status for the rest of the kernel tree.

While there, they get cleaned up, fixed up,
and then hopefully, merged into the main portion of the kernel tree when the proper subsystem maintainers say it is ok.

Whenever code in these directories is loaded, it taints the kernel with a TAINT_CRAP flag so that everyone involved knows to ignore any bug reports.

So while a review would be wonderful to have, it's not being asked for for this specific low-memory "driver". I'd like to see your final version of what you proposed a while ago, if that goes into the kernel tree, then this chunk of code will merely be deleted entirely.

Hope this helps explain things better,

Quoted 12 lines

Quoted 12 lines

Quoted 11 lines

Quoted 4 lines

Quoted 2 lines

I never expected it to be merged. I wrote it to allow us to ship a product.

Then, please write "DON'T MERGE ME" on the top of patch description. we can adjust our viewpoints.

The code will live in the drivers/staging/ directory for now and not get merged into the "main" portion of the kernel tree until everyone can agree on it.

But for now, it is useful and seems to work for a few million devices out there, so we can't just ignore it :)

Quoted 7 lines

Quoted 2 lines

No.
if author don't hope review and merge, we don't have any reason to reviewing.

I don't think you understand the goal/model for the drivers/staging/ subdirectories. This is where "drivers" and other stand-alone chunks of code live while they are not yet up to the real mergable status for the rest of the kernel tree.

I think staging is great activity, but I also think it is no good idea for kernel core piece.

Quoted 3 lines

While there, they get cleaned up, fixed up,
and then hopefully, merged into the main portion of the kernel tree when the proper subsystem maintainers say it is ok.

The fact is simple more. if auther refuse to receive reviewing, the code don't clean up at all, don't fix up at all. then, dropping is better.

Quoted 9 lines

Whenever code in these directories is loaded, it taints the kernel with a TAINT_CRAP flag so that everyone involved knows to ignore any bug reports.

So while a review would be wonderful to have, it's not being asked for for this specific low-memory "driver". I'd like to see your final version of what you proposed a while ago, if that goes into the kernel tree, then this chunk of code will merely be deleted entirely.

Quoted 1 line

Hope this helps explain things better,

Again, I respect for your drivers/staging activity largely. then, I don't oppose any driver merge to staging.

but I don't think driver/staging is good place for non driver code. The problem is, any patch must be reviewed by stakeholder, not maintenar only. then, the patch should post lkml and subsystem mailing list at first.

I like reviewed code than unreviewed code.

Quoted 5 lines

but I don't think driver/staging is good place for non driver code. The problem is, any patch must be reviewed by stakeholder, not maintenar only. then, the patch should post lkml and subsystem mailing list at first.

I like reviewed code than unreviewed code.

Heh, so do I.

And this is an odd "driver", I do know that.

But it solves a real problem that can't be solved any other way currently, which is needed for a real system that is shipping. So, if it can't be solved any other way, do you have a way this kind of thing could be more "correct"?

[Greg KH <[email protected]>]

Quoted 14 lines

Quoted 5 lines

but I don't think driver/staging is good place for non driver code. The problem is, any patch must be reviewed by stakeholder, not maintenar only. then, the patch should post lkml and subsystem mailing list at first.

I like reviewed code than unreviewed code.

Heh, so do I.

And this is an odd "driver", I do know that.

But it solves a real problem that can't be solved any other way currently, which is needed for a real system that is shipping. So, if it can't be solved any other way, do you have a way this kind of thing could be more "correct"?

I think a lot of the confusion here comes from Arve's earlier (very terse) remark: "I never expected it to be merged. I wrote it to allow us to ship a product."

At the risk of putting words in his mouth, I believe this should be parsed as "we wrote this to solve problems necessary to ship products and did not expect it to be merged to mainline as-is".

We'd love to get support for low memory process killing that works for our app model into the mainline.

If that's by reworking this driver
until it's acceptable or by implementing the same functionality a different way, making use of some other subsystem, or whatever, we're not particularly picky. Our goal is, eventually, to maintain as few patches outside of the kernel as possible so things can build "out of the box."

I never expected it to be merged. I wrote it to allow us to ship a product.

Quoted 4 lines

The top problem is, this file stay on non proper place. then, MM folks don't review at all.

I think this patch need to receive MM folks review.

This patch solves two problems for us:
1. It gives us more control over which process gets killed when we run out of memory. This can easily be fixed in the regular oom killer instead.
2. It prevents the system from getting unusable due to excessive demand paging. Before we added the low memory killer, we had devices stuck for many minutes before the system managed to allocate enough memory for the oom killer to kick in.
The second problem is the most critical and if it is solved, we will not need this patch.

Hi!

Quoted 7 lines

I never expected it to be merged. I wrote it to allow us to ship a product.

Quoted 4 lines

The top problem is, this file stay on non proper place. then, MM folks don't review at all.

I think this patch need to receive MM folks review.

Quoted 10 lines

This patch solves two problems for us:
1. It gives us more control over which process gets killed when we run out of memory. This can easily be fixed in the regular oom killer instead.
2. It prevents the system from getting unusable due to excessive demand paging. Before we added the low memory killer, we had devices stuck for many minutes before the system managed to allocate enough memory for the oom killer to kick in.
The second problem is the most critical and if it is solved, we will not need this patch.

Ok, maybe the best way forward is to create a "demo" that can be run on desktop machine,

where machine misbehaves (like becomes useless for 10 minutes before OOM killer kicks in), then draw attetion of Andrew/Nick/etc?

I was thinking that, and it would integrate better with the OLPC work (which IMHO is a nicer interface for some stuff)

You'd want two thresholds

The 'arghhhh....' point where you start killing stuff
The 'uh oh...' point where an OLPC style low memory notifier kicks in

(OLPC's model is a handle you can select/poll for 'memory getting low' so apps can respond to pressure by doing stuff like dumping caches)

The rest ought to follow naturally IFF you can find a clean efficient way to measure that pressure and quantify it as a number. Our default would be like now, the Android default might be to trigger earlier..