public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* Re: lowmemory android driver not needed?
       [not found]                 ` <20090115001224.GC11328@kroah.com>
@ 2009-01-15 13:32                   ` Trilok Soni
  2009-01-15 23:44                     ` Greg KH
  2009-01-16 11:42                     ` KOSAKI Motohiro
  0 siblings, 2 replies; 26+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ 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; 26+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ 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; 26+ 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] 26+ 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; 26+ 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

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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 26+ 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; 26+ 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] 26+ 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; 26+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ 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; 26+ 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

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ 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; 26+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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] 26+ 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; 26+ 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

* 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] 26+ 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; 26+ 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
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 26+ 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; 26+ 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] 26+ messages in thread

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

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20090114010223.GA21380@kroah.com>
     [not found] ` <20090114021801.GA14759@bulgaria.corp.google.com>
     [not found]   ` <d6200be20901131830r7103ff8eoe0314f9faf89d89b@mail.gmail.com>
     [not found]     ` <20090114035237.GB16442@kroah.com>
     [not found]       ` <20090114104307.GA20451@elf.ucw.cz>
     [not found]         ` <20090114104834.18387fca@lxorguk.ukuu.org.uk>
     [not found]           ` <d6200be20901141426r1d48ea33gc9a380451e4aaf93@mail.gmail.com>
     [not found]             ` <20090114231739.GB24111@kroah.com>
     [not found]               ` <d6200be20901141532y6f236104kd521041d640c152e@mail.gmail.com>
     [not found]                 ` <20090115001224.GC11328@kroah.com>
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

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