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]>]
> 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
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.
--
Arve Hj?nnev?g
On Tue, Jan 13, 2009 at 6:18 PM, Brian Swetland <[email protected]> wrote:
>
> Looping in Arve who wrote the low memory killer and can explain things
> in more detail.
>
> Brian
>
> [Greg KH <[email protected]>]
>> 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
> --
> 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/
>
On Tue, Jan 13, 2009 at 06:30:39PM -0800, Arve Hj?nnev?g wrote:
> 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?
thanks,
greg k-h
Hi, Arve.
On Wed, Jan 14, 2009 at 11:30 AM, Arve Hjønnevåg <[email protected]> wrote:> 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.
It have some problems. (drivers/staging/android/lwmemorykiller.c)
1. lowmem_shrink function have to answer about vm's query the cache size fast.2. it don't consider page size and memory size when it makelowmem_minfree's values.3. If system have many processes, for_each_process take a long time.it may result system latency although lowmemkiller intend to avoidlatency.4. Most important thing. Could we use memory controller instead oflowmemkiller ? I am not sure since I don't follow up memory controllerin these days.
I think we have to use existing facility if possible.Previously, There are similar kinds of patches. but It can't mergemainline due to some issue. They can comment about lowmemkiller. Iwill CC them.
> --> Arve Hjønnevåg>>> On Tue, Jan 13, 2009 at 6:18 PM, Brian Swetland <[email protected]> wrote:>>>> Looping in Arve who wrote the low memory killer and can explain things>> in more detail.>>>> Brian>>>> [Greg KH <[email protected]>]>>> 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>> -->> 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/>>> --> 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/>
-- Kinds regards,MinChan Kim????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On Tue 2009-01-13 19:52:37, Greg KH wrote:
> On Tue, Jan 13, 2009 at 06:30:39PM -0800, Arve Hj?nnev?g wrote:
> > 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?
Maybe our oom killer should get a new tunable, telling it how
aggressive it should be, instead?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
> > 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..
Last time, it seems my mail is lost.
My mail is mangled by web mail(gmail) which change my mail's base encoding type.
On Wed, Jan 14, 2009 at 7:48 PM, Alan Cox <[email protected]> wrote:
>> > 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)
I guess OLPC model is similar to mem_notify.
Last year, Kosaki-san made mem_notify patch series.
It may help this situation.
http://lwn.net/Articles/268732/
> 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/
>
--
Kinds regards,
MinChan Kim
On Wed, Jan 14, 2009 at 2:48 AM, Alan Cox <[email protected]> wrote:
>> 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.
--
Arve Hj?nnev?g
On Wed, Jan 14, 2009 at 02:26:48PM -0800, Arve Hj?nnev?g wrote:
> On Wed, Jan 14, 2009 at 2:48 AM, Alan Cox <[email protected]> wrote:
> >> 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.
But the lowmemorykiller android module doesn't have anything to do with
this, right?
thanks,
greg k-h
On Wed, Jan 14, 2009 at 3:17 PM, Greg KH <[email protected]> wrote:
>> 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?
>
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.
--
Arve Hj?nnev?g
On Wed, Jan 14, 2009 at 03:32:38PM -0800, Arve Hj?nnev?g wrote:
> On Wed, Jan 14, 2009 at 3:17 PM, Greg KH <[email protected]> wrote:
> >> 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?
> >
>
> 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?
thanks,
greg k-h
Signed-off-by: Arve Hjønnevåg <[email protected]>
---
drivers/staging/android/lowmemorykiller.txt | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
create mode 100644 drivers/staging/android/lowmemorykiller.txt
diff --git a/drivers/staging/android/lowmemorykiller.txt b/drivers/staging/android/lowmemorykiller.txt
new file mode 100644
index 0000000..bd5c0c0
--- /dev/null
+++ b/drivers/staging/android/lowmemorykiller.txt
@@ -0,0 +1,16 @@
+The lowmemorykiller driver lets user-space specify a set of memory thresholds
+where processes with a range of oom_adj values will get killed. Specify the
+minimum oom_adj values in /sys/module/lowmemorykiller/parameters/adj and the
+number of free pages in /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_adj value of 8 or higher when the free memory drops below 4096 pages
+and kill processes with a oom_adj value of 0 or higher when the free memory
+drops below 1024 pages.
+
+The driver considers memory used for caches to be free, but if a large
+percentage of the cached memory is locked this can be very inaccurate
+and processes may not get killed until the normal oom killer is triggered.
+
--
1.6.1
On Wed, Jan 14, 2009 at 04:54:16PM -0800, Arve Hj?nnev?g wrote:
> Signed-off-by: Arve Hj?nnev?g <[email protected]>
Thanks for doing this, I'll queue it up.
greg k-h
On Wed, 14 Jan 2009, Alan Cox wrote:
> 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..
The /dev/mem_notify patch allowed polling on a system-wide scale for low
memory conditions so that userspace could respond appropriately: either by
droping caches, as you mentioned, or sending a signal. That signal could
quite possibily be SIGKILL for no better reason than preempting what the
kernel oom killer would do.
I think this should be completely seperate from the oom killer, which has
always been a "last resort" to situations where the kernel is completely
out of memory for a task.
If /dev/mem_notify existed in a cgroup form so that different handlers
could be responsible for an aggregate of tasks, I think this addresses
your concerns. That might require some cleverness in the cgroup
filesystem code if this would introduce device files, but there are
probably future use cases for that, as well.
Hi Greg,
On Thu, Jan 15, 2009 at 5:42 AM, Greg KH <[email protected]> wrote:
> On Wed, Jan 14, 2009 at 03:32:38PM -0800, Arve Hj?nnev?g wrote:
>> On Wed, Jan 14, 2009 at 3:17 PM, Greg KH <[email protected]> wrote:
>> >> 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?
>> >
>>
>> 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?
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
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
On Thu, Jan 15, 2009 at 07:02:48PM +0530, Trilok Soni wrote:
> Hi Greg,
>
> On Thu, Jan 15, 2009 at 5:42 AM, Greg KH <[email protected]> wrote:
> > On Wed, Jan 14, 2009 at 03:32:38PM -0800, Arve Hj?nnev?g wrote:
> >> On Wed, Jan 14, 2009 at 3:17 PM, Greg KH <[email protected]> wrote:
> >> >> 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?
> >> >
> >>
> >> 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?
>
> 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 :)
thanks,
greg k-h
On Thu, Jan 15, 2009 at 03:44:04PM -0800, Greg KH wrote:
> On Thu, Jan 15, 2009 at 07:02:48PM +0530, Trilok Soni wrote:
> > 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 :)
>
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.
Hi
>> > 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 :)
>>
> 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.
This thread is very interesting.
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.
> 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
quick review.
> 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 };
.vm_enough_memory is the hook of virtual memory accounting, not rss.
modern almost linux application and glibc largely use virtual address space
than actual resident memory.
then, vm_enough_memory don't provide proper hook.
> 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 }, {
you can set .ctl_name to CTL_UNNUMBERED.
> 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);
this line assume file page can evictable. but it is not true.
we should consider mlocked file page.
> 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;
No generic assumption.
> 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;
Oops, LOWMEM_MAX_UIDS is ugly restriction.
> 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;
It seems racy.
if this code run on smp, lowmem_free_pages can hold unproper value.
> 322 return 0;
> 323 }
then, In my first look, this code deeply depend on embedded and nokia
process management policy.
I don't think it can merge mainline.
also quick review to lowmemorykiller.c
> #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
> };
why do you choice *16?
> static uint32_t lowmem_debug_level = 2;
> static int lowmem_adj[6] = {
why do you choice [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);
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.
> 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) {
Oops. too long locking.
shrink_slab() is freqentlly called function. I don't like costly operation.
> 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;
this code mean,
shrinker->shrink(0, gfp_mask) indicate to recalculate total rss every time.
it is too CPU and battery wasting.
> }
> }
> 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;
> }
On Fri, Jan 16, 2009 at 08:16:51PM +0900, KOSAKI Motohiro wrote:
>
> As far as I know, embedded guys strong want to lowmem notification mecanism.
I think the big server guys also want the same thing :)
> 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.
> LSM and driver notifier is easy breakable because these component
> deeply depend on MM.
Agreed.
> (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.
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?
thanks,
greg k-h
> On Fri, Jan 16, 2009 at 08:16:51PM +0900, KOSAKI Motohiro wrote:
> >
> > As far as I know, embedded guys strong want to lowmem notification mecanism.
>
> I think the big server guys also want the same thing :)
Yeah! I know, because my company is definitry big server vendor :)
> > 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.
>
> > LSM and driver notifier is easy breakable because these component
> > deeply depend on MM.
>
> Agreed.
>
> > (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.
>
> 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.
On Wed, Jan 21, 2009 at 11:50:48AM +0900, KOSAKI Motohiro wrote:
> 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.
>
Building on top of the memory cgroup makes sense given that it has a
lot more knowledge of the actual state in different contexts compared to
something like security_vm_enough_memory()/__vm_enough_memory().
Despite that, a notification layer on top of cgroups in general might be
worth thinking about, particularly since each controller may want to use
notifications for different things. It is conceivable that people will
also want different types of notifications from the memory controller
beyond a simple lowmem notifier.
The LSM lowmem module itself used multiple notification levels to tune
application behaviour for instance, though obviously this is something
that is highly workload dependent.
On Wed, 21 Jan 2009 11:50:48 +0900 (JST)
KOSAKI Motohiro <[email protected]> wrote:
> 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%.
^^^^^^^^^^^^
just depends on workload...
>
> Then, I think any memory accounting activity should use this infrastructure.
> That's my homework.
>
But, memcg requires much memory to track usage of pages, a page_cgroup structure,
40bytes per page on 64bit, 20bytes per page on 32bit.
Probably, for embeded people, this additinal memory usage is not acceptable.
Below is just a noise:
==
As my personal project, I'd like to write on-demand-fake-numa for generic archs.
Now, section size is 128MB on x86-64. I'd like to allow following ops.
# create_new_node (echo node_number > /sys/device/system/node/add_fake_node)
# offline memory (echo offline > /sys/device/system/memory/memoryXXX/state)
# attach offlined memory to new node
(echo node_number > /sys/device/system/memory/memoryXXX/attach)
# online memmory (echo online > /sys/device/system/memory/memoryXXX/state)
Then, memoryXXX (128MB of chunk of memory) is controlled under a new fake node
"node_number".
For embeded systems, you may be able to modify SECTION_SIZE smaller (8-16MB?) and
cpuset will give them enough resource isolation.
(But not so flexible as memory cgroup.)
you can use node+cpuset information to track memory usage by apps.
Note: OOM-Killer kills process within cpuset.
Thanks,
-Kame
On Fri, Jan 16, 2009 at 5:18 AM, KOSAKI Motohiro
<[email protected]> wrote:
> also quick review to lowmemorykiller.c
>> static struct shrinker lowmem_shrinker = {
>> .shrink = lowmem_shrink,
>> .seeks = DEFAULT_SEEKS * 16
>> };
>
> why do you choice *16?
>
To indicate that it is more expensive to restart a killed process than
a regular cache page.
>
>> static uint32_t lowmem_debug_level = 2;
>> static int lowmem_adj[6] = {
>
> why do you choice [6]?
We use six levels.
>> 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.
>
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.
>> read_lock(&tasklist_lock);
>> for_each_process(p) {
>
> Oops. too long locking.
> shrink_slab() is freqentlly called function. I don't like costly operation.
>
>> 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;
>
> 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:
----
diff --git a/drivers/misc/lowmemorykiller.c b/drivers/misc/lowmemorykiller.c
index 3715d56..3f4e41a 100644
--- a/drivers/misc/lowmemorykiller.c
+++ b/drivers/misc/lowmemorykiller.c
@@ -71,6 +71,12 @@ static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
}
if(nr_to_scan > 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;
+ }
+
read_lock(&tasklist_lock);
for_each_process(p) {
if(p->oomkilladj >= 0 && p->mm) {
@@ -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) {
----
--
Arve Hj?nnev?g
Hi
> On Fri, Jan 16, 2009 at 5:18 AM, KOSAKI Motohiro
> <[email protected]> wrote:
> > also quick review to lowmemorykiller.c
>
> >> static struct shrinker lowmem_shrinker = {
> >> .shrink = lowmem_shrink,
> >> .seeks = DEFAULT_SEEKS * 16
> >> };
> >
> > why do you choice *16?
>
> 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.
> >> 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.
Greg, please drop this file from stagin awhile.
(of cource, my intention is this one file only, other hardware depended
driver file is needed to staging)
Naked-by: KOSAKI Motohiro <[email protected]>
> >> 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.
>
> 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.
hm hom.
> 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.
> >> read_lock(&tasklist_lock);
> >> for_each_process(p) {
> >
> > Oops. too long locking.
> > shrink_slab() is freqentlly called function. I don't like costly operation.
> >
> >> 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;
> >
> > 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:
> ----
> diff --git a/drivers/misc/lowmemorykiller.c b/drivers/misc/lowmemorykiller.c
> index 3715d56..3f4e41a 100644
> --- a/drivers/misc/lowmemorykiller.c
> +++ b/drivers/misc/lowmemorykiller.c
> @@ -71,6 +71,12 @@ static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
> }
> if(nr_to_scan > 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?
> read_lock(&tasklist_lock);
> for_each_process(p) {
> if(p->oomkilladj >= 0 && p->mm) {
> @@ -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) {
On Wed, Jan 28, 2009 at 5:48 PM, KOSAKI Motohiro
<[email protected]> wrote:
>> 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.
DEFAULT_SEEKS * 16 is probably not best. 10 or 20 probably works just
as well. I don't remember what values I tried, but at some point
lowmem_shrink did not get called often enough to be effective.
>> >> 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.
> 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.
>> 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.
The first level page table on arm needs four contiguous pages. This is
enough to trigger this problem. I suspect the lack of swap makes this
problem much more apparent.
>> How about this:
>> ----
>> diff --git a/drivers/misc/lowmemorykiller.c b/drivers/misc/lowmemorykiller.c
>> index 3715d56..3f4e41a 100644
>> --- a/drivers/misc/lowmemorykiller.c
>> +++ b/drivers/misc/lowmemorykiller.c
>> @@ -71,6 +71,12 @@ static int lowmem_shrink(int nr_to_scan, gfp_t gfp_mask)
>> }
>> if(nr_to_scan > 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?
It only kills one process at a time, and only if the amount of memory
available is below the selected threshold. The number returned is a
very rough estimate of how much memory can be freed. The old code used
the sum of rss for the killable processes which is not accurate
either.
--
Arve Hj?nnev?g
On Wed, 28 Jan 2009 18:51:07 -0800
Arve Hjønnevåg <[email protected]> wrote:
> >> >> 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.
>
Then, please write "DON'T MERGE ME" on the top of patch description.
we can adjust our viewpoints.
Thx,
-Kame
On Thu, Jan 29, 2009 at 12:45:55PM +0900, KAMEZAWA Hiroyuki wrote:
> On Wed, 28 Jan 2009 18:51:07 -0800
> Arve Hj?nnev?g <[email protected]> wrote:
>
> > >> >> 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.
> >
> 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 :)
thanks,
greg k-h
> > > 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 :)
No.
if author don't hope review and merge, we don't have any reason to reviewing.
On Thu, Jan 29, 2009 at 01:43:51PM +0900, KOSAKI Motohiro wrote:
> > > > 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 :)
>
> 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,
greg k-h
> > > > > 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 :)
> >
> > 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.
> 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.
> 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,
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.
Hi!
> I never expected it to be merged. I wrote it to allow us to ship a product.
>
> > 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.
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?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Thu, Jan 29, 2009 at 02:29:05PM +0900, KOSAKI Motohiro wrote:
> > > > > > 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 :)
> > >
> > > 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.
>
> > 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.
But that's not true at all. And I'll be glad to fix up anything, I just
need to make sure that the system still will work properly when doing
so.
> > 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,
>
> Again, I respect for your drivers/staging activity largely.
> then, I don't oppose any driver merge to staging.
thanks.
> 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"?
thanks,
greg k-h
[Greg KH <[email protected]>]
> > 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."
Brian
Hi
On Wed, Jan 21, 2009 at 8:20 AM, KOSAKI Motohiro
<[email protected]> wrote:
>>
>> 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?
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
On Thu, 2 Apr 2009, Trilok Soni wrote:
> 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?
>
The /dev/mem_notify patches seem to have been abandoned, but there's still
an interest in a per-cgroup oom notifier. This shouldn't be restricted
only to the memory controller, however, since cpusets could use the
feature as well. Thus, it should probably be its own cgroup.
Implementing the same concept as /dev/mem_notify on top of the cgroup
interface would probably require some hacking to support such files, but
that support may not be far off anyway (devices that are only accessible
by an exclusive group of tasks).