OpenVZ Forum - RDF feed
type: channel
items: BC: resource beancounters (v6) (with userpages reclamation + configfs)
items: [PATCH 1/13] BC: atomic_dec_and_lock_irqsave() helper
items: [PATCH 2/13] BC: Kconfig and Makefile
items: [PATCH 3/13] BC: beancounters core and API
items: [PATCH 4/13] BC: context handling
items: [PATCH 5/13] BC: configfs interface
items: [PATCH 6/13] BC: kmemsize accounting (core)
items: [PATCH 7/13] BC: kmemsize accounting (hooks)
items: [PATCH 8/13] BC: privvmpages accounting (core)
items: [PATCH 9/13] BC: privvmpages accounting (hooks)
items: [PATCH 10/13] BC: physpages accounting (core)
items: [PATCH 11/13] BC: physpages accounting (hooks)
items: [PATCH 12/13] BC: numtasks accounting
items: [PATCH 13/13] BC: numfiles accounting
items: Re: [PATCH 6/13] BC: kmemsize accounting (core)
items: Re: [PATCH 1/13] BC: atomic_dec_and_lock_irqsave() helper
items: Re: BC: resource beancounters (v6) (with userpages reclamation + configfs)
items: Re: [ckrm-tech] [PATCH 4/13] BC: context handling

BC: resource beancounters (v6) (with userpages reclamation + configfs)
type: item
description: MAJOR CHANGES in v6 (see details below): - configfs interface instead of syscalls (as wanted by CKRM people...) - added numfiles resource accounting - added numtasks resource accounting numfiles and numtasks controllers demonstrate how clean and simple BC interface is. Patch set is applicable to 2.6.19-rc5-mm1 ----------------------------------------------------- Resource BeanCounters (BC). BC allows to account and control consumption of kernel resources used by *group* of processes (users, containers, ...). Draft BC 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 and not reclaimable 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) Summary of changes from v5 patch set: * configfs interface instead of syscalls (as wanted by CKRM people) * added numfiles resource accounting * added numtasks resource accounting * introduced dummy_resource to handle case when no resource registered * calls to rss accounting are integrated to rmap calls Summary of changes from v4 patch set: * changed set of resources - kmemsize, privvmpages, physpages * added event hooks for resources (init, limit hit etc) * added user pages reclamation (bc_try_to_free_pages) * removed pages sharing accounting - charge to first user * task now carries only one BC pointer, simplified * make set_bcid syscall move arbi…
creator: dev
date: 2006-11-09T16:42:56-00:00

[PATCH 1/13] BC: atomic_dec_and_lock_irqsave() helper
type: item
description: Oleg Nesterov noticed to me that the construction like (used in beancounter patches and free_uid()): local_irq_save(flags); if (atomic_dec_and_lock(&refcnt, &lock)) ... is not that good for preemtible kernels, since with preemption spin_lock() can schedule() to reduce latency. However, it won't schedule if interrupts are disabled. So this patch introduces atomic_dec_and_lock_irqsave() as a logical counterpart to atomic_dec_and_lock(). Signed-Off-By: Pavel Emelianov <xemul@sw.ru> Signed-Off-By: Kirill Korotaev <dev@sw.ru> --- include/linux/spinlock.h | 6 ++++++ kernel/user.c | 5 +---- lib/dec_and_lock.c | 19 +++++++++++++++++++ 3 files changed, 26 insertions(+), 4 deletions(-) --- ./include/linux/spinlock.h.bcprep 2006-11-03 17:46:25.000000000 +0300 +++ ./include/linux/spinlock.h 2006-11-03 17:46:31.000000000 +0300 @@ -319,6 +319,12 @@ extern int _atomic_dec_and_lock(atomic_t #define atomic_dec_and_lock(atomic, lock) \ __cond_lock(lock, _atomic_dec_and_lock(atomic, lock)) +extern int _atomic_dec_and_lock_irqsave(atomic_t *atomic, spinlock_t *lock, + unsigned long *flagsp); +#define atomic_dec_and_lock_irqsave(atomic, lock, flags) \ + __cond_lock(lock, \ + _atomic_dec_and_lock_irqsave(atomic, lock, &flags)) + /** * spin_can_lock - would spin_trylock() succeed? * @lock: the spinlock in question. --- ./kernel/user.c.bcprep 2006-11-03 17:46:25.000000000 +0300 +++ ./kernel/user.c 2006-11-03 17:46:31.000000000 +0300 @@ -108,15 +108,12 @@ void free_uid(struct user_struct *up) if (!up) return; - local_irq_save(flags); - if (atomic_dec_and_lock(&up->__count, &uidhash_lock)) { + if (atomic_dec_and_lock_irqsave(&up->__count, &uidhash_lock, flags)) { uid_hash_remove(up); spin_unlock_irqrestore(&uidhash_lock, flags); key_put(up->uid_keyring); key_put(up->session_keyring); kmem_cache_free(uid_cachep, up); - } else { - local_irq_restore(flags); } } --- ./lib/dec_and_lock.c.bcprep 2006-11-03 17:46:25.000000000 +0300 +++ ./lib/dec_and_lock.c 2006-11-03 17:46:31.000000000 +0300 @@ -33…
creator: dev
date: 2006-11-09T16:47:20-00:00

[PATCH 2/13] BC: Kconfig and Makefile
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> --- init/Kconfig | 4 ++++ kernel/Makefile | 1 + kernel/bc/Kconfig | 17 +++++++++++++++++ kernel/bc/Makefile | 11 +++++++++++ 4 files changed, 33 insertions(+) --- ./init/Kconfig.bckconfig 2006-11-09 11:29:12.000000000 +0300 +++ ./init/Kconfig 2006-11-09 11:30:21.000000000 +0300 @@ -585,6 +585,10 @@ config STOP_MACHINE Need stop_machine() primitive. endmenu +menu "Beancounters" +source "kernel/bc/Kconfig" +endmenu + menu "Block layer" source "block/Kconfig" endmenu --- ./kernel/Makefile.bckconfig 2006-11-09 11:29:12.000000000 +0300 +++ ./kernel/Makefile 2006-11-09 11:30:21.000000000 +0300 @@ -12,6 +12,7 @@ obj-y = sched.o fork.o exec_domain.o obj-$(CONFIG_STACKTRACE) += stacktrace.o obj-y += time/ +obj-$(CONFIG_BEANCOUNTERS) += bc/ obj-$(CONFIG_DEBUG_MUTEXES) += mutex-debug.o obj-$(CONFIG_LOCKDEP) += lockdep.o ifeq ($(CONFIG_PROC_FS),y) --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./kernel/bc/Kconfig 2006-11-09 11:30:21.000000000 +0300 @@ -0,0 +1,17 @@ +config BEANCOUNTERS + bool "Enable resource accounting/control" + default n + depends on CONFIGFS_FS + help + When Y this option provides accounting and allows configuring + limits for user's consumption of exhaustible system resources. + 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 accidental + misbehavior of processes or malicious activity aiming to ``kill'' + the system. It's worth mentioning that resource limits configured + by setrlimit(2) do not give an acceptable level of protection + because they cover only a small fraction of resources and work on a + per-process basis. Per-process accounting doesn't prevent malicious + users from s…
creator: dev
date: 2006-11-09T16:47:51-00:00

[PATCH 3/13] BC: beancounters core and 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 | 171 +++++++++++++++++++++++++++++++ include/linux/types.h | 16 ++ init/main.c | 3 kernel/bc/beancounter.c | 253 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 443 insertions(+) --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./include/bc/beancounter.h 2006-11-03 17:47:18.000000000 +0300 @@ -0,0 +1,171 @@ +/* + * include/bc/beancounter.h + * + * Copyright (C) 2006 OpenVZ SWsoft Inc + * + */ + +#ifndef __BEANCOUNTER_H__ +#define __BEANCOUNTER_H__ + +enum { + BC_KMEMSIZE, + BC_PRIVVMPAGES, + BC_PHYSPAGES, + BC_NUMTASKS, + BC_NUMFILES, + + BC_RESOURCES +}; + +struct bc_resource_parm { + unsigned long barrier; + unsigned long limit; + unsigned long held; + unsigned long minheld; + unsigned long maxheld; + unsigned long failcnt; +}; + +#ifdef __KERNEL__ + +#include <linux/list.h> +#include <linux/spinlock.h> +#include <linux/init.h> +#include <linux/configfs.h> +#include <asm/atomic.h> + +#define BC_MAXVALUE ((unsigned long)LONG_MAX) + +enum bc_severity { + BC_BARRIER, + BC_LIMIT, + BC_FORCE, +}; + +struct beancounter; + +#ifdef CONFIG_BEANCOUNTERS + +struct bc_resource { + char *bcr_name; + + int (*bcr_init)(struct beancounter *bc, int res); + int (*bcr_change)(struct beancounter *bc, + unsigned long new_bar, unsigned long new_lim); + void (*bcr_barrier_hit)(struct beancounter *bc); + int (*bcr_limit_hit)(struct beancounter *bc, unsigned long val, + unsigned long flags); + void (*bcr_fini)(struct beancounter *bc); +}; + +extern struct bc_resource *bc_resources[]; + +struct beancounter { + atomic_t bc_refcount; + spinlock_t bc_lock; + bcid_t bc_id; + struct hlist_node bc_hash; + + struct bc_resourc…
creator: dev
date: 2006-11-09T16:49:24-00:00

[PATCH 4/13] BC: context handling
type: item
description: Contains code responsible for setting BC on task, it's inheriting and setting host context in interrupts. Task references beancounter by exec_bc pointer: exec_bc: current context. All resources are charged to this beancounter. Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- arch/i386/kernel/irq.c | 5 ++ fs/proc/array.c | 6 ++ include/bc/task.h | 53 +++++++++++++++++++++++++ include/linux/sched.h | 5 ++ kernel/bc/beancounter.c | 2 kernel/bc/misc.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++++ kernel/fork.c | 18 +++++++- kernel/irq/handle.c | 6 ++ kernel/softirq.c | 5 ++ 9 files changed, 198 insertions(+), 2 deletions(-) --- ./arch/i386/kernel/irq.c.bcctx 2006-11-09 11:29:09.000000000 +0300 +++ ./arch/i386/kernel/irq.c 2006-11-09 11:32:24.000000000 +0300 @@ -21,6 +21,8 @@ #include <asm/apic.h> #include <asm/uaccess.h> +#include <bc/task.h> + DEFINE_PER_CPU(irq_cpustat_t, irq_stat) ____cacheline_internodealigned_in_smp; EXPORT_PER_CPU_SYMBOL(irq_stat); @@ -75,6 +77,7 @@ fastcall unsigned int do_IRQ(struct pt_r union irq_ctx *curctx, *irqctx; u32 *isp; #endif + struct beancounter *bc; if (unlikely((unsigned)irq >= NR_IRQS)) { printk(KERN_EMERG "%s: cannot handle IRQ %d\n", @@ -99,6 +102,7 @@ fastcall unsigned int do_IRQ(struct pt_r } #endif + bc = set_exec_bc(&init_bc); #ifdef CONFIG_4KSTACKS curctx = (union irq_ctx *) current_thread_info(); @@ -139,6 +143,7 @@ fastcall unsigned int do_IRQ(struct pt_r #endif desc->handle_irq(irq, desc); + reset_exec_bc(bc, &init_bc); irq_exit(); set_irq_regs(old_regs); return 1; --- ./fs/proc/array.c.bcctx 2006-11-09 11:29:11.000000000 +0300 +++ ./fs/proc/array.c 2006-11-09 11:32:24.000000000 +0300 @@ -76,6 +76,8 @@ #include <linux/rcupdate.h> #include <linux/delayacct.h> +#include <bc/beancounter.h> + #include <asm/uaccess.h> #include <asm/pgtable.h> #include <asm/io.h> @@ -180,6 +182,10 @@ static inline char * task_state(struct t p->uid, p->euid, p->suid, p->fsuid, p->gid, p->egid, …
creator: dev
date: 2006-11-09T16:51:08-00:00

[PATCH 5/13] BC: configfs interface
type: item
description: configfs interface to beancounters. It may be done as module in case configfs itself is a module. Usage example: Prepare configfs interface ~~~~~~~~~~~~~~~~~~~~~~~~~~ # mount -t configfs none /cfg # ls /cfg beancounters # ls /cfg/beancounters/ 0 description # cat /cfg/beancounters/description Beancounters controll resource usage of task groups Look at beancounters subsystem structure ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # ls /cfg/beancounters/0/ id kmemsize numfiles numtasks physpages privvmpages tasks threads # ls /cfg/beancounters/0/numfiles barrier failcnt held limit maxheld minheld # cat /cfg/beancounters/0/numfiles/* 2147483647 0 163 2147483647 345 0 Create a new beancounter and move task into it ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # mkdir /cfg/beancounters/1 # echo -n $$ > /cfg/beancounters/1/tasks # cat /proc/$$/status | grep Bcid Bcid: 1 now you can see bc 1 usages etc. Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- configfs.c | 391 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ + 1 files changed, 391 insertions(+) --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./kernel/bc/configfs.c 2006-11-03 17:13:25.000000000 +0300 @@ -0,0 +1,391 @@ +/* + * kernel/bc/configfs.c + * + * Copyright (C) 2006 OpenVZ SWsoft Inc + * + * Pavel Emelianov <xemul@openvz.org> + * + */ + +#include <linux/configfs.h> +#include <linux/module.h> + +#include <bc/beancounter.h> +#include <bc/task.h> + +/* + * Declarations + */ + +enum bc_attr_indices { + BC_ATTR_ID, + BC_ATTR_TASKS, + BC_ATTR_THREADS, + BC_RES_HELD, + BC_RES_MAXHELD, + BC_RES_MINHELD, + BC_RES_BARRIER, + BC_RES_LIMIT, + BC_RES_FAILCNT, +}; + +struct bc_configfs_attribute { + int idx; + struct configfs_attribute attr; +}; + +#define attr_to_bc(attr) container_of(attr, \ + struct bc_configfs_attribute, attr) + +struct bc_resource_group { + int res; + struct config_group group; +}; + +#define item_to_res(i) container_of(i, \ + struct bc_resource…
creator: dev
date: 2006-11-09T16:52:24-00:00

[PATCH 6/13] BC: kmemsize 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/bc/kmem.h | 48 ++++++++++++++++++++ include/linux/gfp.h | 8 ++- include/linux/mm.h | 1 include/linux/mm_types.h | 3 + include/linux/slab.h | 4 + include/linux/vmalloc.h | 1 kernel/bc/kmem.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++ mm/slab.c | 91 ++++++++++++++++++++++++++++++-------- mm/vmalloc.c | 6 ++ 9 files changed, 253 insertions(+), 21 deletions(-) --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./include/bc/kmem.h 2006-11-03 15:48:26.000000000 +0300 @@ -0,0 +1,48 @@ +/* + * include/bc/kmem.h + * + * Copyright (C) 2006 OpenVZ SWsoft Inc + * + */ + +#ifndef __BC_KMEM_H_ +#define __BC_KMEM_H_ + +/* + * BC_KMEMSIZE accounting + */ + +#include <linux/slab.h> +#include <linux/gfp.h> + +struct page; +struct beancounter; + +#ifdef CONFIG_BEANCOUNTERS +int __must_check bc_page_charge(struct page *page, int order, gfp_t flags); +void bc_page_uncharge(struct page *page, int order); + +int __must_check bc_slab_charge(kmem_cache_t *cachep, void *obj, gfp_t flags); +void bc_slab_uncharge(kmem_cache_t *cachep, void *obj); +#else +static inline int __must_check bc_page_charge(struct page *page, int order, + gfp_t flags) +{ + return 0; +} + +static inline void bc_page_uncharge(struct page *page, int order) +{ +} + +static inline int __must_check bc_slab_charge(kmem_cache_t *cachep, void …
creator: dev
date: 2006-11-09T16:53:38-00:00

[PATCH 7/13] BC: kmemsize accounting (hooks)
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 | 6 +++--- 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/mempool.c | 2 ++ mm/page_alloc.c | 11 +++++++++++ mm/slab.c | 30 +++++++++++++++++++++++++++--- 22 files changed, 115 insertions(+), 56 deletions(-) --- ./arch/i386/kernel/ldt.c.bckmem 2006-11-09 11:29:09.000000000 +0300 +++ ./arch/i386/kernel/ldt.c 2006-11-09 11:33:27.000000000 +0300 @@ -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.bckmem 2006-11-09 11:18:45.000000000 +0300 +++ ./arch/i386/mm/init.c 2006-11-09 11:33:27.000000000 +0300 @@ -708,7 +708,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) @@ -717,7 +717,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, P…
creator: dev
date: 2006-11-09T16:54:37-00:00

[PATCH 8/13] BC: privvmpages accounting (core)
type: item
description: This patch instroduces new resource - BC_PRIVVMPAGES. It is an upper estimation of currently used physical memory. There are different approaches to user pages control: a) account all the mappings on mmap/brk and reject as soon as the sum of VMA's lengths reaches the barrier. This approach is very bad as applications always map more than they really use, very often MUCH more. b) account only the really used memory and reject as soon as RSS reaches the limit. This approach is not good either as user space pages are allocated in page fault handler and the only way to reject allocation is to kill the task. Comparing to previous scenarion this is much worse as application won't even be able to terminate gracefully. c) account a part of memory on mmap/brk and reject there, and account the rest of the memory in page fault handlers without any rejects. This type of accounting is used in UBC. d) account physical memory and behave like a standalone kernel - reclaim user memory when run out of it. This type of memory control is to be introduced later as an addition to c). UBC provides all the needed statistics for this (physical memory, swap pages etc.) Privvmpages accounting is described in details in http://wiki.openvz.org/User_pages_accounting A note about sys_mprotect: as it can change mapping state from bc_vm_private to !bc_vm_private and vice-versa appropriate amount of pages is (un)charged in mprotect_fixup. Signed-Off-By: Pavel Emelianov <xemul@sw.ru> Signed-Off-By: Kirill Korotaev <dev@sw.ru> --- include/bc/vmpages.h | 90 +++++++++++++++++++++++++++++++ include/linux/mm.h | 3 + include/linux/sched.h | 3 + kernel/bc/beancounter.c | 1 kernel/bc/vmpages.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 235 insertions(+) --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./include/bc/vmpages.h 2006-11-03 17:49:13.000000000 +0300 @@ -0,0 +1,90 @@ +/* + * include/bc/vmpages.h + * + * Copyright (C) 2006 OpenVZ SWsoft Inc + * + */ + +#ifndef __BC_V…
creator: dev
date: 2006-11-09T16:56:01-00:00

[PATCH 9/13] BC: privvmpages accounting (hooks)
type: item
description: BC privvmpages accounting hooks in generic code. Signed-Off-By: Pavel Emelianov <xemul@sw.ru> Signed-Off-By: Kirill Korotaev <dev@sw.ru> --- fs/binfmt_elf.c | 5 +-- include/asm-alpha/mman.h | 1 include/asm-generic/mman.h | 1 include/asm-mips/mman.h | 1 include/asm-parisc/mman.h | 1 include/asm-xtensa/mman.h | 1 kernel/fork.c | 11 ++++++ mm/mlock.c | 16 +++++++++ mm/mmap.c | 74 ++++++++++++++++++++++++++++++++++++++------- mm/mprotect.c | 18 ++++++++++ mm/mremap.c | 22 ++++++++++--- mm/shmem.c | 23 +++++++++++++ 12 files changed, 153 insertions(+), 21 deletions(-) --- ./fs/binfmt_elf.c.bcvmpagesk 2006-11-03 17:46:10.000000000 +0300 +++ ./fs/binfmt_elf.c 2006-11-03 17:50:04.000000000 +0300 @@ -363,7 +363,7 @@ static unsigned long load_elf_interp(str eppnt = elf_phdata; for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) { if (eppnt->p_type == PT_LOAD) { - int elf_type = MAP_PRIVATE | MAP_DENYWRITE; + int elf_type = MAP_PRIVATE|MAP_DENYWRITE|MAP_EXECPRIO; int elf_prot = 0; unsigned long vaddr = 0; unsigned long k, map_addr; @@ -849,7 +849,8 @@ static int load_elf_binary(struct linux_ if (elf_ppnt->p_flags & PF_X) elf_prot |= PROT_EXEC; - elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE; + elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE | + MAP_EXECPRIO; vaddr = elf_ppnt->p_vaddr; if (loc->elf_ex.e_type == ET_EXEC || load_addr_set) { --- ./include/asm-alpha/mman.h.bcvmpagesk 2006-11-03 17:46:10.000000000 +0300 +++ ./include/asm-alpha/mman.h 2006-11-03 17:50:04.000000000 +0300 @@ -14,6 +14,7 @@ #define MAP_TYPE 0x0f /* Mask for type of mapping (OSF/1 is _wrong_) */ #define MAP_FIXED 0x100 /* Interpret addr exactly */ #define MAP_ANONYMOUS 0x10 /* don't use a file */ +#define MAP_EXECPRIO 0x20 /* charge with BC_LIMIT severity */ /* not used by linux, but here to make sure we don't clash with OSF/1 defines */ #define _MAP_HASSEMAPHORE 0x0200 --- ./include/asm-generic/mman.h.bcvmpagesk 2006-11-03 17:46:10.000000000 +0300 +++ ./include/asm-generic/mman.h 2…
creator: dev
date: 2006-11-09T16:57:24-00:00

[PATCH 10/13] BC: physpages accounting (core)
type: item
description: This is the core of vmrss accounting. The main introduced object is page_beancounter. It ties together page and BCs which use the page. page_beancounter also allows quick per-container pages reclamation. and helps correctly account fractions of memory shared between BCs (http://wiki.openvz.org/RSS_fractions_accounting) Page charge/uncharge is performed on first map/last unmap and is based on page->mapcount calculations. Signed-Off-By: Pavel Emelianov <xemul@sw.ru> Signed-Off-By: Kirill Korotaev <dev@sw.ru> --- include/bc/beancounter.h | 2 include/bc/rsspages.h | 46 ++++++++ include/linux/mm.h | 5 include/linux/mm_types.h | 5 kernel/bc/beancounter.c | 2 kernel/bc/rsspages.c | 267 +++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 326 insertions(+), 1 deletion(-) --- ./include/bc/beancounter.h.ve9 2006-11-07 12:03:41.000000000 +0300 +++ ./include/bc/beancounter.h 2006-11-07 12:03:47.000000000 +0300 @@ -67,6 +67,8 @@ struct beancounter { bcid_t bc_id; struct hlist_node bc_hash; + spinlock_t bc_page_lock; + struct list_head bc_page_list; struct bc_resource_parm bc_parms[BC_RESOURCES]; }; --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./include/bc/rsspages.h 2006-11-07 12:03:57.000000000 +0300 @@ -0,0 +1,46 @@ +/* + * include/bc/rsspages.h + * + * Copyright (C) 2006 OpenVZ SWsoft Inc + * + */ + +#ifndef __BC_RSSPAGES_H_ +#define __BC_RSSPAGES_H_ + +#include <linux/compiler.h> + +struct page; +struct vm_area_struct; +struct page_beancounter; + +#ifdef CONFIG_BEANCOUNTERS +int __must_check bc_rsspage_prepare(struct page *p, + struct vm_area_struct *vma, struct page_beancounter **ppb); +void bc_rsspage_charge(struct page_beancounter *pb); +void bc_rsspage_release(struct page_beancounter *pb); +void bc_rsspage_uncharge(struct page_beancounter *pb); + +unsigned long bc_try_to_free_pages(struct beancounter *bc); +unsigned long bc_isolate_pages(unsigned long nr_to_scan, + struct beancounter *bc, struct list_head *dst, + int active, unsigned long *scanned); …
creator: dev
date: 2006-11-09T16:59:40-00:00

[PATCH 11/13] BC: physpages accounting (hooks)
type: item
description: Introduce calls to BC code over the kernel to add accounting of physical pages. Signed-Off-By: Pavel Emelianov <xemul@sw.ru> Signed-Off-By: Kirill Korotaev <dev@sw.ru> --- fs/exec.c | 15 +++- include/linux/rmap.h | 9 +- mm/fremap.c | 11 ++ mm/memory.c | 52 +++++++++++-- mm/migrate.c | 13 ++- mm/rmap.c | 34 +++++++-- mm/swapfile.c | 15 +++- mm/vmscan.c | 190 +++++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 310 insertions(+), 29 deletions(-) --- ./fs/exec.c.ve10 2006-11-07 11:56:20.000000000 +0300 +++ ./fs/exec.c 2006-11-07 11:57:23.000000000 +0300 @@ -51,6 +51,8 @@ #include <linux/cn_proc.h> #include <linux/audit.h> +#include <bc/rsspages.h> + #include <asm/uaccess.h> #include <asm/mmu_context.h> @@ -309,27 +311,34 @@ void install_arg_page(struct vm_area_str struct mm_struct *mm = vma->vm_mm; pte_t * pte; spinlock_t *ptl; + struct page_beancounter *pb; if (unlikely(anon_vma_prepare(vma))) goto out; + if (bc_rsspage_prepare(page, vma, &pb)) + goto out; + flush_dcache_page(page); pte = get_locked_pte(mm, address, &ptl); if (!pte) - goto out; + goto out_unch; if (!pte_none(*pte)) { pte_unmap_unlock(pte, ptl); - goto out; + goto out_unch; } inc_mm_counter(mm, anon_rss); lru_cache_add_active(page); set_pte_at(mm, address, pte, pte_mkdirty(pte_mkwrite(mk_pte( page, vma->vm_page_prot)))); - page_add_new_anon_rmap(page, vma, address); + page_add_new_anon_rmap(page, vma, address, pb); pte_unmap_unlock(pte, ptl); /* no need for flush_tlb */ return; + +out_unch: + bc_rsspage_release(pb); out: __free_page(page); force_sig(SIGKILL, current); --- ./include/linux/rmap.h.ve10 2006-11-07 11:56:20.000000000 +0300 +++ ./include/linux/rmap.h 2006-11-07 11:57:23.000000000 +0300 @@ -69,9 +69,12 @@ void __anon_vma_link(struct vm_area_stru /* * rmap interfaces called when adding or removing pte of page */ -void page_add_anon_rmap(struct page *, struct vm_area_struct *, unsigned long); -void page_add_new_anon_rmap(struct page *, struct vm_area_struct *, unsigned long); -v…
creator: dev
date: 2006-11-09T17:01:28-00:00

[PATCH 12/13] BC: numtasks accounting
type: item
description: Beautifull tasks accounting/limiting beancounter control. Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- include/linux/sched.h | 1 + kernel/bc/misc.c | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) --- ./include/linux/sched.h.bctasks 2006-11-03 17:47:38.000000000 +0300 +++ ./include/linux/sched.h 2006-11-03 17:51:57.000000000 +0300 @@ -1068,6 +1068,7 @@ int copy_beancounter(struct task_struct #endif #ifdef CONFIG_BEANCOUNTERS struct beancounter *exec_bc; + struct beancounter *task_bc; #endif }; --- ./kernel/bc/misc.c.bctasks 2006-11-03 17:47:38.000000000 +0300 +++ ./kernel/bc/misc.c 2006-11-03 17:51:57.000000000 +0300 @@ -17,6 +17,10 @@ int copy_beancounter(struct task_struct struct beancounter *bc; bc = parent->exec_bc; + if (bc_charge(bc, BC_NUMTASKS, 1, BC_LIMIT)) + return -ENOMEM; + + tsk->task_bc = bc_get(bc); tsk->exec_bc = bc_get(bc); return 0; } @@ -25,6 +29,10 @@ void free_beancounter(struct task_struct { struct beancounter *bc; + bc = tsk->task_bc; + bc_uncharge(bc, BC_NUMTASKS, 1); + bc_put(bc); + bc = tsk->exec_bc; bc_put(bc); } @@ -86,3 +94,26 @@ int bc_task_move(struct task_struct *tsk return err; } EXPORT_SYMBOL(bc_task_move); + +#define BC_NUMTASKS_BARRIER 128 +#define BC_NUMTASKS_LIMIT 128 + +static int bc_task_init(struct beancounter *bc, int i) +{ + bc_init_resource(&bc->bc_parms[BC_NUMTASKS], + BC_NUMTASKS_BARRIER, BC_NUMTASKS_LIMIT); + return 0; +} + +static struct bc_resource bc_task_resource = { + .bcr_name = "numtasks", + .bcr_init = bc_task_init, +}; + +static int __init bc_misc_init_resource(void) +{ + bc_register_resource(BC_NUMTASKS, &bc_task_resource); + return 0; +} + +__initcall(bc_misc_init_resource);
creator: dev
date: 2006-11-09T17:02:17-00:00

[PATCH 13/13] BC: numfiles accounting
type: item
description: Very simple beancounter for accounting and limiting container number of opened files. Signed-off-by: Pavel Emelianov <xemul@sw.ru> Signed-off-by: Kirill Korotaev <dev@sw.ru> --- fs/file_table.c | 12 +++++++++++- include/bc/misc.h | 27 +++++++++++++++++++++++++++ include/linux/fs.h | 3 +++ kernel/bc/misc.c | 41 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 1 deletion(-) --- ./fs/file_table.c.bcnumfiles 2006-11-09 11:29:11.000000000 +0300 +++ ./fs/file_table.c 2006-11-09 11:35:34.000000000 +0300 @@ -23,6 +23,8 @@ #include <linux/sysctl.h> #include <linux/percpu_counter.h> +#include <bc/misc.h> + #include <asm/atomic.h> /* sysctl tunables... */ @@ -44,6 +46,7 @@ static inline void file_free_rcu(struct static inline void file_free(struct file *f) { percpu_counter_dec(&nr_files); + bc_file_uncharge(f); call_rcu(&f->f_u.fu_rcuhead, file_free_rcu); } @@ -108,8 +111,11 @@ struct file *get_empty_filp(void) if (f == NULL) goto fail; - percpu_counter_inc(&nr_files); memset(f, 0, sizeof(*f)); + if (bc_file_charge(f)) + goto fail_charge; + + percpu_counter_inc(&nr_files); if (security_file_alloc(f)) goto fail_sec; @@ -136,6 +142,10 @@ fail_sec: file_free(f); fail: return NULL; + +fail_charge: + kmem_cache_free(filp_cachep, f); + return NULL; } EXPORT_SYMBOL(get_empty_filp); --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./include/bc/misc.h 2006-11-09 11:34:42.000000000 +0300 @@ -0,0 +1,27 @@ +/* + * include/bc/misc.h + * + * Copyright (C) 2006 OpenVZ SWsoft Inc + * + */ + +#ifndef __BC_MISC_H__ +#define __BC_MISC_H__ + +struct file; + +#ifdef CONFIG_BEANCOUNTERS +int __must_check bc_file_charge(struct file *); +void bc_file_uncharge(struct file *); +#else +static inline int __must_check bc_file_charge(struct file *f) +{ + return 0; +} + +static inline void bc_file_uncharge(struct file *f) +{ +} +#endif + +#endif --- ./include/linux/fs.h.bcnumfiles 2006-11-09 11:29:12.000000000 +0300 +++ ./include/linux/fs.h 2006-11-09 11:34:42.000000000 +030…
creator: dev
date: 2006-11-09T17:03:02-00:00

Re: [PATCH 6/13] BC: kmemsize accounting (core)
type: item
description: > +#define __GFP_BC ((__force gfp_t)0x80000u) /* Charge allocation with BC */ Please include the term "beancounter" in that comment. -- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.925.600.0401
creator: Paul Jackson
date: 2006-11-09T19:05:26-00:00

Re: [PATCH 1/13] BC: atomic_dec_and_lock_irqsave() helper
type: item
description: Hello Kirill, Hello Pavel, Kirill Korotaev wrote: > Oleg Nesterov noticed to me that the construction like > (used in beancounter patches and free_uid()): > > local_irq_save(flags); > if (atomic_dec_and_lock(&refcnt, &lock)) > ... > > is not that good for preemtible kernels, since with preemption > spin_lock() can schedule() to reduce latency. However, it won't schedule > if interrupts are disabled. > > So this patch introduces atomic_dec_and_lock_irqsave() as a logical > counterpart to atomic_dec_and_lock(). You should probably send that one independently from the BC patchset. C.
creator: Cedric Le Goater
date: 2006-11-10T15:19:37-00:00

Re: [PATCH 1/13] BC: atomic_dec_and_lock_irqsave() helper
type: item
description: > Hello Kirill, Hello Pavel, > > Kirill Korotaev wrote: > >>Oleg Nesterov noticed to me that the construction like >>(used in beancounter patches and free_uid()): >> >> local_irq_save(flags); >> if (atomic_dec_and_lock(&refcnt, &lock)) >> ... >> >>is not that good for preemtible kernels, since with preemption >>spin_lock() can schedule() to reduce latency. However, it won't schedule >>if interrupts are disabled. >> >>So this patch introduces atomic_dec_and_lock_irqsave() as a logical >>counterpart to atomic_dec_and_lock(). > > > You should probably send that one independently from the BC > patchset. Maybe, but BCs are the only user of this so far... Thanks, Kirill
creator: dev
date: 2006-11-10T16:40:31-00:00

Re: [PATCH 6/13] BC: kmemsize accounting (core)
type: item
description: Hi, On 11/9/06, Kirill Korotaev <dev@sw.ru> wrote: > +#ifdef CONFIG_BEANCOUNTERS > +#define BC_EXTRASIZE sizeof(struct beancounter *) > +static inline size_t slab_mgmt_size_noalign(int flags, size_t nr_objs) > +{ > + size_t size; > + > + size = slab_mgmt_size_raw(nr_objs); > + if (flags & SLAB_BC) > + size = ALIGN(size, BC_EXTRASIZE) + nr_objs * BC_EXTRASIZE; > + return size; Why do we want to track each allocated _object_ in the slab? Isn't tracking pages enough?
creator: Pekka Enberg
date: 2006-11-10T22:46:36-00:00

Re: [PATCH 6/13] BC: kmemsize accounting (core)
type: item
description: On 11/9/06, Kirill Korotaev <dev@sw.ru> wrote: > +#ifdef CONFIG_BEANCOUNTERS > +#define BC_EXTRASIZE sizeof(struct beancounter *) Would much prefer you put all beancounter stuff into one #ifdef block to avoid clutter. > @@ -2579,14 +2635,14 @@ static struct slab *alloc_slabmgmt(struc > slabp->colouroff = colour_off; > slabp->s_mem = objp + colour_off; > slabp->nodeid = nodeid; > +#ifdef CONFIG_BEANCOUNTERS > + if (cachep->flags & SLAB_BC) > + memset(slab_bc_ptrs(cachep, slabp), 0, > + cachep->num * BC_EXTRASIZE); > +#endif No #ifdef within functions, please, but instead, make it an static inline function.
creator: Pekka Enberg
date: 2006-11-10T22:50:40-00:00

Re: [PATCH 6/13] BC: kmemsize accounting (core)
type: item
description: Hi! > --- /dev/null 2006-07-18 14:52:43.075228448 +0400 > +++ ./include/bc/kmem.h 2006-11-03 15:48:26.000000000 +0300 > @@ -0,0 +1,48 @@ > +/* > + * include/bc/kmem.h > + * > + * Copyright (C) 2006 OpenVZ SWsoft Inc > + * > + */ GPL would be nice, as would be email address of someone who worked on this file. > --- /dev/null 2006-07-18 14:52:43.075228448 +0400 > +++ ./kernel/bc/kmem.c 2006-11-03 15:48:26.000000000 +0300 > @@ -0,0 +1,112 @@ > +/* > + * kernel/bc/kmem.c > + * > + * Copyright (C) 2006 OpenVZ SWsoft Inc > + * > + */ Same here. > +void bc_slab_uncharge(kmem_cache_t *cachep, void *objp) > +{ > + unsigned int size; > + struct beancounter *bc, **slab_bcp; > + > + slab_bcp = kmem_cache_bcp(cachep, objp); > + if (*slab_bcp == NULL) > + return; > + > + bc = *slab_bcp; You can do this before if() and spare a dereference. Pavel -- Thanks for all the (sleeping) penguins.
creator: Pavel Machek
date: 2006-11-11T05:50:50-00:00

Re: [PATCH 6/13] BC: kmemsize accounting (core)
type: item
description: Pekka Enberg wrote: > Hi, > > On 11/9/06, Kirill Korotaev <dev@sw.ru> wrote: >> +#ifdef CONFIG_BEANCOUNTERS >> +#define BC_EXTRASIZE sizeof(struct beancounter *) >> +static inline size_t slab_mgmt_size_noalign(int flags, size_t nr_objs) >> +{ >> + size_t size; >> + >> + size = slab_mgmt_size_raw(nr_objs); >> + if (flags & SLAB_BC) >> + size = ALIGN(size, BC_EXTRASIZE) + nr_objs * >> BC_EXTRASIZE; >> + return size; > > Why do we want to track each allocated _object_ in the slab? Isn't > tracking pages enough? No. One page may contain objects allocated in different beancounters.
creator: Pavel Emelianov
date: 2006-11-13T12:13:41-00:00

Re: BC: resource beancounters (v6) (with userpages reclamation + configfs)
type: item
description: On Thu, Nov 09, 2006 at 07:49:28PM +0300, Kirill Korotaev wrote: > MAJOR CHANGES in v6 (see details below): > - configfs interface instead of syscalls (as wanted by CKRM people...) > - added numfiles resource accounting > - added numtasks resource accounting > > numfiles and numtasks controllers demonstrate how > clean and simple BC interface is. > > Patch set is applicable to 2.6.19-rc5-mm1 > > ----------------------------------------------------- > > Resource BeanCounters (BC). > > BC allows to account and control consumption > of kernel resources used by *group* of processes > (users, containers, ...). > > Draft BC 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 and not reclaimable should be accounted and > limited for DoS protection. > e.g. page tables, task structs, vmas etc. kernel memory is not accounted on a linux system right now, and user can probably quite easily use that for DoS ... shouldn't that become a separate user limit in the first place? > - 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). how much overhead does this add to the memory/page management? do we really want to account shared pages at all, if so, why is accounting a fraction to each group the 'proper' way to do so? (would expect to account it to all of them equally) IMHO it would be sufficient to extend existing page accounting to a 'memory' namespace isntead of adding another complex mechanism like the beancounters > - 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, socke…
creator: Herbert Poetzl
date: 2006-11-19T19:41:28-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: On 11/9/06, Kirill Korotaev <dev@sw.ru> wrote: > + > +int bc_task_move(int pid, struct beancounter *bc, int whole) > +{ ... > + > + down_write(&mm->mmap_sem); > + err = stop_machine_run(do_set_bcid, &data, NR_CPUS); > + up_write(&mm->mmap_sem); Isn't this a little heavyweight for moving a task into/between beancounters? Paul
creator: Paul Menage
date: 2006-11-23T07:48:56-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: Paul Menage wrote: > On 11/9/06, Kirill Korotaev <dev@sw.ru> wrote: >> + >> +int bc_task_move(int pid, struct beancounter *bc, int whole) >> +{ > > ... > >> + >> + down_write(&mm->mmap_sem); >> + err = stop_machine_run(do_set_bcid, &data, NR_CPUS); >> + up_write(&mm->mmap_sem); > > Isn't this a little heavyweight for moving a task into/between > beancounters? It's a main reason we were against moving arbitrary task. We need to track the situation when we change beancounter on task that is currently handles an interrupt and thus set a temporary BC as exec one. I see no other way that keeps pair set_exec_bc()/get_exec_bc() lock-less. The problem is even larger than I've described. set_exec_bc() is used widely in OpenVZ beancounters to set temporary context e.g. for skb handling. Thus we need some safe way to "catch" the task in a "safe" place. In OpenVZ we solve this by moving only current into beancounter. In this patch set we have to move arbitrary task and thus - such complication. I repeat - we can do this w/o stop_machine, but this would require locking in set_exec_bc()/get_exec_bc() but it's too bad. Moving tasks happens rarely but setting context is a very common operation (e.g. in each interrupt). We can do the following: if (tsk == current) /* fast way */ tsk->exec_bc = bc; else /* slow way */ stop_machine_run(...); What do you think? > Paul > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ >
creator: Pavel Emelianov
date: 2006-11-23T08:35:10-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: On 11/23/06, Pavel Emelianov <xemul@openvz.org> wrote: > > We can do the following: > > if (tsk == current) > /* fast way */ > tsk->exec_bc = bc; > else > /* slow way */ > stop_machine_run(...); > > What do you think? How about having two pointers per task: - exec_bc, which is the one used for charging - real_bc, which is the task's actual beancounter at the start of irq, do current->exec_bc = &init_bc; at the end of irq, do current->exec_bc = current->real_bc; When moving a task to a different bc do: task->real_bc = new_bc; atomic_cmpxchg(&task->exec_bc, old_bc, new_bc); (with appropriate memory barriers). So if the task is in an irq with a modified exec_bc pointer, we do nothing, otherwise we update exec_bc to point to the new real_bc. Paul
creator: Paul Menage
date: 2006-11-23T08:53:51-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: Paul Menage wrote: > On 11/23/06, Pavel Emelianov <xemul@openvz.org> wrote: >> >> We can do the following: >> >> if (tsk == current) >> /* fast way */ >> tsk->exec_bc = bc; >> else >> /* slow way */ >> stop_machine_run(...); >> >> What do you think? > > How about having two pointers per task: > > - exec_bc, which is the one used for charging > - real_bc, which is the task's actual beancounter > > at the start of irq, do > > current->exec_bc = &init_bc; > > at the end of irq, do > > current->exec_bc = current->real_bc; > > When moving a task to a different bc do: > > task->real_bc = new_bc; > atomic_cmpxchg(&task->exec_bc, old_bc, new_bc); You mean moving is like this: old_bc = task->real_bc; task->real_bc = new_bc; cmpxchg(&tsk->exec_bc, old_bc, new_bc); ? Then this won't work: Initialisation: current->exec_bc = init_bc; current->real_bc = init_bc; ... IRQ: current->exec_bc = init_bc; ... old_bc = tsk->real_bc; /* init_bc */ tsk->real_bc = bc1; cx(tsk->exec_bc, init_bc, bc1); /* ok */ ... Here at the middle of an interrupt we have bc1 set as exec_bc on task which IS wrong! ... current->exec_bc = current->real_bc; We need some way to be sure that task isn't running at the moment we change it's beancounter. Otherwise we're risking that we'll spoil some temporary context. > (with appropriate memory barriers). So if the task is in an irq with a > modified exec_bc pointer, we do nothing, otherwise we update exec_bc > to point to the new real_bc. > > Paul
creator: Pavel Emelianov
date: 2006-11-23T09:20:29-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: On 11/23/06, Pavel Emelianov <xemul@openvz.org> wrote: > You mean moving is like this: > > old_bc = task->real_bc; > task->real_bc = new_bc; > cmpxchg(&tsk->exec_bc, old_bc, new_bc); > > ? Then this won't work: > > Initialisation: > current->exec_bc = init_bc; > current->real_bc = init_bc; > ... > IRQ: > current->exec_bc = init_bc; > ... > old_bc = tsk->real_bc; /* init_bc */ > tsk->real_bc = bc1; > cx(tsk->exec_bc, init_bc, bc1); /* ok */ > ... > Here at the middle of an interrupt > we have bc1 set as exec_bc on task > which IS wrong! You could get round that by having a separate "irq_bc" that's never valid for a task not in an interrupt. Paul
creator: Paul Menage
date: 2006-11-23T09:31:47-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: Paul Menage wrote: > On 11/23/06, Pavel Emelianov <xemul@openvz.org> wrote: >> You mean moving is like this: >> >> old_bc = task->real_bc; >> task->real_bc = new_bc; >> cmpxchg(&tsk->exec_bc, old_bc, new_bc); >> >> ? Then this won't work: >> >> Initialisation: >> current->exec_bc = init_bc; >> current->real_bc = init_bc; >> ... >> IRQ: >> current->exec_bc = init_bc; >> ... >> old_bc = tsk->real_bc; /* init_bc */ >> tsk->real_bc = bc1; >> cx(tsk->exec_bc, init_bc, bc1); /* ok */ >> ... >> Here at the middle of an interrupt >> we have bc1 set as exec_bc on task >> which IS wrong! > > You could get round that by having a separate "irq_bc" that's never > valid for a task not in an interrupt. No no no. This is not what is needed. You see, we do have to set exec_bc as temporary (and atomic) context. Having temporary context is 1. flexible 2. needed by beancounters' network accountig. We have to track this particular scenario. Moreover making get_exec_bc() as if (in_interrupt()) return &irq_bc; else return current->exec_bc; is awful. It must me simple and stupid to allow us making temporary contexts in any place of code. Maybe we can make smth similar to wait_task_inactive and change it's beancounter before unlocking the runqueue? > Paul >
creator: Pavel Emelianov
date: 2006-11-23T09:56:00-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: On 11/23/06, Pavel Emelianov <xemul@openvz.org> wrote: > Paul Menage wrote: > > On 11/23/06, Pavel Emelianov <xemul@openvz.org> wrote: > >> You mean moving is like this: > >> > >> old_bc = task->real_bc; > >> task->real_bc = new_bc; > >> cmpxchg(&tsk->exec_bc, old_bc, new_bc); > >> > >> ? Then this won't work: > >> > >> Initialisation: > >> current->exec_bc = init_bc; > >> current->real_bc = init_bc; > >> ... > >> IRQ: > >> current->exec_bc = init_bc; > >> ... > >> old_bc = tsk->real_bc; /* init_bc */ > >> tsk->real_bc = bc1; > >> cx(tsk->exec_bc, init_bc, bc1); /* ok */ > >> ... > >> Here at the middle of an interrupt > >> we have bc1 set as exec_bc on task > >> which IS wrong! > > > > You could get round that by having a separate "irq_bc" that's never > > valid for a task not in an interrupt. > > No no no. This is not what is needed. You see, we do have to > set exec_bc as temporary (and atomic) context. Having temporary > context is 1. flexible 2. needed by beancounters' network accountig. I don't see why having an irq_bc wouldn't solve this. At the start of the interrupt handler, set current->exec_bc to &irq_bc; at the end set it to current->real_bc; use the cmpxchg() that I suggested to ensure that you never update task->exec_bc from another task if it's not equal to task->real_bc; use RCU to ensure that a beancounter is never freed while someone might be accessing it. > > Maybe we can make smth similar to wait_task_inactive and change > it's beancounter before unlocking the runqueue? That could work too. Paul
creator: Paul Menage
date: 2006-11-23T10:18:46-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: Paul Menage wrote: > On 11/23/06, Pavel Emelianov <xemul@openvz.org> wrote: >> Paul Menage wrote: >> > On 11/23/06, Pavel Emelianov <xemul@openvz.org> wrote: >> >> You mean moving is like this: >> >> >> >> old_bc = task->real_bc; >> >> task->real_bc = new_bc; >> >> cmpxchg(&tsk->exec_bc, old_bc, new_bc); >> >> >> >> ? Then this won't work: >> >> >> >> Initialisation: >> >> current->exec_bc = init_bc; >> >> current->real_bc = init_bc; >> >> ... >> >> IRQ: >> >> current->exec_bc = init_bc; >> >> ... >> >> old_bc = tsk->real_bc; /* init_bc */ >> >> tsk->real_bc = bc1; >> >> cx(tsk->exec_bc, init_bc, bc1); /* ok */ >> >> ... >> >> Here at the middle of an interrupt >> >> we have bc1 set as exec_bc on task >> >> which IS wrong! >> > >> > You could get round that by having a separate "irq_bc" that's never >> > valid for a task not in an interrupt. >> >> No no no. This is not what is needed. You see, we do have to >> set exec_bc as temporary (and atomic) context. Having temporary >> context is 1. flexible 2. needed by beancounters' network accountig. > > I don't see why having an irq_bc wouldn't solve this. At the start of > the interrupt handler, set current->exec_bc to &irq_bc; at the end set > it to current->real_bc; use the cmpxchg() that I suggested to ensure > that you never update task->exec_bc from another task if it's not > equal to task->real_bc; use RCU to ensure that a beancounter is never > freed while someone might be accessing it. Oh, I see. I just didn't get your idea. This will work, but 1. we separate interrupt accounting from all the others' 2. for interrupts only. In case we want to set init_bc as temporary context all will be broken... We need some generic solution independent from what exactly is set as temporary exec_bc. >> >> Maybe we can make smth similar to wait_task_inactive and change >> it's beancounter before unlocking the runqueue? > > That could work too. Could work, but whether everyone will like such intrusion... I agree that stop_machine …
creator: Pavel Emelianov
date: 2006-11-23T10:45:23-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: I've got it! That's what will work: struct task_struct { ... struct beancounter *exec_bc; struct beancounter *tmp_exec_bc; /* is set to NULL on * tsk creation */ }; struct beancounter get_exec_bc(void) { if (current->tmp_exec_bc) return current->tmp_exec_bc; return rcu_dereference(current->exec_bc); } struct beancounter set_tmp_exec_bc(struct beancounter *new) { struct beancounter *old; old = current->tmp_exec_bc; current->tmp_exec_bc = new; return old; } void reset_tmp_exec_bc(struct beancounter *expected_old) { BUG_ON(current->tmp_exec_bc != expected_old); current->tmp_exec_bc = NULL; } void move_task(struct task_struct *tsk, struct beancounter *bc) { struct beancounter *old; mutex_lock(&tsk_move_mutex); old = tsk->exec_bc; get_bc(bc); rcu_assign_pointer(current->exec_bc, bc); syncronize_rcu(); mutex_unlock(&tsk_move_mutex); bc_put(old); } I will implement this in the next beancounter patches. Thanks for discussion :)
creator: Pavel Emelianov
date: 2006-11-24T10:10:33-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: On 11/24/06, Pavel Emelianov <xemul@openvz.org> wrote: > I've got it! That's what will work: > > struct task_struct { > ... > struct beancounter *exec_bc; > struct beancounter *tmp_exec_bc; /* is set to NULL on > * tsk creation > */ > }; > > struct beancounter get_exec_bc(void) > { > if (current->tmp_exec_bc) > return current->tmp_exec_bc; > return rcu_dereference(current->exec_bc); > } Don't forget that this means all callers need to be in an rcu_read_lock() section. > > I will implement this in the next beancounter patches. This is looking remarkably like the mechanism in use for my generic containers patches (inherited from Paul Jackson's cpusets code). In the last set of patches that I posted on Wednesday night, I included the example of the beancounters core and numfiles counter implemented on top of the generic containers - basically pulling out the hash table, refcounting and most of the configfs code (since that's handled by the generic containers), and moving the attribute management configfs code to the use the containerfs filesystem interface instead. The rest is pretty much unchanged. I think you could continue to use the tmp_exec_bc idea with this, and have get_exec_bc() use the tmp_exec_bc if it existed, or else get the bc pointer via the container system. I'd appreciate any feedback you had on that approach. Paul
creator: Paul Menage
date: 2006-11-25T00:09:03-00:00

Re: [ckrm-tech] [PATCH 4/13] BC: context handling
type: item
description: Paul Menage wrote: > On 11/24/06, Pavel Emelianov <xemul@openvz.org> wrote: >> I've got it! That's what will work: >> >> struct task_struct { >> ... >> struct beancounter *exec_bc; >> struct beancounter *tmp_exec_bc; /* is set to NULL on >> * tsk creation >> */ >> }; >> >> struct beancounter get_exec_bc(void) >> { >> if (current->tmp_exec_bc) >> return current->tmp_exec_bc; >> return rcu_dereference(current->exec_bc); >> } > > Don't forget that this means all callers need to be in an > rcu_read_lock() section. Sure. This is done for these particular cases. >> >> I will implement this in the next beancounter patches. > > This is looking remarkably like the mechanism in use for my generic > containers patches (inherited from Paul Jackson's cpusets code). In > the last set of patches that I posted on Wednesday night, I included > the example of the beancounters core and numfiles counter implemented > on top of the generic containers - basically pulling out the hash > table, refcounting and most of the configfs code (since that's handled > by the generic containers), and moving the attribute management > configfs code to the use the containerfs filesystem interface instead. > The rest is pretty much unchanged. > > I think you could continue to use the tmp_exec_bc idea with this, and > have get_exec_bc() use the tmp_exec_bc if it existed, or else get the > bc pointer via the container system. I'll look through your patches this week and send my opinion. > I'd appreciate any feedback you had on that approach. > > Paul
creator: Pavel Emelianov
date: 2006-11-27T08:27:41-00:00