OpenVZ Forum - RDF feed
type: channel
items: [PATCH] BC: resource beancounters (v2)
items: [PATCH 1/6] BC: kconfig
items: [PATCH 2/6] BC: beancounters core (API)
items: [PATCH 3/6] BC: context inheriting and changing
items: [PATCH 4/6] BC: user interface (syscalls)
items: [PATCH 5/6] BC: kernel memory accounting (core)
items: [PATCH 6/6] BC: kernel memory accounting (marks)
items: Re: [PATCH 2/6] BC: beancounters core (API)
items: Re: [PATCH 1/6] BC: kconfig
items: Re: [PATCH 4/6] BC: user interface (syscalls)
items: Re: [PATCH] BC: resource beancounters (v2)
items: Re: [PATCH 6/6] BC: kernel memory accounting (marks)
items: Re: [PATCH 5/6] BC: kernel memory accounting (core)
items: Re: [ckrm-tech] [PATCH 1/6] BC: kconfig
items: Re: BC: resource beancounters (v2)
items: Re: Re: BC: resource beancounters (v2)

[PATCH] BC: resource beancounters (v2)
type: item
description: The following patch set presents base of Resource Beancounters (BC). BC allows to account and control consumption of kernel resources used by group of processes. Draft UBC description on OpenVZ wiki can be found at http://wiki.openvz.org/UBC_parameters The full BC patch set allows to control: - kernel memory. All the kernel objects allocatable on user demand should be accounted and limited for DoS protection. E.g. page tables, task structs, vmas etc. - virtual memory pages. BCs allow to limit a container to some amount of memory and introduces 2-level OOM killer taking into account container's consumption. pages shared between containers are correctly charged as fractions (tunable). - network buffers. These includes TCP/IP rcv/snd buffers, dgram snd buffers, unix, netlinks and other buffers. - minor resources accounted/limited by number: tasks, files, flocks, ptys, siginfo, pinned dcache mem, sockets, iptentries (for containers with virtualized networking) As the first step we want to propose for discussion the most complicated parts of resource management: kernel memory and virtual memory. The patch set to be sent provides core for BC and management of kernel memory only. Virtual memory management will be sent in a couple of days. The patches in these series are: diff-bc-kconfig.patch: Adds kernel/bc/Kconfig file with UBC options and includes it into arch Kconfigs diff-bc-core.patch: Contains core functionality and interfaces of BC: find/create beancounter, initialization, charge/uncharge of resource, core objects' declarations. diff-bc-task.patch: Contains code responsible for setting BC on task, it's inheriting and setting host context in interrupts. Task contains three beancounters: 1. exec_bc - current context. all resources are charged to this beancounter. 2. fork_bc - beancounter which is inherited by task's children on fork diff-bc-syscalls.patch: Patch adds system calls for BC management: 1. sys_get_bcid - get current BC id 2. sys_set_bcid - changes exec_ a…
creator: dev
date: 2006-08-23T10:44:18-00:00

[PATCH 1/6] BC: kconfig
type: item
description: Add kernel/bc/Kconfig file with BC options and include it into arch Kconfigs Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- arch/i386/Kconfig | 2 ++ arch/ia64/Kconfig | 2 ++ arch/powerpc/Kconfig | 2 ++ arch/ppc/Kconfig | 2 ++ arch/sparc/Kconfig | 2 ++ arch/sparc64/Kconfig | 2 ++ arch/x86_64/Kconfig | 2 ++ kernel/bc/Kconfig | 25 +++++++++++++++++++++++++ 8 files changed, 39 insertions(+) --- ./arch/i386/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 +++ ./arch/i386/Kconfig 2006-07-28 14:10:41.000000000 +0400 @@ -1146,6 +1146,8 @@ source "crypto/Kconfig" source "lib/Kconfig" +source "kernel/bc/Kconfig" + # # Use the generic interrupt handling code in kernel/irq/: # --- ./arch/ia64/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 +++ ./arch/ia64/Kconfig 2006-07-28 14:10:56.000000000 +0400 @@ -481,6 +481,8 @@ source "fs/Kconfig" source "lib/Kconfig" +source "kernel/bc/Kconfig" + # # Use the generic interrupt handling code in kernel/irq/: #Add kernel/bc/Kconfig file with BC options and include it into arch Kconfigs Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- arch/i386/Kconfig | 2 ++ arch/ia64/Kconfig | 2 ++ arch/powerpc/Kconfig | 2 ++ arch/ppc/Kconfig | 2 ++ arch/sparc/Kconfig | 2 ++ arch/sparc64/Kconfig | 2 ++ arch/x86_64/Kconfig | 2 ++ kernel/bc/Kconfig | 25 +++++++++++++++++++++++++ 8 files changed, 39 insertions(+) --- ./arch/i386/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 +++ ./arch/i386/Kconfig 2006-07-28 14:10:41.000000000 +0400 @@ -1146,6 +1146,8 @@ source "crypto/Kconfig" source "lib/Kconfig" +source "kernel/bc/Kconfig" + # # Use the generic interrupt handling code in kernel/irq/: # --- ./arch/ia64/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 +++ ./arch/ia64/Kconfig 2006-07-28 14:10:56.000000000 +0400 @@ -481,6 +481,8 @@ source "fs/Kconfig" source "lib/Kconfig" +source "kernel/bc/Kconfig" + # # Use the generic interrupt handling code in kernel/irq/: # --- ./a…
creator: dev
date: 2006-08-23T10:59:31-00:00

[PATCH 2/6] BC: beancounters core (API)
type: item
description: Core functionality and interfaces of BC: find/create beancounter, initialization, charge/uncharge of resource, core objects' declarations. Basic structures: bc_resource_parm - resource description beancounter - set of resources, id, lock Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- include/bc/beancounter.h | 140 ++++++++++++++++++++++ init/main.c | 4 kernel/Makefile | 1 kernel/bc/Makefile | 7 + kernel/bc/beancounter.c | 292 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 444 insertions(+) --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./include/bc/beancounter.h 2006-08-21 13:14:01.000000000 +0400 @@ -0,0 +1,140 @@ +/* + * include/bc/beancounter.h + * + * Copyright (C) 2006 OpenVZ. SWsoft Inc + * + */ + +#ifndef _LINUX_BEANCOUNTER_H +#define _LINUX_BEANCOUNTER_H + +/* + * Resource list. + */ + +#define BC_RESOURCES 0 + +struct bc_resource_parm { + unsigned long barrier; /* A barrier over which resource allocations + * are failed gracefully. e.g. if the amount + * of consumed memory is over the barrier + * further sbrk() or mmap() calls fail, the + * existing processes are not killed. + */ + unsigned long limit; /* hard resource limit */ + unsigned long held; /* consumed resources */ + unsigned long maxheld; /* maximum amount of consumed resources */ + unsigned long minheld; /* minumum amount of consumed resources */ + unsigned long failcnt; /* count of failed charges */ +}; + +/* + * Kernel internal part. + */ + +#ifdef __KERNEL__ + +#include <linux/config.h> +#include <linux/spinlock.h> +#include <linux/list.h> +#include <asm/atomic.h> + +#define BC_MAXVALUE LONG_MAX + +/* + * Resource management structures + * Serialization issues: + * beancounter list management is protected via bc_hash_lock + * task pointers are set only for current task and only once + * refcount is managed atomically + * value and limit comparison and change are protected by per-bc spinlock + */ + +struct beancounter +{ + at…
creator: dev
date: 2006-08-23T11:00:37-00:00

[PATCH 3/6] BC: context inheriting and changing
type: item
description: Contains code responsible for setting BC on task, it's inheriting and setting host context in interrupts. Task references 2 beancounters: 1. exec_bc: current context. all resources are charged to this beancounter. 3. fork_bc: beancounter which is inherited by task's children on fork Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- include/linux/sched.h | 5 ++++ include/bc/task.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++++ kernel/fork.c | 17 +++++++++++++-- kernel/irq/handle.c | 9 ++++++++ kernel/softirq.c | 8 +++++++ kernel/bc/Makefile | 1 kernel/bc/beancounter.c | 3 ++ kernel/bc/misc.c | 32 +++++++++++++++++++++++++++++ 8 files changed, 125 insertions(+), 2 deletions(-) --- ./include/linux/sched.h.ve2 2006-08-21 13:15:39.000000000 +0400 +++ ./include/linux/sched.h 2006-08-21 13:26:03.000000000 +0400 @@ -83,6 +83,8 @@ struct sched_param { #include <linux/timer.h> #include <linux/hrtimer.h> +#include <bc/task.h> + #include <asm/processor.h> struct exec_domain; @@ -1032,6 +1034,9 @@ struct task_struct { spinlock_t delays_lock; struct task_delay_info *delays; #endif +#ifdef CONFIG_BEANCOUNTERS + struct task_beancounter task_bc; +#endif }; static inline pid_t process_group(struct task_struct *tsk) --- ./include/bc/task.h.ve2 2006-08-21 13:26:03.000000000 +0400 +++ ./include/bc/task.h 2006-08-21 13:35:04.000000000 +0400 @@ -0,0 +1,52 @@ +/* + * include/bc/task.h + * + * Copyright (C) 2006 OpenVZ. SWsoft Inc + * + */ + +#ifndef __BC_TASK_H_ +#define __BC_TASK_H_ + +#include <linux/config.h> + +struct beancounter; + +struct task_beancounter { + struct beancounter *exec_bc; + struct beancounter *fork_bc; +}; + +#ifdef CONFIG_BEANCOUNTERS + +#define get_exec_bc() (current->task_bc.exec_bc) +#define set_exec_bc(new) \ + ({ \ + struct beancounter *old; \ + struct task_beancounter *tbc; \ + tbc = &current->task_bc; \ + old = tbc->exec_bc; \ + tbc->exec_bc = new; \ + old; \ + }) +#define reset_exec_bc(old, exp) \ + do { \ + st…
creator: dev
date: 2006-08-23T11:02:40-00:00

[PATCH 4/6] BC: user interface (syscalls)
type: item
description: Add the following system calls for BC management: 1. sys_get_bcid - get current BC id 2. sys_set_bcid - change exec_ and fork_ BCs on current 3. sys_set_bclimit - set limits for resources consumtions 4. sys_get_bcstat - return br_resource_parm on resource Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <xemul@sw.ru> --- arch/i386/kernel/syscall_table.S | 4 + arch/ia64/kernel/entry.S | 4 + arch/sparc/kernel/entry.S | 2 arch/sparc/kernel/systbls.S | 6 + arch/sparc64/kernel/entry.S | 2 arch/sparc64/kernel/systbls.S | 10 ++- include/asm-i386/unistd.h | 6 + include/asm-ia64/unistd.h | 6 + include/asm-powerpc/systbl.h | 4 + include/asm-powerpc/unistd.h | 6 + include/asm-sparc/unistd.h | 4 + include/asm-sparc64/unistd.h | 4 + include/asm-x86_64/unistd.h | 10 ++- kernel/sys_ni.c | 6 + kernel/bc/Makefile | 1 kernel/bc/sys.c | 120 +++++++++++++++++++++++++++++++++++++++ 16 files changed, 186 insertions(+), 9 deletions(-) --- ./arch/i386/kernel/syscall_table.S.ve3 2006-08-21 13:15:37.000000000 +0400 +++ ./arch/i386/kernel/syscall_table.S 2006-08-21 14:15:47.000000000 +0400 @@ -318,3 +318,7 @@ ENTRY(sys_call_table) .long sys_vmsplice .long sys_move_pages .long sys_getcpu + .long sys_get_bcid + .long sys_set_bcid /* 320 */ + .long sys_set_bclimit + .long sys_get_bcstat --- ./arch/ia64/kernel/entry.S.ve3 2006-08-21 13:15:37.000000000 +0400 +++ ./arch/ia64/kernel/entry.S 2006-08-21 14:17:07.000000000 +0400 @@ -1610,5 +1610,9 @@ sys_call_table: data8 sys_sync_file_range // 1300 data8 sys_tee data8 sys_vmsplice + data8 sys_get_bcid + data8 sys_set_bcid + data8 sys_set_bclimit // 1305 + data8 sys_get_bcstat .org sys_call_table + 8*NR_syscalls // guard against failures to increase NR_syscalls --- ./arch/sparc/kernel/entry.S.ve3 2006-07-10 12:39:10.000000000 +0400 +++ ./arch/sparc/kernel/entry.S 2006-08-21 14:29:44.000000000 +0400 @@ -37,7 +37,7 @@ #define curptr g6 -#define NR_SYSCALLS 300 /* Each OS is different... */ +#define NR_SYSCALLS 304 /* Each OS is…
creator: dev
date: 2006-08-23T11:03:39-00:00

[PATCH 5/6] BC: kernel memory accounting (core)
type: item
description: Introduce BC_KMEMSIZE resource which accounts kernel objects allocated by task's request. Reference to BC is kept on struct page or slab object. For slabs each struct slab contains a set of pointers corresponding objects are charged to. Allocation charge rules: 1. Pages - if allocation is performed with __GFP_BC flag - page is charged to current's exec_bc. 2. Slabs - kmem_cache may be created with SLAB_BC flag - in this case each allocation is charged. Caches used by kmalloc are created with SLAB_BC | SLAB_BC_NOCHARGE flags. In this case only __GFP_BC allocations are charged. Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- include/linux/gfp.h | 8 ++- include/linux/mm.h | 6 ++ include/linux/slab.h | 4 + include/linux/vmalloc.h | 1 include/bc/beancounter.h | 4 + include/bc/kmem.h | 33 ++++++++++++ kernel/bc/Makefile | 1 kernel/bc/beancounter.c | 3 + kernel/bc/kmem.c | 89 ++++++++++++++++++++++++++++++++++ mm/mempool.c | 2 mm/page_alloc.c | 11 ++++ mm/slab.c | 121 ++++++++++++++++++++++++++++++++++++++--------- mm/vmalloc.c | 6 ++ 13 files changed, 264 insertions(+), 25 deletions(-) --- ./include/linux/gfp.h.bckmem 2006-07-28 18:43:52.000000000 +0400 +++ ./include/linux/gfp.h 2006-07-31 16:32:22.000000000 +0400 @@ -46,8 +46,10 @@ struct vm_area_struct; #define __GFP_NOMEMALLOC ((__force gfp_t)0x10000u) /* Don't use emergency reserves */ #define __GFP_HARDWALL ((__force gfp_t)0x20000u) /* Enforce hardwall cpuset memory allocs */ #define __GFP_THISNODE ((__force gfp_t)0x40000u)/* No fallback, no policies */ +#define __GFP_BC ((__force gfp_t)0x80000u) /* Charge allocation with BC */ +#define __GFP_BC_LIMIT ((__force gfp_t)0x100000u) /* Charge against BC limit */ -#define __GFP_BITS_SHIFT 20 /* Room for 20 __GFP_FOO bits */ +#define __GFP_BITS_SHIFT 21 /* Room for 21 __GFP_FOO bits */ #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1)) /* if you forget to add the bitmask here kernel will crash, period */ @@ -54…
creator: dev
date: 2006-08-23T11:04:41-00:00

[PATCH 6/6] BC: kernel memory accounting (marks)
type: item
description: Mark some kmem caches with SLAB_BC and some allocations with __GFP_BC to cause charging/limiting of appropriate kernel resources. Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- arch/i386/kernel/ldt.c | 4 ++-- arch/i386/mm/init.c | 4 ++-- arch/i386/mm/pgtable.c | 6 ++++-- drivers/char/tty_io.c | 10 +++++----- fs/file.c | 8 ++++---- fs/locks.c | 2 +- fs/namespace.c | 3 ++- fs/select.c | 7 ++++--- include/asm-i386/thread_info.h | 4 ++-- include/asm-ia64/pgalloc.h | 24 +++++++++++++++++------- include/asm-x86_64/pgalloc.h | 12 ++++++++---- include/asm-x86_64/thread_info.h | 5 +++-- ipc/msgutil.c | 4 ++-- ipc/sem.c | 7 ++++--- ipc/util.c | 8 ++++---- kernel/fork.c | 15 ++++++++------- kernel/posix-timers.c | 3 ++- kernel/signal.c | 2 +- kernel/user.c | 2 +- mm/rmap.c | 3 ++- mm/shmem.c | 3 ++- 21 files changed, 80 insertions(+), 56 deletions(-) --- ./arch/i386/kernel/ldt.c.bcslabs 2006-04-21 11:59:31.000000000 +0400 +++ ./arch/i386/kernel/ldt.c 2006-08-01 13:22:30.000000000 +0400 @@ -39,9 +39,9 @@ static int alloc_ldt(mm_context_t *pc, i oldsize = pc->size; mincount = (mincount+511)&(~511); if (mincount*LDT_ENTRY_SIZE > PAGE_SIZE) - newldt = vmalloc(mincount*LDT_ENTRY_SIZE); + newldt = vmalloc_bc(mincount*LDT_ENTRY_SIZE); else - newldt = kmalloc(mincount*LDT_ENTRY_SIZE, GFP_KERNEL); + newldt = kmalloc(mincount*LDT_ENTRY_SIZE, GFP_KERNEL_BC); if (!newldt) return -ENOMEM; --- ./arch/i386/mm/init.c.bcslabs 2006-07-10 12:39:10.000000000 +0400 +++ ./arch/i386/mm/init.c 2006-08-01 13:17:07.000000000 +0400 @@ -680,7 +680,7 @@ void __init pgtable_cache_init(void) pmd_cache = kmem_cache_create("pmd", PTRS_PER_PMD*sizeof(pmd_t), PTRS_PER_PMD*sizeof(pmd_t), - 0, + SLAB_BC, pmd_ctor, NULL); if (!pmd_cache) @@ -689,7 +689,7 @@ void __init pgtable_cache_init(void) pgd_cache = kmem_cache_create("pgd", PTRS_PER_PGD*sizeof(pgd_t), PTRS_PER_PGD*sizeof(pgd_t), - 0, + SLAB_BC, pgd_ctor, PTRS_PER_PMD == 1 ? pgd_dtor : NULL); if (!pgd_cache) --- ./…
creator: dev
date: 2006-08-23T11:05:46-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: On Wednesday 23 August 2006 13:03, Kirill Korotaev wrote: > +#ifdef CONFIG_BEANCOUNTERS > +extern struct hlist_head bc_hash[]; > +extern spinlock_t bc_hash_lock; I wonder who pokes into that hash from other files? Looks a bit dangerous. > +void __put_beancounter(struct beancounter *bc); > +static inline void put_beancounter(struct beancounter *bc) > +{ > + __put_beancounter(bc); > +} The wrapper seems pointless too. The file could use a overview comment what the various counter types actually are. > + bc_print_id(bc, uid, sizeof(uid)); > + printk(KERN_WARNING "BC %s %s warning: %s " Doesn't this need some rate limiting? Or can it be only triggered by code bugs? > + bc = &default_beancounter; > + memset(bc, 0, sizeof(default_beancounter)); You don't trust the BSS to be zero? @) -Andi
creator: Andi Kleen
date: 2006-08-23T11:37:48-00:00

Re: [PATCH 1/6] BC: kconfig
type: item
description: On Wed, Aug 23, 2006 at 03:01:55PM +0400, Kirill Korotaev wrote: > --- ./kernel/bc/Kconfig.bckm > +++ ./kernel/bc/Kconfig > @@ -0,0 +1,25 @@ > +# > +# Resource beancounters (BC) > +# > +# Copyright (C) 2006 OpenVZ. SWsoft Inc > + > +menu "User resources" > + > +config BEANCOUNTERS > + bool "Enable resource accounting/control" > + default n > + help > + This patch provides accounting and allows to configure > + limits for user's consumption of exhaustible system resources. If BC will be merged there will be no patch, so text should be rewritten slightly. > + The most important resource controlled by this patch is > unswappable + memory (either mlock'ed or used by internal kernel > structures and + buffers). The main goal of this patch is to > protect processes > + from running short of important resources because of an > accidental > + misbehavior of processes or malicious activity aiming to > ``kill'' + the system. It's worth to mention that resource limits > configured + by setrlimit(2) do not give an acceptable level of > protection + because they cover only small fraction of resources > and work on a + per-process basis. Per-process accounting doesn't > prevent malicious > + users from spawning a lot of resource-consuming processes.
creator: Alexey Dobriyan
date: 2006-08-23T13:04:52-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: Andi Kleen wrote: > On Wednesday 23 August 2006 13:03, Kirill Korotaev wrote: > > >>+#ifdef CONFIG_BEANCOUNTERS >>+extern struct hlist_head bc_hash[]; >>+extern spinlock_t bc_hash_lock; > > > I wonder who pokes into that hash from other files? Looks a bit dangerous. it was kernel/ub/proc.c with proc interface :) however, we removed it from this patchset version, but forgot extern's... will remove >>+void __put_beancounter(struct beancounter *bc); >>+static inline void put_beancounter(struct beancounter *bc) >>+{ >>+ __put_beancounter(bc); >>+} > > > The wrapper seems pointless too. yep, almost the same reason :) > The file could use a overview comment what the various counter > types actually are. you mean comment about what resource parameters we introduce? ok, will add it with each resource patch. >>+ bc_print_id(bc, uid, sizeof(uid)); >>+ printk(KERN_WARNING "BC %s %s warning: %s " > > > Doesn't this need some rate limiting? Or can it be only triggered > by code bugs? only due to code bugs. >>+ bc = &default_beancounter; >>+ memset(bc, 0, sizeof(default_beancounter)); > > > You don't trust the BSS to be zero? @) :)) Kirill
creator: dev
date: 2006-08-23T13:25:42-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: On Wed, Aug 23, 2006 at 03:03:07PM +0400, Kirill Korotaev wrote: > --- /dev/null > +++ ./include/bc/beancounter.h > +#define BC_RESOURCES 0 Do you want userspace to see it? > +struct bc_resource_parm { > + unsigned long barrier; /* A barrier over which resource allocations > + * are failed gracefully. e.g. if the amount > + * of consumed memory is over the barrier > + * further sbrk() or mmap() calls fail, the > + * existing processes are not killed. > + */ > + unsigned long limit; /* hard resource limit */ > + unsigned long held; /* consumed resources */ > + unsigned long maxheld; /* maximum amount of consumed resources */ > + unsigned long minheld; /* minumum amount of consumed resources */ Stupid question: when minimum amount is useful? > +/* > + * Kernel internal part. > + */ Redundant comment, YMMV. > +#ifdef __KERNEL__ > + > +#include <linux/config.h> config.h is unneeded. You can drop it from everywhere. > +struct beancounter > +{ nit: "struct beancounter {" > + atomic_t bc_refcount; > + spinlock_t bc_lock; > + uid_t bc_id; > + struct hlist_node hash; > + > + /* resources statistics and settings */ > + struct bc_resource_parm bc_parms[BC_RESOURCES]; > +}; > + > +enum severity { BC_BARRIER, BC_LIMIT, BC_FORCE }; bc_severity? > --- /dev/null 2006-07-18 14:52:43.075228448 +0400 > +++ ./kernel/bc/beancounter.c 2006-08-21 13:13:11.000000000 +0400 > +#define bc_hash_fun(x) ((((x) >> 8) ^ (x)) & (BC_HASH_SIZE - 1)) > + > +struct hlist_head bc_hash[BC_HASH_SIZE]; > +spinlock_t bc_hash_lock; > + > +EXPORT_SYMBOL(bc_hash); > +EXPORT_SYMBOL(bc_hash_lock); tasklist_lock was unexported recently and this looks equally low-level. I couldn't find place in patchbomb where you use it in modular fashion. perhaps, i need more sleep. > +void __put_beancounter(struct beancounter *bc) > +{ > + unsigned long flags; > + > + /* equivalent to atomic_dec_and_lock_irqsave() */ > + local_irq_save(flags); > + if (likely(!atomic_dec_and_lock(&bc->bc_refcount, &bc_hash_lock))) { > + local_ir…
creator: Alexey Dobriyan
date: 2006-08-23T13:30:55-00:00

Re: [PATCH 4/6] BC: user interface (syscalls)
type: item
description: Sorry, wrong patch was attached. This one fixes fixed but missed SPARC hunks. -------------------------------------- Add the following system calls for BC management: 1. sys_get_bcid - get current BC id 2. sys_set_bcid - change exec_ and fork_ BCs on current 3. sys_set_bclimit - set limits for resources consumtions 4. sys_get_bcstat - return br_resource_parm on resource Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- arch/i386/kernel/syscall_table.S | 4 + arch/ia64/kernel/entry.S | 4 + arch/sparc/kernel/entry.S | 2 arch/sparc/kernel/systbls.S | 6 + arch/sparc64/kernel/entry.S | 2 arch/sparc64/kernel/systbls.S | 10 ++- include/asm-i386/unistd.h | 6 + include/asm-ia64/unistd.h | 6 + include/asm-powerpc/systbl.h | 4 + include/asm-powerpc/unistd.h | 6 + include/asm-sparc/unistd.h | 4 + include/asm-sparc64/unistd.h | 4 + include/asm-x86_64/unistd.h | 10 ++- kernel/sys_ni.c | 6 + kernel/bc/Makefile | 1 kernel/bc/sys.c | 120 +++++++++++++++++++++++++++++++++++++++ 16 files changed, 186 insertions(+), 9 deletions(-) --- ./arch/i386/kernel/syscall_table.S.ve3 2006-08-21 13:15:37.000000000 +0400 +++ ./arch/i386/kernel/syscall_table.S 2006-08-21 14:15:47.000000000 +0400 @@ -318,3 +318,7 @@ ENTRY(sys_call_table) .long sys_vmsplice .long sys_move_pages .long sys_getcpu + .long sys_get_bcid + .long sys_set_bcid /* 320 */ + .long sys_set_bclimit + .long sys_get_bcstat --- ./arch/ia64/kernel/entry.S.ve3 2006-08-21 13:15:37.000000000 +0400 +++ ./arch/ia64/kernel/entry.S 2006-08-21 14:17:07.000000000 +0400 @@ -1610,5 +1610,9 @@ sys_call_table: data8 sys_sync_file_range // 1300 data8 sys_tee data8 sys_vmsplice + data8 sys_get_bcid + data8 sys_set_bcid + data8 sys_set_bclimit // 1305 + data8 sys_get_bcstat .org sys_call_table + 8*NR_syscalls // guard against failures to increase NR_syscalls --- ./arch/sparc/kernel/entry.S.ve3 2006-07-10 12:39:10.000000000 +0400 +++ ./arch/sparc/kernel/entry.S 2006-08-21 14:29:44.000000000 +0400 @@ -37,7 +37,7…
creator: dev
date: 2006-08-23T13:40:51-00:00

Re: [PATCH 4/6] BC: user interface (syscalls)
type: item
description: On Wed, Aug 23, 2006 at 03:06:05PM +0400, Kirill Korotaev wrote: > --- ./kernel/bc/sys.c.ve3 > +++ ./kernel/bc/sys.c > +asmlinkage long sys_set_bclimit(uid_t id, unsigned long resource, > + unsigned long *limits) unsigned long __user *limits > +{ > + int error; > + unsigned long flags; > + struct beancounter *bc; > + unsigned long new_limits[2]; > + > + error = -EPERM; > + if(!capable(CAP_SYS_RESOURCE)) > + goto out; > + > + error = -EINVAL; > + if (resource >= BC_RESOURCES) > + goto out; > + > + error = -EFAULT; > + if (copy_from_user(&new_limits, limits, sizeof(new_limits))) > + goto out; > +int sys_get_bcstat(uid_t id, unsigned long resource, > + struct bc_resource_parm *uparm) __user *uparm > +{ > + int error; > + unsigned long flags; > + struct beancounter *bc; > + struct bc_resource_parm parm; > + > + error = -EINVAL; > + if (resource > BC_RESOURCES) > + goto out; > + > + error = -ENOENT; > + bc = beancounter_findcreate(id, 0); > + if (bc == NULL) > + goto out; > + > + spin_lock_irqsave(&bc->bc_lock, flags); > + parm = bc->bc_parms[resource]; > + spin_unlock_irqrestore(&bc->bc_lock, flags); > + put_beancounter(bc); > + > + error = 0; > + if (copy_to_user(uparm, &parm, sizeof(parm))) > + error = -EFAULT;
creator: Alexey Dobriyan
date: 2006-08-23T13:41:47-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: Alexey Dobriyan wrote: > On Wed, Aug 23, 2006 at 03:03:07PM +0400, Kirill Korotaev wrote: > > >>--- /dev/null >>+++ ./include/bc/beancounter.h > > >>+#define BC_RESOURCES 0 > > > Do you want userspace to see it? yep. >>+struct bc_resource_parm { >>+ unsigned long barrier; /* A barrier over which resource allocations >>+ * are failed gracefully. e.g. if the amount >>+ * of consumed memory is over the barrier >>+ * further sbrk() or mmap() calls fail, the >>+ * existing processes are not killed. >>+ */ >>+ unsigned long limit; /* hard resource limit */ >>+ unsigned long held; /* consumed resources */ >>+ unsigned long maxheld; /* maximum amount of consumed resources */ >>+ unsigned long minheld; /* minumum amount of consumed resources */ > > > Stupid question: when minimum amount is useful? to monitor usage statistics (range of used resources). this value will be usefull when ubstat will be added. this field probably would be more logical to add later, but since it is part of user space interface it is left here for not changing API later. >>+/* >>+ * Kernel internal part. >>+ */ > > > Redundant comment, YMMV. > > >>+#ifdef __KERNEL__ >>+ >>+#include <linux/config.h> > > > config.h is unneeded. You can drop it from everywhere. ok. >>+struct beancounter >>+{ > > > nit: > "struct beancounter {" > > >>+ atomic_t bc_refcount; >>+ spinlock_t bc_lock; >>+ uid_t bc_id; >>+ struct hlist_node hash; >>+ >>+ /* resources statistics and settings */ >>+ struct bc_resource_parm bc_parms[BC_RESOURCES]; >>+}; >>+ >>+enum severity { BC_BARRIER, BC_LIMIT, BC_FORCE }; > > > bc_severity? ok > > >>--- /dev/null 2006-07-18 14:52:43.075228448 +0400 >>+++ ./kernel/bc/beancounter.c 2006-08-21 13:13:11.000000000 +0400 > > >>+#define bc_hash_fun(x) ((((x) >> 8) ^ (x)) & (BC_HASH_SIZE - 1)) >>+ >>+struct hlist_head bc_hash[BC_HASH_SIZE]; >>+spinlock_t bc_hash_lock; >>+ >>+EXPORT_SYMBOL(bc_hash); >>+EXPORT_SYMBOL(bc_hash_lock); > > > tasklist_lock was unexported recently and this looks equally lo…
creator: dev
date: 2006-08-23T13:46:45-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: On Wednesday 23 August 2006 15:27, Kirill Korotaev wrote: > Andi Kleen wrote: > > On Wednesday 23 August 2006 13:03, Kirill Korotaev wrote: > > > > > >>+#ifdef CONFIG_BEANCOUNTERS > >>+extern struct hlist_head bc_hash[]; > >>+extern spinlock_t bc_hash_lock; > > > > > > I wonder who pokes into that hash from other files? Looks a bit dangerous. > it was kernel/ub/proc.c with proc interface :) > however, we removed it from this patchset version, but forgot extern's... > > will remove Best remove the EXPORT_SYMBOLs too and make it static. > >>+void __put_beancounter(struct beancounter *bc); > >>+static inline void put_beancounter(struct beancounter *bc) > >>+{ > >>+ __put_beancounter(bc); > >>+} > > > > > > The wrapper seems pointless too. > yep, almost the same reason :) > > > The file could use a overview comment what the various counter > > types actually are. > you mean comment about what resource parameters we introduce? I meant about what a barrier counter etc. is and what makes it different from other counters. The individual resources can be probably described elswhere. -Andi
creator: Andi Kleen
date: 2006-08-23T13:48:05-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: > >>+void __put_beancounter(struct beancounter *bc) > >>+{ > >>+ unsigned long flags; > >>+ > >>+ /* equivalent to atomic_dec_and_lock_irqsave() */ > >>+ local_irq_save(flags); > >>+ if (likely(!atomic_dec_and_lock(&bc->bc_refcount, &bc_hash_lock))) { > >>+ local_irq_restore(flags); > >>+ if (unlikely(atomic_read(&bc->bc_refcount) < 0)) > >>+ printk(KERN_ERR "BC: Bad refcount: bc=%p, " > >>+ "luid=%d, ref=%d\n", > >>+ bc, bc->bc_id, > >>+ atomic_read(&bc->bc_refcount)); > > > > > >Should this BUG_ON() ? > BUG_ON doesn't print much information :) > ok, will replace but printk + BUG does ;-)
creator: Alexey Dobriyan
date: 2006-08-23T13:53:11-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: On Wed, 23 Aug 2006 15:03:07 +0400 Kirill Korotaev <dev@sw.ru> wrote: > Core functionality and interfaces of BC: > find/create beancounter, initialization, > charge/uncharge of resource, core objects' declarations. > > Basic structures: > bc_resource_parm - resource description > beancounter - set of resources, id, lock > > > .. > > +enum severity { BC_BARRIER, BC_LIMIT, BC_FORCE }; That's a bit generic-sounding. Make it bc_severity? > +static inline void bc_adjust_held_minmax(struct beancounter *bc, > + int resource) > +{ > + if (bc->bc_parms[resource].maxheld < bc->bc_parms[resource].held) > + bc->bc_parms[resource].maxheld = bc->bc_parms[resource].held; > + if (bc->bc_parms[resource].minheld > bc->bc_parms[resource].held) > + bc->bc_parms[resource].minheld = bc->bc_parms[resource].held; > +} That might be a bit big to inline. Suggest you check that the compiler successfully CSE's the `bc->bc_parms[resource]' evaulation. If not, create a temporary. > +#define beancounter_findcreate(id, f) (NULL) > +#define get_beancounter(bc) (NULL) > +#define put_beancounter(bc) do { } while (0) > +#define bc_charge_locked(bc, r, v, s) (0) > +#define bc_charge(bc, r, v) (0) akpm:/home/akpm> cat t.c void foo(void) { (0); } akpm:/home/akpm> gcc -c -Wall t.c t.c: In function 'foo': t.c:4: warning: statement with no effect > +#define bc_hash_fun(x) ((((x) >> 8) ^ (x)) & (BC_HASH_SIZE - 1)) Use hash_long()? > +/* > + * Per resource beancounting. Resources are tied to their luid. > + * The resource structure itself is tagged both to the process and > + * the charging resources (a socket doesn't want to have to search for > + * things at irq time for example). Reference counters keep things in > + * hand. > + * > + * The case where a user creates resource, kills all his processes and > + * then starts new ones is correctly handled this way. The refcounters > + * will mean the old entry is still around with resource tied to it. > + */ > + > +struct beancounter *beancounter_findcreate(uid…
creator: Andrew Morton
date: 2006-08-23T16:42:02-00:00

Re: [PATCH 4/6] BC: user interface (syscalls)
type: item
description: On Wed, 23 Aug 2006 17:43:16 +0400 Kirill Korotaev <dev@sw.ru> wrote: > +asmlinkage long sys_set_bclimit(uid_t id, unsigned long resource, > + unsigned long *limits) I'm still a bit mystified about the use of uid_t here. It's not a uid, is it?
creator: Andrew Morton
date: 2006-08-23T16:50:31-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: On Wed, 23 Aug 2006 14:46:19 +0400 Kirill Korotaev <dev@sw.ru> wrote: > The following patch set presents base of > Resource Beancounters (BC). > BC allows to account and control consumption > of kernel resources used by group of processes. > > Draft UBC description on OpenVZ wiki can be found at > http://wiki.openvz.org/UBC_parameters > > The full BC patch set allows to control: > - kernel memory. All the kernel objects allocatable > on user demand should be accounted and limited > for DoS protection. > E.g. page tables, task structs, vmas etc. > > - virtual memory pages. BCs allow to > limit a container to some amount of memory and > introduces 2-level OOM killer taking into account > container's consumption. > pages shared between containers are correctly > charged as fractions (tunable). > > - network buffers. These includes TCP/IP rcv/snd > buffers, dgram snd buffers, unix, netlinks and > other buffers. > > - minor resources accounted/limited by number: > tasks, files, flocks, ptys, siginfo, pinned dcache > mem, sockets, iptentries (for containers with > virtualized networking) > > As the first step we want to propose for discussion > the most complicated parts of resource management: > kernel memory and virtual memory. The patches look reasonable to me - mergeable after updating them for today's batch of review commentlets. I have two high-level problems though. a) I don't yet have a sense of whether this implementation is appropriate/sufficient for the various other applications which people are working on. If the general shape is OK and we think this implementation can be grown into one which everyone can use then fine. And... > The patch set to be sent provides core for BC and > management of kernel memory only. Virtual memory > management will be sent in a couple of days. We need to go over this work before we can commit to the BC core. Last time I looked at the VM accounting patch it seemed rather unpleasing from a maintainability POV. And, if I understand…
creator: Andrew Morton
date: 2006-08-23T17:05:32-00:00

Re: [PATCH 4/6] BC: user interface (syscalls)
type: item
description: Ar Mer, 2006-08-23 am 09:50 -0700, ysgrifennodd Andrew Morton: > On Wed, 23 Aug 2006 17:43:16 +0400 > Kirill Korotaev <dev@sw.ru> wrote: > > > +asmlinkage long sys_set_bclimit(uid_t id, unsigned long resource, > > + unsigned long *limits) > > I'm still a bit mystified about the use of uid_t here. It's not a uid, is > it? Its a uid_t because of setluid() and twenty odd years of existing unix practice. Alan
creator: Alan Cox
date: 2006-08-23T17:08:28-00:00

Re: [PATCH 6/6] BC: kernel memory accounting (marks)
type: item
description: I'm still a bit concerned about if we actually need the 'struct page' pointer. I've gone through all of the users, and I'm not sure that I see any that _require_ having a pointer in 'struct page'. I think it will take some rework, especially with the pagetables, but it should be quite doable. vmalloc: Store in vm_struct fd_set_bits: poll_get: mount hashtable: Don't need alignment. use the slab? pagetables: either store in an extra field of 'struct page', or use the mm's. mm should always be available when alloc/freeing a pagetable page Did I miss any? -- Dave
creator: Dave Hansen
date: 2006-08-23T18:30:20-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: Kirill Korotaev wrote: > > [ ... ] > > Patch set is applicable to 2.6.18-rc4-mm2 Applying patch 1_6_BC_kconfig.patch patching file arch/i386/Kconfig Hunk #1 succeeded at 1184 with fuzz 2 (offset 38 lines). patching file arch/ia64/Kconfig Hunk #1 succeeded at 505 with fuzz 2 (offset 24 lines). patching file arch/i386/Kconfig Hunk #1 succeeded at 1186 with fuzz 2 (offset 40 lines). patching file arch/ia64/Kconfig Hunk #1 succeeded at 507 with fuzz 2 (offset 26 lines). patching file arch/powerpc/Kconfig Hunk #1 succeeded at 1040 with fuzz 2 (offset 2 lines). patching file arch/ppc/Kconfig Hunk #1 succeeded at 1416 with fuzz 2 (offset 2 lines). patching file arch/sparc/Kconfig Hunk #1 FAILED at 296. 1 out of 1 hunk FAILED -- rejects in file arch/sparc/Kconfig patching file arch/sparc64/Kconfig Hunk #1 FAILED at 432. 1 out of 1 hunk FAILED -- rejects in file arch/sparc64/Kconfig patching file arch/x86_64/Kconfig Hunk #1 FAILED at 655. 1 out of 1 hunk FAILED -- rejects in file arch/x86_64/Kconfig patching file kernel/bc/Kconfig patching file arch/powerpc/Kconfig Hunk #1 succeeded at 1042 with fuzz 2 (offset 4 lines). patching file arch/ppc/Kconfig Hunk #1 succeeded at 1418 with fuzz 2 (offset 4 lines). patching file arch/sparc/Kconfig Hunk #1 FAILED at 296. 1 out of 1 hunk FAILED -- rejects in file arch/sparc/Kconfig patching file arch/sparc64/Kconfig Hunk #1 FAILED at 432. 1 out of 1 hunk FAILED -- rejects in file arch/sparc64/Kconfig patching file arch/x86_64/Kconfig Hunk #1 FAILED at 655. 1 out of 1 hunk FAILED -- rejects in file arch/x86_64/Kconfig patching file kernel/bc/Kconfig Patch 1_6_BC_kconfig.patch does not apply (enforce with -f) C.
creator: Cedric Le Goater
date: 2006-08-23T21:00:31-00:00

Re: [PATCH 1/6] BC: kconfig
type: item
description: On Wed, 2006-08-23 at 15:01 +0400, Kirill Korotaev wrote: > --- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 > +++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 > @@ -432,3 +432,5 @@ source "security/Kconfig" > source "crypto/Kconfig" > > source "lib/Kconfig" > + > +source "kernel/bc/Kconfig" ... > --- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 > +++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 > @@ -432,3 +432,5 @@ source "security/Kconfig" > source "crypto/Kconfig" > > source "lib/Kconfig" > + > +source "kernel/bc/Kconfig" Is it just me, or do these patches look a little funky? Looks like it is trying to patch the same thing into the same file, twice. Also, the patches look to be -p0 instead of -p1. I'm having a few problems applying them. -- Dave
creator: Dave Hansen
date: 2006-08-23T22:04:58-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: On Wed, 2006-08-23 at 17:49 +0400, Kirill Korotaev wrote: > Alexey Dobriyan wrote: > > On Wed, Aug 23, 2006 at 03:03:07PM +0400, Kirill Korotaev wrote: > > > > > >>--- /dev/null > >>+++ ./include/bc/beancounter.h > > > > > >>+#define BC_RESOURCES 0 > > > > > > Do you want userspace to see it? > yep. > > >>+struct bc_resource_parm { > >>+ unsigned long barrier; /* A barrier over which resource allocations > >>+ * are failed gracefully. e.g. if the amount > >>+ * of consumed memory is over the barrier > >>+ * further sbrk() or mmap() calls fail, the > >>+ * existing processes are not killed. > >>+ */ > >>+ unsigned long limit; /* hard resource limit */ > >>+ unsigned long held; /* consumed resources */ > >>+ unsigned long maxheld; /* maximum amount of consumed resources */ > >>+ unsigned long minheld; /* minumum amount of consumed resources */ > > > > > > Stupid question: when minimum amount is useful? > to monitor usage statistics (range of used resources). > this value will be usefull when ubstat will be added. > this field probably would be more logical to add later, > but since it is part of user space interface it is left here > for not changing API later. Then I think it belongs in a separate patch. Add it and the scattered bits and pieces that use it with the same patch. Then folks can clearly see what it's for, where it impacts the code, and how it works. Yes, factoring it out causes the API to evolve over the course of applying the patch series -- IMHO that evolution is useful information to convey to reviewers too. Cheers, -Matt Helsley
creator: Matt Helsley
date: 2006-08-23T22:05:55-00:00

Re: [PATCH 1/6] BC: kconfig
type: item
description: On Wed, 2006-08-23 at 15:04 -0700, Dave Hansen wrote: > On Wed, 2006-08-23 at 15:01 +0400, Kirill Korotaev wrote: > > --- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 > > +++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 > > @@ -432,3 +432,5 @@ source "security/Kconfig" > > source "crypto/Kconfig" > > > > source "lib/Kconfig" > > + > > +source "kernel/bc/Kconfig" > ... > > --- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 > > +++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 > > @@ -432,3 +432,5 @@ source "security/Kconfig" > > source "crypto/Kconfig" > > > > source "lib/Kconfig" > > + > > +source "kernel/bc/Kconfig" > > Is it just me, or do these patches look a little funky? Looks like it > is trying to patch the same thing into the same file, twice. Also, the > patches look to be -p0 instead of -p1. They do appear to be -p0 They aren't adding the same thing twice to the same file. This patch makes different arches source the same Kconfig. I seem to recall Chandra suggested that instead of doing it this way it would be more appropriate to add the source line to init/Kconfig because it's more central and arch-independent. I tend to agree. Cheers, -Matt Helsley
creator: Matt Helsley
date: 2006-08-23T22:13:42-00:00

Re: [PATCH 1/6] BC: kconfig
type: item
description: On Wed, 2006-08-23 at 15:32 -0700, Randy.Dunlap wrote: > On Wed, 23 Aug 2006 15:13:42 -0700 Matt Helsley wrote: > > > On Wed, 2006-08-23 at 15:04 -0700, Dave Hansen wrote: > > > On Wed, 2006-08-23 at 15:01 +0400, Kirill Korotaev wrote: > > > > --- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 > > > > +++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 > > > > @@ -432,3 +432,5 @@ source "security/Kconfig" > > > > source "crypto/Kconfig" > > > > > > > > source "lib/Kconfig" > > > > + > > > > +source "kernel/bc/Kconfig" > > > ... > > > > --- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 > > > > +++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 > > > > @@ -432,3 +432,5 @@ source "security/Kconfig" > > > > source "crypto/Kconfig" > > > > > > > > source "lib/Kconfig" > > > > + > > > > +source "kernel/bc/Kconfig" > > > > > > Is it just me, or do these patches look a little funky? Looks like it > > > is trying to patch the same thing into the same file, twice. Also, the > > > patches look to be -p0 instead of -p1. > > > > They do appear to be -p0 > > > > They aren't adding the same thing twice to the same file. This patch > > makes different arches source the same Kconfig. > > Look again. There are 2 diffstat blocks and 2 of these at least: > > --- ./kernel/bc/Kconfig.bckm 2006-07-28 13:07:38.000000000 +0400 > +++ ./kernel/bc/Kconfig 2006-07-28 13:09:51.000000000 +0400 > @@ -0,0 +1,25 @@ > > > > I seem to recall Chandra suggested that instead of doing it this way it > > would be more appropriate to add the source line to init/Kconfig because > > it's more central and arch-independent. I tend to agree. > > --- > ~Randy My mistake, Dave is right. Cheers, -Matt Helsley
creator: Matt Helsley
date: 2006-08-23T22:27:59-00:00

Re: [PATCH 1/6] BC: kconfig
type: item
description: On Wed, 23 Aug 2006 15:13:42 -0700 Matt Helsley wrote: > On Wed, 2006-08-23 at 15:04 -0700, Dave Hansen wrote: > > On Wed, 2006-08-23 at 15:01 +0400, Kirill Korotaev wrote: > > > --- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 > > > +++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 > > > @@ -432,3 +432,5 @@ source "security/Kconfig" > > > source "crypto/Kconfig" > > > > > > source "lib/Kconfig" > > > + > > > +source "kernel/bc/Kconfig" > > ... > > > --- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 > > > +++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 > > > @@ -432,3 +432,5 @@ source "security/Kconfig" > > > source "crypto/Kconfig" > > > > > > source "lib/Kconfig" > > > + > > > +source "kernel/bc/Kconfig" > > > > Is it just me, or do these patches look a little funky? Looks like it > > is trying to patch the same thing into the same file, twice. Also, the > > patches look to be -p0 instead of -p1. > > They do appear to be -p0 > > They aren't adding the same thing twice to the same file. This patch > makes different arches source the same Kconfig. Look again. There are 2 diffstat blocks and 2 of these at least: --- ./kernel/bc/Kconfig.bckm 2006-07-28 13:07:38.000000000 +0400 +++ ./kernel/bc/Kconfig 2006-07-28 13:09:51.000000000 +0400 @@ -0,0 +1,25 @@ > I seem to recall Chandra suggested that instead of doing it this way it > would be more appropriate to add the source line to init/Kconfig because > it's more central and arch-independent. I tend to agree. --- ~Randy
creator: rdunlap
date: 2006-08-23T22:29:09-00:00

Re: [PATCH 6/6] BC: kernel memory accounting (marks)
type: item
description: On Wed, 2006-08-23 at 15:08 +0400, Kirill Korotaev wrote: > include/asm-i386/thread_info.h | 4 ++-- > include/asm-ia64/pgalloc.h | 24 +++++++++++++++++------- > include/asm-x86_64/pgalloc.h | 12 ++++++++---- > include/asm-x86_64/thread_info.h | 5 +++-- Do you think we need to cover a few more architectures before considering merging this, or should we just fix them up as we need them? I'm working on a patch to unify as many of the alloc_thread_info() functions as I can. That should at least give you one place to modify and track the thread_info allocations. I've only compiled for x86_64 and i386, but I'm working on more. A preliminary version is attached. -- Dave --- clean-dave/include/asm-alpha/thread_info.h | 5 --- clean-dave/include/asm-frv/thread_info.h | 17 ---------- clean-dave/include/asm-h8300/thread_info.h | 8 +---- clean-dave/include/asm-i386/thread_info.h | 18 ----------- clean-dave/include/asm-m32r/thread_info.h | 17 ---------- clean-dave/include/asm-m68k/thread_info.h | 9 ----- clean-dave/include/asm-powerpc/thread_info.h | 27 ----------------- clean-dave/include/linux/thread_alloc.h | 42 +++++++++++++++++++++++++++ clean-dave/kernel/fork.c | 1 9 files changed, 47 insertions(+), 97 deletions(-) diff -puN arch/arm/kernel/process.c~unify-alloc-thread-info arch/arm/kernel/process.c diff -puN include/asm-alpha/thread_info.h~unify-alloc-thread-info include/asm-alpha/thread_info.h --- clean/include/asm-alpha/thread_info.h~unify-alloc-thread-inf o 2006-08-23 15:38:37.000000000 -0700 +++ clean-dave/include/asm-alpha/thread_info.h 2006-08-23 15:40:23.000000000 -0700 @@ -50,10 +50,7 @@ register struct thread_info *__current_t #define current_thread_info() __current_thread_info /* Thread information allocation. */ -#define THREAD_SIZE (2*PAGE_SIZE) -#define alloc_thread_info(tsk) \ - ((struct thread_info *) __get_free_pages(GFP_KERNEL,1)) -#define free_thread_info(ti) free_pages((unsigned long) (ti), 1) +#define THREAD_SHIFT (PAGE_SHIFT+1) #endif /* __ASSEMBLY__ *…
creator: Dave Hansen
date: 2006-08-23T23:03:51-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: On Wed, 2006-08-23 at 10:05 -0700, Andrew Morton wrote: > On Wed, 23 Aug 2006 14:46:19 +0400 > Kirill Korotaev <dev@sw.ru> wrote: > > > The following patch set presents base of > > Resource Beancounters (BC). > > BC allows to account and control consumption > > of kernel resources used by group of processes. > > > > Draft UBC description on OpenVZ wiki can be found at > > http://wiki.openvz.org/UBC_parameters > > > > The full BC patch set allows to control: > > - kernel memory. All the kernel objects allocatable > > on user demand should be accounted and limited > > for DoS protection. > > E.g. page tables, task structs, vmas etc. > > > > - virtual memory pages. BCs allow to > > limit a container to some amount of memory and > > introduces 2-level OOM killer taking into account > > container's consumption. > > pages shared between containers are correctly > > charged as fractions (tunable). > > > > - network buffers. These includes TCP/IP rcv/snd > > buffers, dgram snd buffers, unix, netlinks and > > other buffers. > > > > - minor resources accounted/limited by number: > > tasks, files, flocks, ptys, siginfo, pinned dcache > > mem, sockets, iptentries (for containers with > > virtualized networking) > > > > As the first step we want to propose for discussion > > the most complicated parts of resource management: > > kernel memory and virtual memory. > > The patches look reasonable to me - mergeable after updating them for > today's batch of review commentlets. If you are considering this infrastructure for generic resource management, I have few concerns: - There is no CPU controller under this framework - There is no I/O controller under this framework - Minimum of 3 parameters need to be used to manage memory. (in other words, usage is not simple. In order to provide a minimum guarantee of a resource, one needs to define a new parameter) > > I have two high-level problems though. > > a) I don't yet have a sense of whether this implementation > is appropriate/suffi…
creator: Chandra Seetharaman
date: 2006-08-24T00:17:39-00:00

Re: [PATCH 1/6] BC: kconfig
type: item
description: Is there a reason why these can be moved to a arch-neutral place ? PS: Please keep ckrm-tech on Cc: please. On Wed, 2006-08-23 at 15:01 +0400, Kirill Korotaev wrote: > Add kernel/bc/Kconfig file with BC options and > include it into arch Kconfigs > > Signed-off-by: Pavel Emelianov <xemul@sw.ru> > Signed-off-by: Kirill Korotaev <dev@sw.ru> > > --- > > arch/i386/Kconfig | 2 ++ > arch/ia64/Kconfig | 2 ++ > arch/powerpc/Kconfig | 2 ++ > arch/ppc/Kconfig | 2 ++ > arch/sparc/Kconfig | 2 ++ > arch/sparc64/Kconfig | 2 ++ > arch/x86_64/Kconfig | 2 ++ > kernel/bc/Kconfig | 25 +++++++++++++++++++++++++ > 8 files changed, 39 insertions(+) > > --- ./arch/i386/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 > +++ ./arch/i386/Kconfig 2006-07-28 14:10:41.000000000 +0400 > @@ -1146,6 +1146,8 @@ source "crypto/Kconfig" > > source "lib/Kconfig" > > +source "kernel/bc/Kconfig" > + > # > # Use the generic interrupt handling code in kernel/irq/: > # > --- ./arch/ia64/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 > +++ ./arch/ia64/Kconfig 2006-07-28 14:10:56.000000000 +0400 > @@ -481,6 +481,8 @@ source "fs/Kconfig" > > source "lib/Kconfig" > > +source "kernel/bc/Kconfig" > + > # > # Use the generic interrupt handling code in kernel/irq/: > #Add kernel/bc/Kconfig file with BC options and > include it into arch Kconfigs > > Signed-off-by: Pavel Emelianov <xemul@sw.ru> > Signed-off-by: Kirill Korotaev <dev@sw.ru> > > --- > > arch/i386/Kconfig | 2 ++ > arch/ia64/Kconfig | 2 ++ > arch/powerpc/Kconfig | 2 ++ > arch/ppc/Kconfig | 2 ++ > arch/sparc/Kconfig | 2 ++ > arch/sparc64/Kconfig | 2 ++ > arch/x86_64/Kconfig | 2 ++ > kernel/bc/Kconfig | 25 +++++++++++++++++++++++++ > 8 files changed, 39 insertions(+) > > --- ./arch/i386/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 > +++ ./arch/i386/Kconfig 2006-07-28 14:10:41.000000000 +0400 > @@ -1146,6 +1146,8 @@ source "crypto/Kconfig" > > source "lib/Kconfig" > > +source "kernel/bc/Kconfig" > + > # > # Use the generic interrupt handling code in …
creator: Chandra Seetharaman
date: 2006-08-24T00:23:48-00:00

Re: [PATCH 4/6] BC: user interface (syscalls)
type: item
description: On Wed, 2006-08-23 at 15:06 +0400, Kirill Korotaev wrote: <snip> > +asmlinkage long sys_set_bclimit(uid_t id, unsigned long resource, > + unsigned long *limits) > +{ > + int error; > + unsigned long flags; > + struct beancounter *bc; > + unsigned long new_limits[2]; > + > + error = -EPERM; > + if(!capable(CAP_SYS_RESOURCE)) > + goto out; > + > + error = -EINVAL; > + if (resource >= BC_RESOURCES) > + goto out; > + > + error = -EFAULT; > + if (copy_from_user(&new_limits, limits, sizeof(new_limits))) > + goto out; > + > + error = -EINVAL; > + if (new_limits[0] > BC_MAXVALUE || new_limits[1] > BC_MAXVALUE) > + goto out; > + > + error = -ENOENT; > + bc = beancounter_findcreate(id, 0); > + if (bc == NULL) > + goto out; > + > + spin_lock_irqsave(&bc->bc_lock, flags); > + bc->bc_parms[resource].barrier = new_limits[0]; > + bc->bc_parms[resource].limit = new_limits[1]; No check for barrier <= limit > + spin_unlock_irqrestore(&bc->bc_lock, flags); > + > + put_beancounter(bc); > + error = 0; > +out: > + return error; > +} <snip> -- ------------------------------------------------------------ ---------- Chandra Seetharaman | Be careful what you choose.... - sekharan@us.ibm.com | .......you may get it. ------------------------------------------------------------ ----------
creator: Chandra Seetharaman
date: 2006-08-24T00:30:56-00:00

Re: [PATCH 5/6] BC: kernel memory accounting (core)
type: item
description: On Wed, 2006-08-23 at 15:06 +0400, Kirill Korotaev wrote: <snip> > --- ./include/bc/beancounter.h.bckmem 2006-07-28 18:43:52.000000000 +0400 > +++ ./include/bc/beancounter.h 2006-08-03 16:03:01.000000000 +0400 > @@ -14,7 +14,9 @@ > * Resource list. > */ > > -#define BC_RESOURCES 0 > +#define BC_KMEMSIZE 0 > + > +#define BC_RESOURCES 1 As suggested before, a clean interface to define these would be better > > struct resource_parm { > /* > --- ./include/bc/kmem.h.bckmem 2006-07-28 18:43:52.000000000 +0400 > +++ ./include/bc/kmem.h 2006-07-31 17:37:05.000000000 +0400 > @@ -0,0 +1,33 @@ > +/* > + * include/bc/kmem.h > + * > + * Copyright (C) 2006 OpenVZ. SWsoft Inc > + * > + */ > + > +#ifndef __BC_KMEM_H_ > +#define __BC_KMEM_H_ > + > +#include <linux/config.h> > + > +/* > + * BC_KMEMSIZE accounting > + */ > + > +struct mm_struct; > +struct page; > +struct beancounter; > + > +#ifdef CONFIG_BEANCOUNTERS > +int bc_page_charge(struct page *page, int order, gfp_t flags); > +void bc_page_uncharge(struct page *page, int order); > + > +int bc_slab_charge(kmem_cache_t *cachep, void *obj, gfp_t flags); > +void bc_slab_uncharge(kmem_cache_t *cachep, void *obj); > +#else > +#define bc_page_charge(pg, o, mask) (0) > +#define bc_page_uncharge(pg, o) do { } while (0) > +#define bc_slab_charge(cachep, o, f) (0) > +#define bc_slab_uncharge(cachep, o) do { } while (0) > +#endif > +#endif /* __BC_SLAB_H_ */ > --- ./kernel/bc/Makefile.bcsys 2006-07-28 14:08:37.000000000 +0400 > +++ ./kernel/bc/Makefile 2006-08-01 11:08:39.000000000 +0400 > @@ -7,3 +7,4 @@ > obj-$(CONFIG_BEANCOUNTERS) += beancounter.o > obj-$(CONFIG_BEANCOUNTERS) += misc.o > obj-$(CONFIG_BEANCOUNTERS) += sys.o > +obj-$(CONFIG_BEANCOUNTERS) += kmem.o > --- ./kernel/bc/beancounter.c.bckmem 2006-07-28 18:43:52.000000000 +0400 > +++ ./kernel/bc/beancounter.c 2006-08-03 16:14:17.000000000 +0400 > @@ -19,6 +19,7 @@ static void init_beancounter_struct(stru > struct beancounter init_bc; > > const char *bc_rnames[] = { > + "kmemsiz…
creator: Chandra Seetharaman
date: 2006-08-24T00:36:28-00:00

Re: [PATCH 4/6] BC: user interface (syscalls)
type: item
description: On Wed, 23 Aug 2006 18:29:42 +0100 Alan Cox <alan@lxorguk.ukuu.org.uk> wrote: > Ar Mer, 2006-08-23 am 09:50 -0700, ysgrifennodd Andrew Morton: > > On Wed, 23 Aug 2006 17:43:16 +0400 > > Kirill Korotaev <dev@sw.ru> wrote: > > > > > +asmlinkage long sys_set_bclimit(uid_t id, unsigned long resource, > > > + unsigned long *limits) > > > > I'm still a bit mystified about the use of uid_t here. It's not a uid, is > > it? > > Its a uid_t because of setluid() and twenty odd years of existing unix > practice. > I don't understand. This number is an identifier for an accounting container, which was somehow dreamed up by userspace. AFAICT it is wholly unrelated to user ID's. (How does userspace avoid collisions, btw?)
creator: Andrew Morton
date: 2006-08-24T04:35:12-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: > > The following patch set presents base of > Resource Beancounters (BC). Why are they called beans? Jan Engelhardt --
creator: Jan Engelhardt
date: 2006-08-24T05:52:36-00:00

Re: [PATCH 6/6] BC: kernel memory accounting (marks)
type: item
description: On Wed, 23 Aug 2006, Dave Hansen wrote: > I'm working on a patch to unify as many of the alloc_thread_info() > functions as I can. That should at least give you one place to modify > and track the thread_info allocations. I've only compiled for x86_64 > and i386, but I'm working on more. A preliminary version is attached. > --- clean/include/asm-m68k/thread_info.h~unify-alloc-thread-info 2006-08-23 15:44:52.000000000 -0700 > +++ clean-dave/include/asm-m68k/thread_info.h 2006-08-23 15:45:32.000000000 -0700 > @@ -24,14 +24,7 @@ struct thread_info { > }, \ > } > > -/* THREAD_SIZE should be 8k, so handle differently for 4k and 8k machines */ > -#if PAGE_SHIFT == 13 /* 8k machines */ > -#define alloc_thread_info(tsk) ((struct thread_info *)__get_free_pages(GFP_KERNEL,0)) > -#define free_thread_info(ti) free_pages((unsigned long)(ti),0) > -#else /* otherwise assume 4k pages */ > -#define alloc_thread_info(tsk) ((struct thread_info *)__get_free_pages(GFP_KERNEL,1)) > -#define free_thread_info(ti) free_pages((unsigned long)(ti),1) > -#endif /* PAGE_SHIFT == 13 */ > +#define THREAD_SHIFT 1 ^ Shouldn't this be 13? > --- /dev/null 2005-03-30 22:36:15.000000000 -0800 > +++ clean-dave/include/linux/thread_alloc.h 2006-08-23 16:00:41.000000000 -0700 > @@ -0,0 +1,42 @@ > +#ifndef _LINUX_THREAD_ALLOC > +#define _LINUX_THREAD_ALLOC > + > +#ifndef THREAD_SHIFT > +#define THREAD_SHIFT PAGE_SHIFT > +#endif > +#ifndef THREAD_ORDER > +#define THREAD_ORDER (THREAD_SHIFT - PAGE_SHIFT) > +#endif > + > +struct thread_info; > +struct task; > + > +#if THREAD_SHIFT >= PAGE_SHIFT > +static inline struct thread_info *alloc_thread_info(struct task_struct *tsk) > +{ > + gfp_t flags = GFP_KERNEL; > +#ifdef CONFIG_DEBUG_STACK_USAGE > + flags |= __GFP_ZERO; > +#endif > + return (struct thread_info *)__get_free_pages(flags, THREAD_ORDER); > +} > +static inline void free_thread_info(struct thread_info *ti) > +{ > + free_pages((unsigned long)ti, THREAD_ORDER); > +} > +#else /* THREAD_SHIFT < PAGE_SHIFT *…
creator: Geert Uytterhoeven
date: 2006-08-24T09:30:09-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: Ar Iau, 2006-08-24 am 07:52 +0200, ysgrifennodd Jan Engelhardt: > > > > The following patch set presents base of > > Resource Beancounters (BC). > > Why are they called beans? "Beancounter" is a slang English language name for an accountant. Alan
creator: Alan Cox
date: 2006-08-24T10:38:07-00:00

Re: [PATCH 4/6] BC: user interface (syscalls)
type: item
description: Ar Mer, 2006-08-23 am 21:35 -0700, ysgrifennodd Andrew Morton: > > Its a uid_t because of setluid() and twenty odd years of existing unix > > practice. > > > > I don't understand. This number is an identifier for an accounting > container, which was somehow dreamed up by userspace. Which happens to be a uid_t. It could easily be anyother_t of itself and you can create a container_id_t or whatever. It is just a number. The ancient Unix implementations of this kind of resource management and security are built around setluid() which sets a uid value that cannot be changed again and is normally used for security purposes. That happened to be a uid_t and in simple setups at login uid = luid = euid would be the norm. Thus the Linux one happens to be a uid_t. It could be something else but for the "container per user" model whatever a container is must be able to hold all possible uid_t values. So we can certainly do something like typedef uid_t container_id_t; Alan
creator: Alan Cox
date: 2006-08-24T10:42:55-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: On 08/23, Andrew Morton wrote: > > On Wed, 23 Aug 2006 15:03:07 +0400 > Kirill Korotaev <dev@sw.ru> wrote: > > > +void __put_beancounter(struct beancounter *bc) > > +{ > > + unsigned long flags; > > + > > + /* equivalent to atomic_dec_and_lock_irqsave() */ > > + local_irq_save(flags); > > + if (likely(!atomic_dec_and_lock(&bc->bc_refcount, &bc_hash_lock))) { > > + local_irq_restore(flags); > > + if (unlikely(atomic_read(&bc->bc_refcount) < 0)) > > + printk(KERN_ERR "BC: Bad refcount: bc=%p, " > > + "luid=%d, ref=%d\n", > > + bc, bc->bc_id, > > + atomic_read(&bc->bc_refcount)); > > + return; > > + } > > + > > + BUG_ON(bc == &init_bc); > > + verify_held(bc); > > + hlist_del(&bc->hash); > > + spin_unlock_irqrestore(&bc_hash_lock, flags); > > + kmem_cache_free(bc_cachep, bc); > > +} > > I wonder if it's safe and worthwhile to optimise away the local_irq_save(): Suppose ->bc_refcount == 1 > if (atomic_dec_and_test(&bc->bc_refcount)) { Yes, preempted or blocks on spin_lock() below. another cpu locks bc_hash_lock, does get_beancounter() (beancounter_findcreate), then does put_beancounter(), and frees it. > spin_lock_irqsave(&bc_hash_lock, flags); > if (atomic_read(&bc->bc_refcount) == 0) { Yes, > free it > Already freed. Oleg.
creator: Oleg Nesterov
date: 2006-08-24T11:09:01-00:00

Re: [ckrm-tech] [PATCH 1/6] BC: kconfig
type: item
description: > Is there a reason why these can be moved to a arch-neutral place ? I think a good place for BC would be kernel/Kconfig.bc but still this should be added into archs. ok? > PS: Please keep ckrm-tech on Cc: please. Sorry, it is very hard to track emails coming from authors and 3 mailing lists. Better tell me the interested people emails. Thanks, Kirill > > On Wed, 2006-08-23 at 15:01 +0400, Kirill Korotaev wrote: > >>Add kernel/bc/Kconfig file with BC options and >>include it into arch Kconfigs >> >>Signed-off-by: Pavel Emelianov <xemul@sw.ru> >>Signed-off-by: Kirill Korotaev <dev@sw.ru> >> >>--- >> >> arch/i386/Kconfig | 2 ++ >> arch/ia64/Kconfig | 2 ++ >> arch/powerpc/Kconfig | 2 ++ >> arch/ppc/Kconfig | 2 ++ >> arch/sparc/Kconfig | 2 ++ >> arch/sparc64/Kconfig | 2 ++ >> arch/x86_64/Kconfig | 2 ++ >> kernel/bc/Kconfig | 25 +++++++++++++++++++++++++ >> 8 files changed, 39 insertions(+) >> >>--- ./arch/i386/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 >>+++ ./arch/i386/Kconfig 2006-07-28 14:10:41.000000000 +0400 >>@@ -1146,6 +1146,8 @@ source "crypto/Kconfig" >> >> source "lib/Kconfig" >> >>+source "kernel/bc/Kconfig" >>+ >> # >> # Use the generic interrupt handling code in kernel/irq/: >> # >>--- ./arch/ia64/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 >>+++ ./arch/ia64/Kconfig 2006-07-28 14:10:56.000000000 +0400 >>@@ -481,6 +481,8 @@ source "fs/Kconfig" >> >> source "lib/Kconfig" >> >>+source "kernel/bc/Kconfig" >>+ >> # >> # Use the generic interrupt handling code in kernel/irq/: >> #Add kernel/bc/Kconfig file with BC options and >>include it into arch Kconfigs >> >>Signed-off-by: Pavel Emelianov <xemul@sw.ru> >>Signed-off-by: Kirill Korotaev <dev@sw.ru> >> >>--- >> >> arch/i386/Kconfig | 2 ++ >> arch/ia64/Kconfig | 2 ++ >> arch/powerpc/Kconfig | 2 ++ >> arch/ppc/Kconfig | 2 ++ >> arch/sparc/Kconfig | 2 ++ >> arch/sparc64/Kconfig | 2 ++ >> arch/x86_64/Kconfig | 2 ++ >> kernel/bc/Kconfig | 25 +++++++++++++++++++++++++ >> 8 files changed, 39 insertions(+) >>…
creator: dev
date: 2006-08-24T11:47:07-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: Andrew Morton wrote: [... snip ...] >>+#define bc_charge_locked(bc, r, v, s) (0) >>> +#define bc_charge(bc, r, v) (0) > >akpm:/home/akpm> cat t.c >void foo(void) >{ > (0); >} >akpm:/home/akpm> gcc -c -Wall t.c >t.c: In function 'foo': >t.c:4: warning: statement with no effect these functions return value should always be checked (!). i.e. it is never called like: ub_charge(bc, r, v); >>+struct beancounter *beancounter_findcreate(uid_t uid, int mask) >>+{ >>+ struct beancounter *new_bc, *bc; >>+ unsigned long flags; >>+ struct hlist_head *slot; >>+ struct hlist_node *pos; >>+ >>+ slot = &bc_hash[bc_hash_fun(uid)]; >>+ new_bc = NULL; >>+ >>+retry: >>+ spin_lock_irqsave(&bc_hash_lock, flags); >>+ hlist_for_each_entry (bc, pos, slot, hash) >>+ if (bc->bc_id == uid) >>+ break; >>+ >>+ if (pos != NULL) { >>+ get_beancounter(bc); >>+ spin_unlock_irqrestore(&bc_hash_lock, flags); >>+ >>+ if (new_bc != NULL) >>+ kmem_cache_free(bc_cachep, new_bc); >>+ return bc; >>+ } >>+ >>+ if (!(mask & BC_ALLOC)) >>+ goto out_unlock; >>+ >>+ if (new_bc != NULL) >>+ goto out_install; >>+ >>+ spin_unlock_irqrestore(&bc_hash_lock, flags); >>+ >>+ new_bc = kmem_cache_alloc(bc_cachep, >>+ mask & BC_ALLOC_ATOMIC ? GFP_ATOMIC : GFP_KERNEL); >>+ if (new_bc == NULL) >>+ goto out; >>+ >>+ memcpy(new_bc, &default_beancounter, sizeof(*new_bc)); >>+ init_beancounter_struct(new_bc, uid); >>+ goto retry; >>+ >>+out_install: >>+ hlist_add_head(&new_bc->hash, slot); >>+out_unlock: >>+ spin_unlock_irqrestore(&bc_hash_lock, flags); >>+out: >>+ return new_bc; >>+} > > > Can remove the global bc_hash_lock and make the locking per-hash-bucket. it is not performance critical path IMHO. this lock is taken on container create/change/destroy/user interfaces only. >>+static inline void verify_held(struct beancounter *bc) >>+{ >>+ int i; >>+ >>+ for (i = 0; i < BC_RESOURCES; i++) >>+ if (bc->bc_parms[i].held != 0) >>+ bc_print_resource_warning(bc, i, >>+ "resource is held on put", 0, 0); >>+} >>+ >>+void __put_beanc…
creator: dev
date: 2006-08-24T12:03:38-00:00

Re: [PATCH 4/6] BC: user interface (syscalls)
type: item
description: On Thu, Aug 24, 2006 at 12:04:16PM +0100, Alan Cox wrote: > Ar Mer, 2006-08-23 am 21:35 -0700, ysgrifennodd Andrew Morton: > > > Its a uid_t because of setluid() and twenty odd years of existing unix > > > practice. > > > > > > > I don't understand. This number is an identifier for an accounting > > container, which was somehow dreamed up by userspace. > > Which happens to be a uid_t. It could easily be anyother_t of itself and > you can create a container_id_t or whatever. It is just a number. > > The ancient Unix implementations of this kind of resource management and > security are built around setluid() which sets a uid value that cannot > be changed again and is normally used for security purposes. That > happened to be a uid_t and in simple setups at login uid = luid = euid > would be the norm. > > Thus the Linux one happens to be a uid_t. It could be something else but > for the "container per user" model whatever a container is must be able > to hold all possible uid_t values. So we can certainly do something like > > typedef uid_t container_id_t; What about cid_t? Google mentions cid_t was used in HP-UX specific IPC (only if _INCLUDE_HPUX_SOURCE is defined).
creator: Alexey Dobriyan
date: 2006-08-24T13:08:22-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: On Thu, 24 Aug 2006 16:06:11 +0400 Kirill Korotaev <dev@sw.ru> wrote: > >>+#define bc_charge_locked(bc, r, v, s) (0) > >>> +#define bc_charge(bc, r, v) (0) > > > >akpm:/home/akpm> cat t.c > >void foo(void) > >{ > > (0); > >} > >akpm:/home/akpm> gcc -c -Wall t.c > >t.c: In function 'foo': > >t.c:4: warning: statement with no effect > > these functions return value should always be checked (!). We have __must_check for that. > i.e. it is never called like: > ub_charge(bc, r, v); Also... if (bc_charge(tpyo, undefined_variable, syntax_error)) will happily compile if !CONFIG_BEANCOUNTER. Turning these stubs into static inline __must_check functions fixes all this.
creator: Andrew Morton
date: 2006-08-24T15:00:30-00:00

Re: [PATCH 6/6] BC: kernel memory accounting (marks)
type: item
description: On Thu, 2006-08-24 at 11:30 +0200, Geert Uytterhoeven wrote: > > +#define THREAD_SHIFT 1 > ^ > Shouldn't this be 13? Yep. Thanks! I need to go over the whole things again and proofread. -- Dave
creator: Dave Hansen
date: 2006-08-24T15:52:22-00:00

Re: [PATCH 5/6] BC: kernel memory accounting (core)
type: item
description: On 08/23, Kirill Korotaev wrote: > > +int bc_slab_charge(kmem_cache_t *cachep, void *objp, gfp_t flags) > +{ > + unsigned int size; > + struct beancounter *bc, **slab_bcp; > + > + bc = get_exec_bc(); > + if (bc == NULL) > + return 0; Is it possible to .exec_bc == NULL ? If yes, why do we need init_bc? We can do 'set_exec_bc(NULL)' in __do_IRQ() instead. Oleg.
creator: Oleg Nesterov
date: 2006-08-24T16:59:39-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: On 08/23, Kirill Korotaev wrote: > > +struct beancounter *beancounter_findcreate(uid_t uid, int mask) > +{ > + struct beancounter *new_bc, *bc; > + unsigned long flags; > + struct hlist_head *slot; > + struct hlist_node *pos; > + > + slot = &bc_hash[bc_hash_fun(uid)]; > + new_bc = NULL; > + > +retry: > + spin_lock_irqsave(&bc_hash_lock, flags); > + hlist_for_each_entry (bc, pos, slot, hash) > + if (bc->bc_id == uid) > + break; > + > + if (pos != NULL) { > + get_beancounter(bc); > + spin_unlock_irqrestore(&bc_hash_lock, flags); > + > + if (new_bc != NULL) > + kmem_cache_free(bc_cachep, new_bc); > + return bc; > + } > + > + if (!(mask & BC_ALLOC)) > + goto out_unlock; Very minor nit: it is not clear why we are doing this check under bc_hash_lock. I'd suggest to do if (!(mask & BC_ALLOC)) goto out; after unlock(bc_hash_lock) and kill out_unlock label. > + if (new_bc != NULL) > + goto out_install; > + > + spin_unlock_irqrestore(&bc_hash_lock, flags); > + > + new_bc = kmem_cache_alloc(bc_cachep, > + mask & BC_ALLOC_ATOMIC ? GFP_ATOMIC : GFP_KERNEL); > + if (new_bc == NULL) > + goto out; > + > + memcpy(new_bc, &default_beancounter, sizeof(*new_bc)); May be it is just me, but I need a couple of seconds to parse this 'memcpy'. How about *new_bc = default_beancounter; ? Oleg.
creator: Oleg Nesterov
date: 2006-08-24T17:09:45-00:00

Re: [ckrm-tech] [PATCH 1/6] BC: kconfig
type: item
description: On Thu, 2006-08-24 at 15:47 +0400, Kirill Korotaev wrote: > > Is there a reason why these can be moved to a arch-neutral place ? > I think a good place for BC would be kernel/Kconfig.bc I think kernel/bc/Kconfig is fine. > but still this should be added into archs. > ok? Sourcing the bc Kconfig from arch Kconfigs would seem to suggest that resource management is only possible on a proper subset of archs. Since this is not the case wouldn't it be better to source the bc Kconfig from an arch-independent Kconfig (init/Kconfig for example)? > > PS: Please keep ckrm-tech on Cc: please. Also, thank you for CC'ing CKRM-Tech with your earlier posting of these patches. > Sorry, it is very hard to track emails coming from authors and 3 mailing lists. Yes, it can be difficult to keep track of all the email authors. > Better tell me the interested people emails. CC'ing only the known-interested people wouldn't be better. If anything I think it's harder for everyone than simply CC'ing a relevant mailing list like LKML and CKRM-Tech in this case. Cheers, -Matt Helsley
creator: Matt Helsley
date: 2006-08-24T22:23:31-00:00

Re: [PATCH 5/6] BC: kernel memory accounting (core)
type: item
description: Oleg Nesterov wrote: > On 08/23, Kirill Korotaev wrote: > >>+int bc_slab_charge(kmem_cache_t *cachep, void *objp, gfp_t flags) >>+{ >>+ unsigned int size; >>+ struct beancounter *bc, **slab_bcp; >>+ >>+ bc = get_exec_bc(); >>+ if (bc == NULL) >>+ return 0; > > > Is it possible to .exec_bc == NULL ? > > If yes, why do we need init_bc? We can do 'set_exec_bc(NULL)' in __do_IRQ() > instead. no, exec_bc can't be NULL. thanks for catching old hunks which historically exist due to old times when host system was not accounted (bc was NULL). Thanks, Kirill
creator: dev
date: 2006-08-25T10:06:45-00:00

Re: [PATCH 2/6] BC: beancounters core (API)
type: item
description: Andrew Morton wrote: > On Thu, 24 Aug 2006 16:06:11 +0400 > Kirill Korotaev <dev@sw.ru> wrote: > > >>>>+#define bc_charge_locked(bc, r, v, s) (0) >>>> >>>>>+#define bc_charge(bc, r, v) (0) >>> >>>akpm:/home/akpm> cat t.c >>>void foo(void) >>>{ >>> (0); >>>} >>>akpm:/home/akpm> gcc -c -Wall t.c >>>t.c: In function 'foo': >>>t.c:4: warning: statement with no effect >> >>these functions return value should always be checked (!). > > > We have __must_check for that. > > >>i.e. it is never called like: >> ub_charge(bc, r, v); > > > Also... > > if (bc_charge(tpyo, undefined_variable, syntax_error)) > > will happily compile if !CONFIG_BEANCOUNTER. > > Turning these stubs into static inline __must_check functions fixes all this. ok. will replace all empty stubs with inlines (with __must_check where appropriate) Thanks, Kirill
creator: dev
date: 2006-08-25T10:51:18-00:00

Re: [PATCH 4/6] BC: user interface (syscalls)
type: item
description: Alexey Dobriyan wrote: > On Thu, Aug 24, 2006 at 12:04:16PM +0100, Alan Cox wrote: > >>Ar Mer, 2006-08-23 am 21:35 -0700, ysgrifennodd Andrew Morton: >> >>>>Its a uid_t because of setluid() and twenty odd years of existing unix >>>>practice. >>>> >>> >>>I don't understand. This number is an identifier for an accounting >>>container, which was somehow dreamed up by userspace. >> >>Which happens to be a uid_t. It could easily be anyother_t of itself and >>you can create a container_id_t or whatever. It is just a number. >> >>The ancient Unix implementations of this kind of resource management and >>security are built around setluid() which sets a uid value that cannot >>be changed again and is normally used for security purposes. That >>happened to be a uid_t and in simple setups at login uid = luid = euid >>would be the norm. >> >>Thus the Linux one happens to be a uid_t. It could be something else but >>for the "container per user" model whatever a container is must be able >>to hold all possible uid_t values. So we can certainly do something like >> >>typedef uid_t container_id_t; > > > What about cid_t? Google mentions cid_t was used in HP-UX specific IPC (only if > _INCLUDE_HPUX_SOURCE is defined). bcid_t? Thanks, Kirill
creator: dev
date: 2006-08-25T10:54:18-00:00

Re: [PATCH 1/6] BC: kconfig
type: item
description: Matt Helsley wrote: > On Wed, 2006-08-23 at 15:04 -0700, Dave Hansen wrote: > >>On Wed, 2006-08-23 at 15:01 +0400, Kirill Korotaev wrote: >> >>>--- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 >>>+++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 >>>@@ -432,3 +432,5 @@ source "security/Kconfig" >>> source "crypto/Kconfig" >>> >>> source "lib/Kconfig" >>>+ >>>+source "kernel/bc/Kconfig" >> >>... >> >>>--- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 >>>+++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 >>>@@ -432,3 +432,5 @@ source "security/Kconfig" >>> source "crypto/Kconfig" >>> >>> source "lib/Kconfig" >>>+ >>>+source "kernel/bc/Kconfig" >> >>Is it just me, or do these patches look a little funky? Looks like it >>is trying to patch the same thing into the same file, twice. Also, the >>patches look to be -p0 instead of -p1. > > > They do appear to be -p0 it is -p1. patches are generated with gendiff and ./ in names is for -p1 > They aren't adding the same thing twice to the same file. This patch > makes different arches source the same Kconfig. > > I seem to recall Chandra suggested that instead of doing it this way it > would be more appropriate to add the source line to init/Kconfig because > it's more central and arch-independent. I tend to agree. agreed. init/Kconfig looks like a good place for including kernel/bc/Kconfig Kirill
creator: dev
date: 2006-08-25T11:27:40-00:00

Re: [PATCH 1/6] BC: kconfig
type: item
description: Dave Hansen wrote: > On Wed, 2006-08-23 at 15:01 +0400, Kirill Korotaev wrote: > >>--- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 >>+++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 >>@@ -432,3 +432,5 @@ source "security/Kconfig" >> source "crypto/Kconfig" >> >> source "lib/Kconfig" >>+ >>+source "kernel/bc/Kconfig" > > ... > >>--- ./arch/sparc64/Kconfig.arkcfg 2006-07-17 17:01:11.000000000 +0400 >>+++ ./arch/sparc64/Kconfig 2006-08-10 17:56:36.000000000 +0400 >>@@ -432,3 +432,5 @@ source "security/Kconfig" >> source "crypto/Kconfig" >> >> source "lib/Kconfig" >>+ >>+source "kernel/bc/Kconfig" > > > Is it just me, or do these patches look a little funky? Looks like it > is trying to patch the same thing into the same file, twice. Also, the > patches look to be -p0 instead of -p1. > > I'm having a few problems applying them. Oh, it's my fault. I pasted text twice :/ Kirill
creator: dev
date: 2006-08-25T11:31:28-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: Andrew Morton wrote: >>As the first step we want to propose for discussion >>the most complicated parts of resource management: >>kernel memory and virtual memory. > > > The patches look reasonable to me - mergeable after updating them for > today's batch of review commentlets. sure. will do updates as long as there are reasonable comments. > I have two high-level problems though. > > a) I don't yet have a sense of whether this implementation > is appropriate/sufficient for the various other > applications which people are working on. > > If the general shape is OK and we think this > implementation can be grown into one which everyone can > use then fine. > > And... > > >>The patch set to be sent provides core for BC and >>management of kernel memory only. Virtual memory >>management will be sent in a couple of days. > > > We need to go over this work before we can commit to the BC > core. Last time I looked at the VM accounting patch it > seemed rather unpleasing from a maintainability POV. hmmm... in which regard? > And, if I understand it correctly, the only response to a job > going over its VM limits is to kill it, rather than trimming > it. Which sounds like a big problem? No, UBC virtual memory management refuses occur on mmap()'s. Andrey Savochkin wrote already a brief summary on vm resource management: ------------- cut ---------------- The task of limiting a container to 4.5GB of memory bottles down to the question: what to do when the container starts to use more than assigned 4.5GB of memory? At this moment there are only 3 viable alternatives. A) Have separate memory management for each container, with separate buddy allocator, lru lists, page replacement mechanism. That implies a considerable overhead, and the main challenge there is sharing of pages between these separate memory managers. B) Return errors on extension of mappings, but not on page faults, where memory is actually consumed. In this case it makes sense to take into account not only the …
creator: dev
date: 2006-08-25T11:47:28-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: On Fri, 25 Aug 2006 15:49:15 +0400 Kirill Korotaev <dev@sw.ru> wrote: > > We need to go over this work before we can commit to the BC > > core. Last time I looked at the VM accounting patch it > > seemed rather unpleasing from a maintainability POV. > hmmm... in which regard? Little changes all over the MM code which might get accidentally broken. > > And, if I understand it correctly, the only response to a job > > going over its VM limits is to kill it, rather than trimming > > it. Which sounds like a big problem? > No, UBC virtual memory management refuses occur on mmap()'s. That's worse, isn't it? Firstly it rules out big sparse mappings and secondly mmap_and_use(80% of container size) fork_and_immediately_exec(/bin/true) will fail at the fork? > Andrey Savochkin wrote already a brief summary on vm resource management: > > ------------- cut ---------------- > The task of limiting a container to 4.5GB of memory bottles down to the > question: what to do when the container starts to use more than assigned > 4.5GB of memory? > > At this moment there are only 3 viable alternatives. > > A) Have separate memory management for each container, > with separate buddy allocator, lru lists, page replacement mechanism. > That implies a considerable overhead, and the main challenge there > is sharing of pages between these separate memory managers. > > B) Return errors on extension of mappings, but not on page faults, where > memory is actually consumed. > In this case it makes sense to take into account not only the size of used > memory, but the size of created mappings as well. > This is approximately what "privvmpages" accounting/limiting provides in > UBC. > > C) Rely on OOM killer. > This is a fall-back method in UBC, for the case "privvmpages" limits > still leave the possibility to overload the system. > D) Virtual scan of mm's in the over-limit container E) Modify existing physical scanner to be able to skip pages which belong to not-over-limit containers. F) Somethi…
creator: Andrew Morton
date: 2006-08-25T14:30:03-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: > D) Virtual scan of mm's in the over-limit container > > E) Modify existing physical scanner to be able to skip pages which > belong to not-over-limit containers. The same applies to dentries/inodes too, doesn't it? But their scan is already virtual so their LRUs could be just split into a per container list (but then didn't Christoph L. plan to rewrite that code anyways?) For memory my guess would be that (E) would be easier than (D) for user/file memory though less efficient. -Andi
creator: Andi Kleen
date: 2006-08-25T14:48:00-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: Andrew Morton wrote: > On Fri, 25 Aug 2006 15:49:15 +0400 > Kirill Korotaev <dev@sw.ru> wrote: > > >>>We need to go over this work before we can commit to the BC >>>core. Last time I looked at the VM accounting patch it >>>seemed rather unpleasing from a maintainability POV. >> >>hmmm... in which regard? > > > Little changes all over the MM code which might get accidentally broken. I still think doing simple accounting per-page would be a better way to go than trying to pin down all "user allocatable" kernel allocations. And would require all of about 2 hooks in the page allocator. And would track *actual* RAM allocated by that container. Can we continue that discussion (ie. why it isn't good enough). Last I was told it is not perfect and can be unfair... sounds like it fits the semantics perfectly ;) -- SUSE Labs, Novell Inc. Send instant messages to your online friends http://au.messenger.yahoo.com
creator: Nick Piggin
date: 2006-08-25T15:14:03-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: Ar Sad, 2006-08-26 am 01:14 +1000, ysgrifennodd Nick Piggin: > I still think doing simple accounting per-page would be a better way to > go than trying to pin down all "user allocatable" kernel allocations. > And would require all of about 2 hooks in the page allocator. And would > track *actual* RAM allocated by that container. You have a variety of kernel objects you want to worry about and they have very differing properties. Some are basically shared resources - page cache, dentries, inodes, etc and can be balanced pretty well by the kernel (ok the dentries are a bit of a problem right now). Others are very specific "owned" resources - like file handles, sockets and vmas. Tracking actual RAM use by container/user/.. isn't actually that interesting. It's also inconveniently sub page granularity. Its a whole seperate question whether you want a separate bean counter limit for sockets, file handles, vmas etc. Alan
creator: Alan Cox
date: 2006-08-25T15:36:08-00:00

Re: BC: resource beancounters (v2)
type: item
description: On Fri, Aug 25, 2006 at 07:30:03AM -0700, Andrew Morton wrote: > On Fri, 25 Aug 2006 15:49:15 +0400 > Kirill Korotaev <dev@sw.ru> wrote: > > > Andrey Savochkin wrote already a brief summary on vm resource management: > > > > ------------- cut ---------------- > > The task of limiting a container to 4.5GB of memory bottles down to the > > question: what to do when the container starts to use more than assigned > > 4.5GB of memory? > > > > At this moment there are only 3 viable alternatives. > > > > A) Have separate memory management for each container, > > with separate buddy allocator, lru lists, page replacement mechanism. > > That implies a considerable overhead, and the main challenge there > > is sharing of pages between these separate memory managers. > > > > B) Return errors on extension of mappings, but not on page faults, where > > memory is actually consumed. > > In this case it makes sense to take into account not only the size of used > > memory, but the size of created mappings as well. > > This is approximately what "privvmpages" accounting/limiting provides in > > UBC. > > > > C) Rely on OOM killer. > > This is a fall-back method in UBC, for the case "privvmpages" limits > > still leave the possibility to overload the system. > > > > D) Virtual scan of mm's in the over-limit container > > E) Modify existing physical scanner to be able to skip pages which > belong to not-over-limit containers. I've actually tried (E), but it didn't work as I wished. It didn't handle well shared pages. Then, in my experiments such modified scanner was unable to regulate quality-of-service. When I ran 2 over-the-limit containers, they worked equally slow regardless of their limits and work set size. That is, I didn't observe a smooth transition "under limit, maximum performance" to "slightly over limit, a bit reduced performance" to "significantly over limit, poor performance". Neither did I see any fairness in how containers got penalized for exceeding their limits. My e…
creator: Andrey Savochkin
date: 2006-08-25T16:30:26-00:00

Re: BC: resource beancounters (v2)
type: item
description: On Fri, 25 Aug 2006 20:30:26 +0400 Andrey Savochkin <saw@sw.ru> wrote: > On Fri, Aug 25, 2006 at 07:30:03AM -0700, Andrew Morton wrote: > > > > D) Virtual scan of mm's in the over-limit container > > > > E) Modify existing physical scanner to be able to skip pages which > > belong to not-over-limit containers. > > I've actually tried (E), but it didn't work as I wished. > > It didn't handle well shared pages. > Then, in my experiments such modified scanner was unable to regulate > quality-of-service. When I ran 2 over-the-limit containers, they worked > equally slow regardless of their limits and work set size. > That is, I didn't observe a smooth transition "under limit, maximum > performance" to "slightly over limit, a bit reduced performance" to > "significantly over limit, poor performance". Neither did I see any fairness > in how containers got penalized for exceeding their limits. > > My explanation of what I observed is that > - since filesystem caches play a huge role in performance, page scanner will > be very limited in controlling container's performance if caches > stay shared between containers, > - in the absence of decent disk I/O manager, stalls due to swapin/swapout > are more influenced by disk subsystem than by page scanner policy. > So in fact modified page scanner provides control over memory usage only as > "stay under limits or die", and doesn't show many advantages over (B) or (C). > At the same time, skipping pages visibly penalizes "good citizens", not only > in disk bandwidth but in CPU overhead as well. > > So I settled for (A)-(C) for now. > But it certainly would be interesting to hear if someone else makes such > experiments. > Makes sense. If one is looking for good machine partitioning then a shared disk is obviously a great contention point. To address that we'd need to be able to say "container A swaps to /dev/sda1 and container B swaps to /dev/sdb1". But the swap system at present can't do that.
creator: Andrew Morton
date: 2006-08-25T17:50:47-00:00

Re: BC: resource beancounters (v2)
type: item
description: Have you seen/tried the memory controller in CKRM/Resource Groups ? http://sourceforge.net/projects/ckrm It maintains a per resource group LRU lists and also maintains a list of over-guarantee groups (with ordering based on where they are in their guarantee-limit scale). So, when a reclaim needs to happen, pages are first freed from a group that is way over its limit, and then the next one and so on. Few things that it does that are not good: - doesn't account shared pages accurately - moves all pages from a task when the task moves to a different group - totally new reclamation path regards, chandra On Fri, 2006-08-25 at 20:30 +0400, Andrey Savochkin wrote: > On Fri, Aug 25, 2006 at 07:30:03AM -0700, Andrew Morton wrote: > > On Fri, 25 Aug 2006 15:49:15 +0400 > > Kirill Korotaev <dev@sw.ru> wrote: > > > > > Andrey Savochkin wrote already a brief summary on vm resource management: > > > > > > ------------- cut ---------------- > > > The task of limiting a container to 4.5GB of memory bottles down to the > > > question: what to do when the container starts to use more than assigned > > > 4.5GB of memory? > > > > > > At this moment there are only 3 viable alternatives. > > > > > > A) Have separate memory management for each container, > > > with separate buddy allocator, lru lists, page replacement mechanism. > > > That implies a considerable overhead, and the main challenge there > > > is sharing of pages between these separate memory managers. > > > > > > B) Return errors on extension of mappings, but not on page faults, where > > > memory is actually consumed. > > > In this case it makes sense to take into account not only the size of used > > > memory, but the size of created mappings as well. > > > This is approximately what "privvmpages" accounting/limiting provides in > > > UBC. > > > > > > C) Rely on OOM killer. > > > This is a fall-back method in UBC, for the case "privvmpages" limits > > > still leave the possibility to overload the system. > > > > > > > D) …
creator: Chandra Seetharaman
date: 2006-08-25T19:00:05-00:00

Re: BC: resource beancounters (v2)
type: item
description: On Fri, 2006-08-25 at 20:30 +0400, Andrey Savochkin wrote: > On Fri, Aug 25, 2006 at 07:30:03AM -0700, Andrew Morton wrote: > > On Fri, 25 Aug 2006 15:49:15 +0400 > > Kirill Korotaev <dev@sw.ru> wrote: > > > > > Andrey Savochkin wrote already a brief summary on vm resource management: > > > > > > ------------- cut ---------------- > > > The task of limiting a container to 4.5GB of memory bottles down to the > > > question: what to do when the container starts to use more than assigned > > > 4.5GB of memory? > > > > > > At this moment there are only 3 viable alternatives. > > > > > > A) Have separate memory management for each container, > > > with separate buddy allocator, lru lists, page replacement mechanism. > > > That implies a considerable overhead, and the main challenge there > > > is sharing of pages between these separate memory managers. > > > Yes, sharing of pages across different containers/managers will be a problem. Why not just disallow that scenario (that is what fake nodes proposal would also end up doing). > > > B) Return errors on extension of mappings, but not on page faults, where > > > memory is actually consumed. > > > In this case it makes sense to take into account not only the size of used > > > memory, but the size of created mappings as well. > > > This is approximately what "privvmpages" accounting/limiting provides in > > > UBC. > > > Keeping a tab on all the virtual mappings in a container must also be troublesome. And IMO is not the right way to go...this is even a stricter version of overcommit_memory...right? > > > > C) Rely on OOM killer. > > > This is a fall-back method in UBC, for the case "privvmpages" limits > > > still leave the possibility to overload the system. > > > > > > > D) Virtual scan of mm's in the over-limit container > > This seems like an interesting choice. If we can quickly inactivate some pages belonging to tasks in over_the_limit container. > > E) Modify existing physical scanner to be able to skip pages which…
creator: Rohit Seth
date: 2006-08-26T02:15:52-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: Alan Cox wrote: > Ar Sad, 2006-08-26 am 01:14 +1000, ysgrifennodd Nick Piggin: > >>I still think doing simple accounting per-page would be a better way to >>go than trying to pin down all "user allocatable" kernel allocations. >>And would require all of about 2 hooks in the page allocator. And would >>track *actual* RAM allocated by that container. > > > You have a variety of kernel objects you want to worry about and they > have very differing properties. > > Some are basically shared resources - page cache, dentries, inodes, etc > and can be balanced pretty well by the kernel (ok the dentries are a bit > of a problem right now). Others are very specific "owned" resources - > like file handles, sockets and vmas. That's true (OTOH I'd argue it would still be very useful for things like pagecache, so one container can't start a couple of 'dd' loops and turn everyone else to crap). And while the sharing may not be exactly captured, statistically things should balance over time. So I'm not arguing about _also_ accounting resources that are limited in other ways (than just the RAM they consume). But as a DoS protection measure on RAM usage, trying to account all kernel allocations that are user triggerable just sounds hard to maintain, holey, ugly, invsive (and not perfect either -- in fact it still isn't clear to me that it is any better than my proposal). > > Tracking actual RAM use by container/user/.. isn't actually that > interesting. It's also inconveniently sub page granularity. If it isn't interesting, then I don't think we want it (at least, until someone does get an interest in it). > > Its a whole seperate question whether you want a separate bean counter > limit for sockets, file handles, vmas etc. Yeah that's fair enough. We obviously want to avoid exposing limits on things that it doesn't make sense to limit, or that is a kernel implementation detail as much as possible. eg. so I would be happy to limit virtual address, less happy to limit vmas alone (unle…
creator: Nick Piggin
date: 2006-08-26T03:55:43-00:00

Re: BC: resource beancounters (v2)
type: item
description: Ar Gwe, 2006-08-25 am 19:15 -0700, ysgrifennodd Rohit Seth: > Yes, sharing of pages across different containers/managers will be a > problem. Why not just disallow that scenario (that is what fake nodes > proposal would also end up doing). Because it destroys the entire point of using containers instead of something like Xen - which is sharing. Also at the point I am using beancounters per user I don't want glibc per use, libX11 per use glib per use gtk per user etc..
creator: Alan Cox
date: 2006-08-26T16:16:02-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: Andi Kleen wrote: >>D) Virtual scan of mm's in the over-limit container >> >>E) Modify existing physical scanner to be able to skip pages which >> belong to not-over-limit containers. > > > The same applies to dentries/inodes too, doesn't it? But their > scan is already virtual so their LRUs could be just split into > a per container list (but then didn't Christoph L. plan to > rewrite that code anyways?) how do you propose to handle shared files in this case? > For memory my guess would be that (E) would be easier than (D) for user/file > memory though less efficient. Kirill
creator: dev
date: 2006-08-28T08:27:16-00:00

Re: BC: resource beancounters (v2)
type: item
description: On Sat, 2006-08-26 at 17:37 +0100, Alan Cox wrote: > Ar Gwe, 2006-08-25 am 19:15 -0700, ysgrifennodd Rohit Seth: > > Yes, sharing of pages across different containers/managers will be a > > problem. Why not just disallow that scenario (that is what fake nodes > > proposal would also end up doing). > > Because it destroys the entire point of using containers instead of > something like Xen - which is sharing. Also at the point I am using > beancounters per user I don't want glibc per use, libX11 per use glib > per use gtk per user etc.. > > I'm not saying per use glibc etc. That will indeed be useless and bring it to virtualization world. Just like fake node, one should be allowed to use pages that are already in (for example) page cache- so that you don't end up duplicating all shared stuff. But as far as charging is concerned, charge it to container who either got the page in page cache OR if FS based semantics exist then charge it to the container where the file belongs. What I was suggesting is to not charge a page to different counters. -rohit
creator: Rohit Seth
date: 2006-08-28T16:48:41-00:00

Re: Re: BC: resource beancounters (v2)
type: item
description: Rohit Seth wrote: > On Sat, 2006-08-26 at 17:37 +0100, Alan Cox wrote: > >> Ar Gwe, 2006-08-25 am 19:15 -0700, ysgrifennodd Rohit Seth: >> >>> Yes, sharing of pages across different containers/managers will be a >>> problem. Why not just disallow that scenario (that is what fake nodes >>> proposal would also end up doing). >>> >> Because it destroys the entire point of using containers instead of >> something like Xen - which is sharing. Also at the point I am using >> beancounters per user I don't want glibc per use, libX11 per use glib >> per use gtk per user etc.. >> >> >> > > I'm not saying per use glibc etc. That will indeed be useless and bring > it to virtualization world. Just like fake node, one should be allowed > to use pages that are already in (for example) page cache- so that you > don't end up duplicating all shared stuff. But as far as charging is > concerned, charge it to container who either got the page in page cache > OR if FS based semantics exist then charge it to the container where the > file belongs. What I was suggesting is to not charge a page to > different counters. > Consider the following simple scenario: there are 50 containers (numbered, say, 1 to 50) all sharing a single installation of Fedora Core 5. They all run sshd, apache, syslogd, crond and some other stuff like that. This is actually quite a real scenario. In the world that you propose the container which was unlucky to start first (probably the one with ID of either 1 or 50) will be charged for all the memory, and all the others will have most of their memory for free. And in such a world per-container memory accounting or limiting is just not possible.
creator: kir
date: 2006-08-28T17:40:18-00:00

Re: Re: BC: resource beancounters (v2)
type: item
description: On Mon, 2006-08-28 at 21:41 +0400, Kir Kolyshkin wrote: > Rohit Seth wrote: > > > > I'm not saying per use glibc etc. That will indeed be useless and bring > > it to virtualization world. Just like fake node, one should be allowed > > to use pages that are already in (for example) page cache- so that you > > don't end up duplicating all shared stuff. But as far as charging is > > concerned, charge it to container who either got the page in page cache > > OR if FS based semantics exist then charge it to the container where the > > file belongs. What I was suggesting is to not charge a page to > > different counters. > > > > Consider the following simple scenario: there are 50 containers > (numbered, say, 1 to 50) all sharing a single installation of Fedora > Core 5. They all run sshd, apache, syslogd, crond and some other stuff > like that. This is actually quite a real scenario. > > In the world that you propose the container which was unlucky to start > first (probably the one with ID of either 1 or 50) will be charged for > all the memory, and all the > others will have most of their memory for free. And in such a world > per-container memory accounting or limiting is just not possible. If you are only having task based accounting then yes the first container using a page will be charged. And when it hit its limit then it will inactivate some of the pages. If some other container now uses the same page (that got inactivated) again then this next container will be charged for that page. Though if we have file/directory based accounting then shared pages belonging to /usr/lib or /usr/bin can go to a common container. -rohit
creator: Rohit Seth
date: 2006-08-28T22:28:09-00:00

Re: [PATCH 6/6] BC: kernel memory accounting (marks)
type: item
description: Dave Hansen wrote: > I'm still a bit concerned about if we actually need the 'struct page' > pointer. I've gone through all of the users, and I'm not sure that I > see any that _require_ having a pointer in 'struct page'. I think it > will take some rework, especially with the pagetables, but it should be > quite doable. don't worry: 1. we will introduce a separate patch moving this pointer into mirroring array 2. this pointer is still required for _user_ pages tracking, that's why I don't follow your suggestion right now... > vmalloc: > Store in vm_struct > fd_set_bits: > poll_get: > mount hashtable: > Don't need alignment. use the slab? > pagetables: > either store in an extra field of 'struct page', or use the > mm's. mm should always be available when alloc/freeing a > pagetable page > > Did I miss any? flocks, pipe buffers, task_struct, sighand, signal, vmas, posix timers, uid_cache, shmem dirs, Thanks, Kirill
creator: dev
date: 2006-08-29T09:52:54-00:00

Re: Re: BC: resource beancounters (v2)
type: item
description: Ar Llu, 2006-08-28 am 15:28 -0700, ysgrifennodd Rohit Seth: > Though if we have file/directory based accounting then shared pages > belonging to /usr/lib or /usr/bin can go to a common container. So that one user can map all the spare libraries and config files and DoS the system by preventing people from accessing the libraries they do need ?
creator: Alan Cox
date: 2006-08-29T10:15:35-00:00

Re: [PATCH 6/6] BC: kernel memory accounting (marks)
type: item
description: Dave Hansen wrote: > On Wed, 2006-08-23 at 15:08 +0400, Kirill Korotaev wrote: > >> include/asm-i386/thread_info.h | 4 ++-- >> include/asm-ia64/pgalloc.h | 24 +++++++++++++++++------- >> include/asm-x86_64/pgalloc.h | 12 ++++++++---- >> include/asm-x86_64/thread_info.h | 5 +++-- > > > Do you think we need to cover a few more architectures before > considering merging this, or should we just fix them up as we need them? I think doing a part of job is usually bad as it never gets fixed fully then :/ > I'm working on a patch to unify as many of the alloc_thread_info() > functions as I can. That should at least give you one place to modify > and track the thread_info allocations. I've only compiled for x86_64 > and i386, but I'm working on more. A preliminary version is attached. Oh, I think such code unification is nice! Would be perferct if all of them could be merged to some generic function. Please, keep me on CC! Thanks, Kirill
creator: dev
date: 2006-08-29T14:34:31-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: Andrew Morton wrote: > On Fri, 25 Aug 2006 15:49:15 +0400 > Kirill Korotaev <dev@sw.ru> wrote: > > >>>We need to go over this work before we can commit to the BC >>>core. Last time I looked at the VM accounting patch it >>>seemed rather unpleasing from a maintainability POV. >> >>hmmm... in which regard? > > > Little changes all over the MM code which might get accidentally broken. > > >>>And, if I understand it correctly, the only response to a job >>>going over its VM limits is to kill it, rather than trimming >>>it. Which sounds like a big problem? >> >>No, UBC virtual memory management refuses occur on mmap()'s. > > > That's worse, isn't it? Firstly it rules out big sparse mappings and secondly 1) if mappings are private then yes, you can not mmap too much. This is logical, since this whole mappings are potentially allocatable and there is no way to control it later except for SIGKILL. 2) if mappings are shared file mappings (shmem is handled in similar way) then you can mmap as much as you want, since these pages can be reclaimed. > mmap_and_use(80% of container size) > fork_and_immediately_exec(/bin/true) > > will fail at the fork? yes, it will fail on fork() or exec() in case of much private (1) mappings. fail on fork() and exec() is much friendlier then SIGKILL, don't you think so? private mappings parameter which is limited by UBC is a kind of upper estimation of container RSS. From our experience such an estimation is ~ 5-20% higher then a real physical memory used (with real-life applications). >>Andrey Savochkin wrote already a brief summary on vm resource management: >> >>------------- cut ---------------- >>The task of limiting a container to 4.5GB of memory bottles down to the >>question: what to do when the container starts to use more than assigned >>4.5GB of memory? >> >>At this moment there are only 3 viable alternatives. >> >>A) Have separate memory management for each container, >> with separate buddy allocator, lru lists, page replacement mecha…
creator: dev
date: 2006-08-29T15:33:04-00:00

Re: [PATCH 6/6] BC: kernel memory accounting (marks)
type: item
description: On Tue, 2006-08-29 at 13:52 +0400, Kirill Korotaev wrote: > 1. we will introduce a separate patch moving this pointer > into mirroring array > 2. this pointer is still required for _user_ pages tracking, > that's why I don't follow your suggestion right now... You hadn't mentioned that part. ;) I guess we'll wait for the user patches before these can go any further. -- Dave
creator: Dave Hansen
date: 2006-08-29T15:48:45-00:00

Re: [PATCH 6/6] BC: kernel memory accounting (marks)
type: item
description: Dave Hansen wrote: > On Tue, 2006-08-29 at 13:52 +0400, Kirill Korotaev wrote: > >>1. we will introduce a separate patch moving this pointer >> into mirroring array >>2. this pointer is still required for _user_ pages tracking, >> that's why I don't follow your suggestion right now... > > > You hadn't mentioned that part. ;) I was crying about it 2 or 3 times :) But no suprise that you hadn't notice that due to too many emails on BC. > I guess we'll wait for the user patches before these can go any further. :))) are you the one to decide? :) then lets drink some beer :))) Thanks, Kirill
creator: dev
date: 2006-08-29T15:53:09-00:00

Re: [PATCH] BC: resource beancounters (v2)
type: item
description: Kirill Korotaev wrote: >>> ------------- cut ---------------- >>> The task of limiting a container to 4.5GB of memory bottles down to the >>> question: what to do when the container starts to use more than assigned >>> 4.5GB of memory? >>> >>> At this moment there are only 3 viable alternatives. >>> >>> A) Have separate memory management for each container, >>> with separate buddy allocator, lru lists, page replacement mechanism. >>> That implies a considerable overhead, and the main challenge there >>> is sharing of pages between these separate memory managers. >>> >>> B) Return errors on extension of mappings, but not on page faults, where >>> memory is actually consumed. >>> In this case it makes sense to take into account not only the size >>> of used >>> memory, but the size of created mappings as well. >>> This is approximately what "privvmpages" accounting/limiting >>> provides in >>> UBC. >>> >>> C) Rely on OOM killer. >>> This is a fall-back method in UBC, for the case "privvmpages" limits >>> still leave the possibility to overload the system. >>> >> >> >> D) Virtual scan of mm's in the over-limit container >> >> E) Modify existing physical scanner to be able to skip pages which >> belong to not-over-limit containers. >> >> F) Something else ;) > We fully agree that other possible algorithms can and should exist. > My idea only is that any of them would need accounting anyway > (which is the most part of beancounters). > Throtling, modified scanners etc. can be implemented as a separate > BC parameters. Thus, an administrator will be able to select > which policy should be applied to the container which is near its limit. > > So the patches I'm trying to send are a step-by-step accounting of all > the resources and their simple limitations. More comprehensive limitation > policy will be built on top of it later. > One of the issues I see is that bean counters are not very flexible. Tasks cannot change bean counters dynamically after fork()/exec() that is -…
creator: Balbir Singh
date: 2006-08-29T17:08:45-00:00

Re: Re: BC: resource beancounters (v2)
type: item
description: On Tue, 2006-08-29 at 11:15 +0100, Alan Cox wrote: > Ar Llu, 2006-08-28 am 15:28 -0700, ysgrifennodd Rohit Seth: > > Though if we have file/directory based accounting then shared pages > > belonging to /usr/lib or /usr/bin can go to a common container. > > So that one user can map all the spare libraries and config files and > DoS the system by preventing people from accessing the libraries they do > need ? > Well, there is a risk whenever there is sharing across containers. The point though is, give the choice to sysadmin to configure the platform the way it is appropriate. -rohit
creator: Rohit Seth
date: 2006-08-29T17:30:35-00:00

Re: Re: BC: resource beancounters (v2)
type: item
description: Ar Maw, 2006-08-29 am 10:30 -0700, ysgrifennodd Rohit Seth: > On Tue, 2006-08-29 at 11:15 +0100, Alan Cox wrote: > > Ar Llu, 2006-08-28 am 15:28 -0700, ysgrifennodd Rohit Seth: > > > Though if we have file/directory based accounting then shared pages > > > belonging to /usr/lib or /usr/bin can go to a common container. > > > > So that one user can map all the spare libraries and config files and > > DoS the system by preventing people from accessing the libraries they do > > need ? > > > > Well, there is a risk whenever there is sharing across containers. The > point though is, give the choice to sysadmin to configure the platform > the way it is appropriate. In other words your suggestion doesn't actually work for the real world cases like web serving.
creator: Alan Cox
date: 2006-08-29T18:46:07-00:00

Re: Re: BC: resource beancounters (v2)
type: item
description: On Tue, 2006-08-29 at 20:06 +0100, Alan Cox wrote: > Ar Maw, 2006-08-29 am 10:30 -0700, ysgrifennodd Rohit Seth: > > On Tue, 2006-08-29 at 11:15 +0100, Alan Cox wrote: > > > Ar Llu, 2006-08-28 am 15:28 -0700, ysgrifennodd Rohit Seth: > > > > Though if we have file/directory based accounting then shared pages > > > > belonging to /usr/lib or /usr/bin can go to a common container. > > > > > > So that one user can map all the spare libraries and config files and > > > DoS the system by preventing people from accessing the libraries they do > > > need ? > > > > > > > Well, there is a risk whenever there is sharing across containers. The > > point though is, give the choice to sysadmin to configure the platform > > the way it is appropriate. > > In other words your suggestion doesn't actually work for the real world > cases like web serving. > Containers are not going to solve all the problems particularly the scenarios like when a machine is a web server and an odd user can log on to the same machine and (w/o any ulimits) claim all the memory that is present in the system. Though it is quite possible to implement a combination of two (task and fs based) policies in containers and sysadmin can set a preference of each each container. [this probably is another reason for having a per page container pointer]. -rohit
creator: Rohit Seth
date: 2006-08-29T19:15:16-00:00