public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* lowmemory android driver not needed?
@ 2009-01-14  1:02 Greg KH
  2009-01-14  2:18 ` Brian Swetland
  0 siblings, 1 reply; 41+ messages in thread
From: Greg KH @ 2009-01-14  1:02 UTC (permalink / raw)
  To: San Mehat; +Cc: Alan Cox, Brian Swetland, Robert Love, linux-kernel

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

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

* Re: lowmemory android driver not needed?
  2009-01-14  1:02 lowmemory android driver not needed? Greg KH
@ 2009-01-14  2:18 ` Brian Swetland
  2009-01-14  2:30   ` Arve Hjønnevåg
  0 siblings, 1 reply; 41+ messages in thread
From: Brian Swetland @ 2009-01-14  2:18 UTC (permalink / raw)
  To: Greg KH, arve; +Cc: San Mehat, Alan Cox, Robert Love, linux-kernel


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

Brian

[Greg KH <greg@kroah.com>]
> 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

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

* Re: lowmemory android driver not needed?
  2009-01-14  2:18 ` Brian Swetland
@ 2009-01-14  2:30   ` Arve Hjønnevåg
  2009-01-14  3:52     ` Greg KH
  2009-01-14  6:45     ` MinChan Kim
  0 siblings, 2 replies; 41+ messages in thread
From: Arve Hjønnevåg @ 2009-01-14  2:30 UTC (permalink / raw)
  To: Brian Swetland
  Cc: Greg KH, arve, San Mehat, Alan Cox, Robert Love, linux-kernel

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 <swetland@google.com> wrote:
>
> Looping in Arve who wrote the low memory killer and can explain things
> in more detail.
>
> Brian
>
> [Greg KH <greg@kroah.com>]
>> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

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

* Re: lowmemory android driver not needed?
  2009-01-14  2:30   ` Arve Hjønnevåg
@ 2009-01-14  3:52     ` Greg KH
  2009-01-14 10:43       ` Pavel Machek
  2009-01-14  6:45     ` MinChan Kim
  1 sibling, 1 reply; 41+ messages in thread
From: Greg KH @ 2009-01-14  3:52 UTC (permalink / raw)
  To: Arve Hjønnevåg
  Cc: Brian Swetland, arve, San Mehat, Alan Cox, Robert Love,
	linux-kernel

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

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

* Re: lowmemory android driver not needed?
  2009-01-14  2:30   ` Arve Hjønnevåg
  2009-01-14  3:52     ` Greg KH
@ 2009-01-14  6:45     ` MinChan Kim
  1 sibling, 0 replies; 41+ messages in thread
From: MinChan Kim @ 2009-01-14  6:45 UTC (permalink / raw)
  To: Arve Hjønnevåg, Balbir Singh, KOSAKI Motohiro,
	Andrew Morton, Rik van Riel, linux-mm
  Cc: Brian Swetland, Greg KH, arve, San Mehat, Alan Cox, Robert Love,
	linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 2513 bytes --]

Hi, Arve.
On Wed, Jan 14, 2009 at 11:30 AM, Arve Hjønnevåg <arve@android.com> 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 <swetland@google.com> wrote:>>>> Looping in Arve who wrote the low memory killer and can explain things>> in more detail.>>>> Brian>>>> [Greg KH <greg@kroah.com>]>>> 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 majordomo@vger.kernel.org>> 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 majordomo@vger.kernel.org> 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Ç+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: lowmemory android driver not needed?
  2009-01-14  3:52     ` Greg KH
@ 2009-01-14 10:43       ` Pavel Machek
  2009-01-14 10:48         ` Alan Cox
  0 siblings, 1 reply; 41+ messages in thread
From: Pavel Machek @ 2009-01-14 10:43 UTC (permalink / raw)
  To: Greg KH
  Cc: Arve Hj?nnev?g, Brian Swetland, arve, San Mehat, Alan Cox,
	Robert Love, linux-kernel

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

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

* Re: lowmemory android driver not needed?
  2009-01-14 10:43       ` Pavel Machek
@ 2009-01-14 10:48         ` Alan Cox
  2009-01-14 12:18           ` MinChan Kim
                             ` (2 more replies)
  0 siblings, 3 replies; 41+ messages in thread
From: Alan Cox @ 2009-01-14 10:48 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Greg KH, Arve Hj?nnev?g, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel

> > 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..

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

* Re: lowmemory android driver not needed?
  2009-01-14 10:48         ` Alan Cox
@ 2009-01-14 12:18           ` MinChan Kim
  2009-01-14 22:26           ` Arve Hjønnevåg
  2009-01-15  7:55           ` David Rientjes
  2 siblings, 0 replies; 41+ messages in thread
From: MinChan Kim @ 2009-01-14 12:18 UTC (permalink / raw)
  To: Alan Cox
  Cc: Pavel Machek, Greg KH, Arve Hj?nnev?g, Brian Swetland, arve,
	San Mehat, Robert Love, linux-kernel, KOSAKI Motohiro,
	Rik van Riel

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 <alan@lxorguk.ukuu.org.uk> 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 majordomo@vger.kernel.org
> 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

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

* Re: lowmemory android driver not needed?
  2009-01-14 10:48         ` Alan Cox
  2009-01-14 12:18           ` MinChan Kim
@ 2009-01-14 22:26           ` Arve Hjønnevåg
  2009-01-14 23:17             ` Greg KH
  2009-01-15  7:55           ` David Rientjes
  2 siblings, 1 reply; 41+ messages in thread
From: Arve Hjønnevåg @ 2009-01-14 22:26 UTC (permalink / raw)
  To: Alan Cox
  Cc: Pavel Machek, Greg KH, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel

On Wed, Jan 14, 2009 at 2:48 AM, Alan Cox <alan@lxorguk.ukuu.org.uk> 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

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

* Re: lowmemory android driver not needed?
  2009-01-14 22:26           ` Arve Hjønnevåg
@ 2009-01-14 23:17             ` Greg KH
  2009-01-14 23:32               ` Arve Hjønnevåg
  0 siblings, 1 reply; 41+ messages in thread
From: Greg KH @ 2009-01-14 23:17 UTC (permalink / raw)
  To: Arve Hjønnevåg
  Cc: Alan Cox, Pavel Machek, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel

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 <alan@lxorguk.ukuu.org.uk> 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

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

* Re: lowmemory android driver not needed?
  2009-01-14 23:17             ` Greg KH
@ 2009-01-14 23:32               ` Arve Hjønnevåg
  2009-01-15  0:12                 ` Greg KH
  0 siblings, 1 reply; 41+ messages in thread
From: Arve Hjønnevåg @ 2009-01-14 23:32 UTC (permalink / raw)
  To: Greg KH
  Cc: Alan Cox, Pavel Machek, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel

On Wed, Jan 14, 2009 at 3:17 PM, Greg KH <greg@kroah.com> 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

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

* Re: lowmemory android driver not needed?
  2009-01-14 23:32               ` Arve Hjønnevåg
@ 2009-01-15  0:12                 ` Greg KH
  2009-01-15  0:54                   ` [PATCH] Staging: android: Add lowmemorykiller documentation Arve Hjønnevåg
  2009-01-15 13:32                   ` lowmemory android driver not needed? Trilok Soni
  0 siblings, 2 replies; 41+ messages in thread
From: Greg KH @ 2009-01-15  0:12 UTC (permalink / raw)
  To: Arve Hjønnevåg
  Cc: Alan Cox, Pavel Machek, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel

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 <greg@kroah.com> 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

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

* [PATCH] Staging: android: Add lowmemorykiller documentation.
  2009-01-15  0:12                 ` Greg KH
@ 2009-01-15  0:54                   ` Arve Hjønnevåg
  2009-01-15  0:59                     ` Greg KH
  2009-01-15 13:32                   ` lowmemory android driver not needed? Trilok Soni
  1 sibling, 1 reply; 41+ messages in thread
From: Arve Hjønnevåg @ 2009-01-15  0:54 UTC (permalink / raw)
  To: greg
  Cc: Alan Cox, Pavel Machek, Brian Swetland, San Mehat, Robert Love,
	linux-kernel, Arve Hjønnevåg

Signed-off-by: Arve Hjønnevåg <arve@android.com>
---
 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


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

* Re: [PATCH] Staging: android: Add lowmemorykiller documentation.
  2009-01-15  0:54                   ` [PATCH] Staging: android: Add lowmemorykiller documentation Arve Hjønnevåg
@ 2009-01-15  0:59                     ` Greg KH
  0 siblings, 0 replies; 41+ messages in thread
From: Greg KH @ 2009-01-15  0:59 UTC (permalink / raw)
  To: Arve Hjønnevåg
  Cc: Alan Cox, Pavel Machek, Brian Swetland, San Mehat, Robert Love,
	linux-kernel

On Wed, Jan 14, 2009 at 04:54:16PM -0800, Arve Hjønnevåg wrote:
> Signed-off-by: Arve Hjønnevåg <arve@android.com>

Thanks for doing this, I'll queue it up.

greg k-h

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

* Re: lowmemory android driver not needed?
  2009-01-14 10:48         ` Alan Cox
  2009-01-14 12:18           ` MinChan Kim
  2009-01-14 22:26           ` Arve Hjønnevåg
@ 2009-01-15  7:55           ` David Rientjes
  2 siblings, 0 replies; 41+ messages in thread
From: David Rientjes @ 2009-01-15  7:55 UTC (permalink / raw)
  To: Alan Cox
  Cc: Pavel Machek, Greg KH, Arve Hj?nnev?g, Brian Swetland, arve,
	San Mehat, Robert Love, linux-kernel

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.

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

* Re: lowmemory android driver not needed?
  2009-01-15  0:12                 ` Greg KH
  2009-01-15  0:54                   ` [PATCH] Staging: android: Add lowmemorykiller documentation Arve Hjønnevåg
@ 2009-01-15 13:32                   ` Trilok Soni
  2009-01-15 23:44                     ` Greg KH
  2009-01-16 11:42                     ` KOSAKI Motohiro
  1 sibling, 2 replies; 41+ messages in thread
From: Trilok Soni @ 2009-01-15 13:32 UTC (permalink / raw)
  To: Greg KH
  Cc: Arve Hjønnevåg, Alan Cox, Pavel Machek, Brian Swetland,
	arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrjölä, viktor.rosendahl

Hi Greg,

On Thu, Jan 15, 2009 at 5:42 AM, Greg KH <greg@kroah.com> 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 <greg@kroah.com> 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

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

* Re: lowmemory android driver not needed?
  2009-01-15 13:32                   ` lowmemory android driver not needed? Trilok Soni
@ 2009-01-15 23:44                     ` Greg KH
  2009-01-16  9:02                       ` Paul Mundt
  2009-01-16 11:42                     ` KOSAKI Motohiro
  1 sibling, 1 reply; 41+ messages in thread
From: Greg KH @ 2009-01-15 23:44 UTC (permalink / raw)
  To: Trilok Soni
  Cc: Arve Hjønnevåg, Alan Cox, Pavel Machek, Brian Swetland,
	arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrjölä, viktor.rosendahl

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 <greg@kroah.com> 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 <greg@kroah.com> 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

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

* Re: lowmemory android driver not needed?
  2009-01-15 23:44                     ` Greg KH
@ 2009-01-16  9:02                       ` Paul Mundt
  2009-01-16 11:16                         ` KOSAKI Motohiro
  2009-02-03 20:55                         ` Tony Lindgren
  0 siblings, 2 replies; 41+ messages in thread
From: Paul Mundt @ 2009-01-16  9:02 UTC (permalink / raw)
  To: Greg KH
  Cc: Trilok Soni, Arve Hj?nnev?g, Alan Cox, Pavel Machek,
	Brian Swetland, arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren, ext Juha Yrj?l?,
	viktor.rosendahl

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.

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

* Re: lowmemory android driver not needed?
  2009-01-16  9:02                       ` Paul Mundt
@ 2009-01-16 11:16                         ` KOSAKI Motohiro
  2009-01-16 15:23                           ` Greg KH
  2009-02-03 20:55                         ` Tony Lindgren
  1 sibling, 1 reply; 41+ messages in thread
From: KOSAKI Motohiro @ 2009-01-16 11:16 UTC (permalink / raw)
  To: Paul Mundt, Greg KH, Trilok Soni, Arve Hj?nnev?g, Alan Cox,
	Pavel Machek, Brian Swetland, arve, San Mehat, Robert Love,
	linux-kernel, linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrj?l?, viktor.rosendahl

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.

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

* Re: lowmemory android driver not needed?
  2009-01-15 13:32                   ` lowmemory android driver not needed? Trilok Soni
  2009-01-15 23:44                     ` Greg KH
@ 2009-01-16 11:42                     ` KOSAKI Motohiro
  2009-01-16 13:18                       ` KOSAKI Motohiro
  1 sibling, 1 reply; 41+ messages in thread
From: KOSAKI Motohiro @ 2009-01-16 11:42 UTC (permalink / raw)
  To: Trilok Soni
  Cc: Greg KH, Arve Hjønnevåg, Alan Cox, Pavel Machek,
	Brian Swetland, arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrjölä, viktor.rosendahl

> 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.

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

* Re: lowmemory android driver not needed?
  2009-01-16 11:42                     ` KOSAKI Motohiro
@ 2009-01-16 13:18                       ` KOSAKI Motohiro
  2009-01-22  6:13                         ` Arve Hjønnevåg
  0 siblings, 1 reply; 41+ messages in thread
From: KOSAKI Motohiro @ 2009-01-16 13:18 UTC (permalink / raw)
  To: Arve Hjønnevåg
  Cc: Greg KH, Alan Cox, Pavel Machek, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel, linux-omap@vger.kernel.org,
	Tony Lindgren, ext Juha Yrjölä, viktor.rosendahl,
	Trilok Soni

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;
> }

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

* Re: lowmemory android driver not needed?
  2009-01-16 11:16                         ` KOSAKI Motohiro
@ 2009-01-16 15:23                           ` Greg KH
  2009-01-21  2:50                             ` KOSAKI Motohiro
  0 siblings, 1 reply; 41+ messages in thread
From: Greg KH @ 2009-01-16 15:23 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Paul Mundt, Trilok Soni, Arve Hj?nnev?g, Alan Cox, Pavel Machek,
	Brian Swetland, arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren, ext Juha Yrj?l?,
	viktor.rosendahl

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

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

* Re: lowmemory android driver not needed?
  2009-01-16 15:23                           ` Greg KH
@ 2009-01-21  2:50                             ` KOSAKI Motohiro
  2009-01-21  3:05                               ` Paul Mundt
                                                 ` (2 more replies)
  0 siblings, 3 replies; 41+ messages in thread
From: KOSAKI Motohiro @ 2009-01-21  2:50 UTC (permalink / raw)
  To: Greg KH
  Cc: kosaki.motohiro, Paul Mundt, Trilok Soni, Arve Hj?nnev?g,
	Alan Cox, Pavel Machek, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel, linux-omap@vger.kernel.org,
	Tony Lindgren, ext Juha Yrj?l?, viktor.rosendahl

> 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.




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

* Re: lowmemory android driver not needed?
  2009-01-21  2:50                             ` KOSAKI Motohiro
@ 2009-01-21  3:05                               ` Paul Mundt
  2009-01-21  3:29                               ` KAMEZAWA Hiroyuki
  2009-04-01 18:38                               ` Trilok Soni
  2 siblings, 0 replies; 41+ messages in thread
From: Paul Mundt @ 2009-01-21  3:05 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Greg KH, Trilok Soni, Arve Hj?nnev?g, Alan Cox, Pavel Machek,
	Brian Swetland, arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren, ext Juha Yrj?l?,
	viktor.rosendahl

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.

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

* Re: lowmemory android driver not needed?
  2009-01-21  2:50                             ` KOSAKI Motohiro
  2009-01-21  3:05                               ` Paul Mundt
@ 2009-01-21  3:29                               ` KAMEZAWA Hiroyuki
  2009-04-01 18:38                               ` Trilok Soni
  2 siblings, 0 replies; 41+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-01-21  3:29 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Greg KH, Paul Mundt, Trilok Soni, Arve Hj?nnev?g, Alan Cox,
	Pavel Machek, Brian Swetland, arve, San Mehat, Robert Love,
	linux-kernel, linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrj?l?, viktor.rosendahl

On Wed, 21 Jan 2009 11:50:48 +0900 (JST)
KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> 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







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

* Re: lowmemory android driver not needed?
  2009-01-16 13:18                       ` KOSAKI Motohiro
@ 2009-01-22  6:13                         ` Arve Hjønnevåg
  2009-01-29  1:48                           ` KOSAKI Motohiro
  0 siblings, 1 reply; 41+ messages in thread
From: Arve Hjønnevåg @ 2009-01-22  6:13 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Greg KH, Alan Cox, Pavel Machek, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel, linux-omap@vger.kernel.org,
	Tony Lindgren, ext Juha Yrjölä, viktor.rosendahl,
	Trilok Soni

On Fri, Jan 16, 2009 at 5:18 AM, KOSAKI Motohiro
<kosaki.motohiro@jp.fujitsu.com> 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

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

* Re: lowmemory android driver not needed?
  2009-01-22  6:13                         ` Arve Hjønnevåg
@ 2009-01-29  1:48                           ` KOSAKI Motohiro
  2009-01-29  2:51                             ` Arve Hjønnevåg
  0 siblings, 1 reply; 41+ messages in thread
From: KOSAKI Motohiro @ 2009-01-29  1:48 UTC (permalink / raw)
  To: Arve Hj�nnev虍
  Cc: kosaki.motohiro, Greg KH, Alan Cox, Pavel Machek, Brian Swetland,
	arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrj�l・, viktor.rosendahl, Trilok Soni

Hi

> On Fri, Jan 16, 2009 at 5:18 AM, KOSAKI Motohiro
> <kosaki.motohiro@jp.fujitsu.com> 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 <kosaki.motohiro@jp.fujitsu.com>


> >> 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) {






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

* Re: lowmemory android driver not needed?
  2009-01-29  1:48                           ` KOSAKI Motohiro
@ 2009-01-29  2:51                             ` Arve Hjønnevåg
  2009-01-29  3:45                               ` KAMEZAWA Hiroyuki
  2009-01-29 22:06                               ` Pavel Machek
  0 siblings, 2 replies; 41+ messages in thread
From: Arve Hjønnevåg @ 2009-01-29  2:51 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Greg KH, Alan Cox, Pavel Machek, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel, linux-omap@vger.kernel.org,
	Tony Lindgren, ext Juha Yrj・, viktor.rosendahl,
	Trilok Soni

On Wed, Jan 28, 2009 at 5:48 PM, KOSAKI Motohiro
<kosaki.motohiro@jp.fujitsu.com> 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

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

* Re: lowmemory android driver not needed?
  2009-01-29  2:51                             ` Arve Hjønnevåg
@ 2009-01-29  3:45                               ` KAMEZAWA Hiroyuki
  2009-01-29  4:27                                 ` Greg KH
  2009-01-29 22:06                               ` Pavel Machek
  1 sibling, 1 reply; 41+ messages in thread
From: KAMEZAWA Hiroyuki @ 2009-01-29  3:45 UTC (permalink / raw)
  To: Arve Hjønnevåg
  Cc: KOSAKI Motohiro, Greg KH, Alan Cox, Pavel Machek, Brian Swetland,
	arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrj・, viktor.rosendahl, Trilok Soni

On Wed, 28 Jan 2009 18:51:07 -0800
Arve Hjønnevåg <arve@android.com> 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


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

* Re: lowmemory android driver not needed?
  2009-01-29  3:45                               ` KAMEZAWA Hiroyuki
@ 2009-01-29  4:27                                 ` Greg KH
  2009-01-29  4:43                                   ` KOSAKI Motohiro
  0 siblings, 1 reply; 41+ messages in thread
From: Greg KH @ 2009-01-29  4:27 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Arve Hjønnevåg, KOSAKI Motohiro, Alan Cox, Pavel Machek,
	Brian Swetland, arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrj・, viktor.rosendahl, Trilok Soni

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 <arve@android.com> 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

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

* Re: lowmemory android driver not needed?
  2009-01-29  4:27                                 ` Greg KH
@ 2009-01-29  4:43                                   ` KOSAKI Motohiro
  2009-01-29  4:59                                     ` Greg KH
  0 siblings, 1 reply; 41+ messages in thread
From: KOSAKI Motohiro @ 2009-01-29  4:43 UTC (permalink / raw)
  To: Greg KH
  Cc: kosaki.motohiro, KAMEZAWA Hiroyuki, Arve Hj�nnev虍,
	Alan Cox, Pavel Machek, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel, linux-omap@vger.kernel.org,
	Tony Lindgren, ext Juha Yrj�l・, viktor.rosendahl,
	Trilok Soni

> > > 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.




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

* Re: lowmemory android driver not needed?
  2009-01-29  4:43                                   ` KOSAKI Motohiro
@ 2009-01-29  4:59                                     ` Greg KH
  2009-01-29  5:29                                       ` KOSAKI Motohiro
  0 siblings, 1 reply; 41+ messages in thread
From: Greg KH @ 2009-01-29  4:59 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: KAMEZAWA Hiroyuki, Arve Hj�nnev虍, Alan Cox,
	Pavel Machek, Brian Swetland, arve, San Mehat, Robert Love,
	linux-kernel, linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrj�l・, viktor.rosendahl, Trilok Soni

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

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

* Re: lowmemory android driver not needed?
  2009-01-29  4:59                                     ` Greg KH
@ 2009-01-29  5:29                                       ` KOSAKI Motohiro
  2009-01-30  6:20                                         ` Greg KH
  0 siblings, 1 reply; 41+ messages in thread
From: KOSAKI Motohiro @ 2009-01-29  5:29 UTC (permalink / raw)
  To: Greg KH
  Cc: kosaki.motohiro, KAMEZAWA Hiroyuki, Arve Hj?nnev虍,
	Alan Cox, Pavel Machek, Brian Swetland, arve, San Mehat,
	Robert Love, linux-kernel, linux-omap@vger.kernel.org,
	Tony Lindgren, ext Juha Yrj?l・ , viktor.rosendahl,
	Trilok Soni

> > > > > 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.




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

* Re: lowmemory android driver not needed?
  2009-01-29  2:51                             ` Arve Hjønnevåg
  2009-01-29  3:45                               ` KAMEZAWA Hiroyuki
@ 2009-01-29 22:06                               ` Pavel Machek
  2009-02-03 14:08                                 ` KOSAKI Motohiro
  1 sibling, 1 reply; 41+ messages in thread
From: Pavel Machek @ 2009-01-29 22:06 UTC (permalink / raw)
  To: Arve Hj?nnev?g
  Cc: KOSAKI Motohiro, Greg KH, Alan Cox, Brian Swetland, arve,
	San Mehat, Robert Love, linux-kernel, linux-omap@vger.kernel.org,
	Tony Lindgren, ext Juha Yrj??????, viktor.rosendahl, Trilok Soni

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

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

* Re: lowmemory android driver not needed?
  2009-01-29  5:29                                       ` KOSAKI Motohiro
@ 2009-01-30  6:20                                         ` Greg KH
  2009-01-30  6:41                                           ` Brian Swetland
  0 siblings, 1 reply; 41+ messages in thread
From: Greg KH @ 2009-01-30  6:20 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: KAMEZAWA Hiroyuki, Arve Hj?nnev虍, Alan Cox, Pavel Machek,
	Brian Swetland, arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren, ext Juha Yrj?l・,
	viktor.rosendahl, Trilok Soni

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

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

* Re: lowmemory android driver not needed?
  2009-01-30  6:20                                         ` Greg KH
@ 2009-01-30  6:41                                           ` Brian Swetland
  2009-02-03 13:40                                             ` KOSAKI Motohiro
  0 siblings, 1 reply; 41+ messages in thread
From: Brian Swetland @ 2009-01-30  6:41 UTC (permalink / raw)
  To: Greg KH
  Cc: KOSAKI Motohiro, KAMEZAWA Hiroyuki, Arve Hj?nnev虍,
	Alan Cox, Pavel Machek, arve, San Mehat, Robert Love,
	linux-kernel, linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrj?l・, viktor.rosendahl, Trilok Soni

[Greg KH <greg@kroah.com>]
> > 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

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

* Re: lowmemory android driver not needed?
  2009-01-30  6:41                                           ` Brian Swetland
@ 2009-02-03 13:40                                             ` KOSAKI Motohiro
  0 siblings, 0 replies; 41+ messages in thread
From: KOSAKI Motohiro @ 2009-02-03 13:40 UTC (permalink / raw)
  To: Brian Swetland
  Cc: kosaki.motohiro, Greg KH, KAMEZAWA Hiroyuki, Arve Hj?nnev虍,
	Alan Cox, Pavel Machek, arve, San Mehat, Robert Love,
	linux-kernel, linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrj?l・, viktor.rosendahl, Trilok Soni

Hi

> [Greg KH <greg@kroah.com>]
> > > 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 agree this patch address correct requirement.


> 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".  

ok. I believe you.
I also hope that I'm working with various background guys.


thanks.

> 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."






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

* Re: lowmemory android driver not needed?
  2009-01-29 22:06                               ` Pavel Machek
@ 2009-02-03 14:08                                 ` KOSAKI Motohiro
  0 siblings, 0 replies; 41+ messages in thread
From: KOSAKI Motohiro @ 2009-02-03 14:08 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Arve Hj?nnev?g, Greg KH, Alan Cox, Brian Swetland, arve,
	San Mehat, Robert Love, linux-kernel, linux-omap@vger.kernel.org,
	Tony Lindgren, ext Juha Yrj??????, viktor.rosendahl, Trilok Soni

Hi!

2009/1/30 Pavel Machek <pavel@suse.cz>:
> 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,

good idea! :)


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

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

* Re: lowmemory android driver not needed?
  2009-01-16  9:02                       ` Paul Mundt
  2009-01-16 11:16                         ` KOSAKI Motohiro
@ 2009-02-03 20:55                         ` Tony Lindgren
  1 sibling, 0 replies; 41+ messages in thread
From: Tony Lindgren @ 2009-02-03 20:55 UTC (permalink / raw)
  To: Paul Mundt, Greg KH, Trilok Soni, Arve Hj?nnev?g, Alan Cox,
	Pavel Machek, Brian Swetland, arve, San Mehat, Robert Love,
	linux-kernel, linux-omap@vger.kernel.org, ext Juha Yrj?l?,
	viktor.rosendahl

* Paul Mundt <lethal@linux-sh.org> [090116 01:05]:
> 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.

FYI, I've dropped the lowmem.c module from the linux-omap tree
considering the rest of this thread. The commit is
c00b5565aaaefadfe68d7c32d05617c81bb9edff for reference.

Regards,

Tony

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

* Re: lowmemory android driver not needed?
  2009-01-21  2:50                             ` KOSAKI Motohiro
  2009-01-21  3:05                               ` Paul Mundt
  2009-01-21  3:29                               ` KAMEZAWA Hiroyuki
@ 2009-04-01 18:38                               ` Trilok Soni
  2009-04-01 19:33                                 ` David Rientjes
  2 siblings, 1 reply; 41+ messages in thread
From: Trilok Soni @ 2009-04-01 18:38 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Greg KH, Paul Mundt, Arve Hj?nnev?g, Alan Cox, Pavel Machek,
	Brian Swetland, arve, San Mehat, Robert Love, linux-kernel,
	linux-omap@vger.kernel.org, Tony Lindgren, ext Juha Yrj?l?,
	viktor.rosendahl

Hi

On Wed, Jan 21, 2009 at 8:20 AM, KOSAKI Motohiro
<kosaki.motohiro@jp.fujitsu.com> 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

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

* Re: lowmemory android driver not needed?
  2009-04-01 18:38                               ` Trilok Soni
@ 2009-04-01 19:33                                 ` David Rientjes
  0 siblings, 0 replies; 41+ messages in thread
From: David Rientjes @ 2009-04-01 19:33 UTC (permalink / raw)
  To: Trilok Soni
  Cc: KOSAKI Motohiro, Greg KH, Paul Mundt, Arve Hj?nnev?g, Alan Cox,
	Pavel Machek, Brian Swetland, arve, San Mehat, Robert Love,
	linux-kernel, linux-omap@vger.kernel.org, Tony Lindgren,
	ext Juha Yrj?l?, viktor.rosendahl

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).

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

end of thread, other threads:[~2009-04-01 19:34 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-14  1:02 lowmemory android driver not needed? Greg KH
2009-01-14  2:18 ` Brian Swetland
2009-01-14  2:30   ` Arve Hjønnevåg
2009-01-14  3:52     ` Greg KH
2009-01-14 10:43       ` Pavel Machek
2009-01-14 10:48         ` Alan Cox
2009-01-14 12:18           ` MinChan Kim
2009-01-14 22:26           ` Arve Hjønnevåg
2009-01-14 23:17             ` Greg KH
2009-01-14 23:32               ` Arve Hjønnevåg
2009-01-15  0:12                 ` Greg KH
2009-01-15  0:54                   ` [PATCH] Staging: android: Add lowmemorykiller documentation Arve Hjønnevåg
2009-01-15  0:59                     ` Greg KH
2009-01-15 13:32                   ` lowmemory android driver not needed? Trilok Soni
2009-01-15 23:44                     ` Greg KH
2009-01-16  9:02                       ` Paul Mundt
2009-01-16 11:16                         ` KOSAKI Motohiro
2009-01-16 15:23                           ` Greg KH
2009-01-21  2:50                             ` KOSAKI Motohiro
2009-01-21  3:05                               ` Paul Mundt
2009-01-21  3:29                               ` KAMEZAWA Hiroyuki
2009-04-01 18:38                               ` Trilok Soni
2009-04-01 19:33                                 ` David Rientjes
2009-02-03 20:55                         ` Tony Lindgren
2009-01-16 11:42                     ` KOSAKI Motohiro
2009-01-16 13:18                       ` KOSAKI Motohiro
2009-01-22  6:13                         ` Arve Hjønnevåg
2009-01-29  1:48                           ` KOSAKI Motohiro
2009-01-29  2:51                             ` Arve Hjønnevåg
2009-01-29  3:45                               ` KAMEZAWA Hiroyuki
2009-01-29  4:27                                 ` Greg KH
2009-01-29  4:43                                   ` KOSAKI Motohiro
2009-01-29  4:59                                     ` Greg KH
2009-01-29  5:29                                       ` KOSAKI Motohiro
2009-01-30  6:20                                         ` Greg KH
2009-01-30  6:41                                           ` Brian Swetland
2009-02-03 13:40                                             ` KOSAKI Motohiro
2009-01-29 22:06                               ` Pavel Machek
2009-02-03 14:08                                 ` KOSAKI Motohiro
2009-01-15  7:55           ` David Rientjes
2009-01-14  6:45     ` MinChan Kim

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