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

[PATCH] BC: resource beancounters (v3)
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-atomic-dec-and-lock-irqsave.patch introduce atomic_dec_and_lock_irqsave() 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 ma…
creator: dev
date: 2006-08-29T14:31:56-00:00

[PATCH 1/7] introduce atomic_dec_and_lock_irqsave()
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.dlirq 2006-08-28 10:17:35.000000000 +0400 +++ ./include/linux/spinlock.h 2006-08-28 11:22:37.000000000 +0400 @@ -266,6 +266,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.dlirq 2006-07-10 12:39:20.000000000 +0400 +++ ./kernel/user.c 2006-08-28 11:08:56.000000000 +0400 @@ -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.dlirq 2006-04-21 11:59:36.000000000 +0400 +++ ./lib/dec_and_lock.c 2006-08-28 11:22:08.000000000 +0400 @@ -33,3 …
creator: dev
date: 2006-08-29T14:47:35-00:00

[PATCH 2/7] 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> --- init/Kconfig | 2 ++ kernel/bc/Kconfig | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+) --- ./init/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 +++ ./init/Kconfig 2006-07-28 14:10:41.000000000 +0400 @@ -222,6 +222,8 @@ source "crypto/Kconfig" Say N if unsure. +source "kernel/bc/Kconfig" + config SYSCTL bool --- ./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 @@ +# +# Resource beancounters (BC) +# +# Copyright (C) 2006 OpenVZ. SWsoft Inc + +menu "User resources" + +config BEANCOUNTERS + bool "Enable resource accounting/control" + default n + help + When Y this option provides accounting and allows to configure + 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 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. + +endmenu
creator: dev
date: 2006-08-29T14:48:19-00:00

[PATCH 3/7] 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 | 150 +++++++++++++++++++++++++++ include/linux/types.h | 16 ++ init/main.c | 4 kernel/Makefile | 1 kernel/bc/Makefile | 7 + kernel/bc/beancounter.c | 256 +++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 434 insertions(+) --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./include/bc/beancounter.h 2006-08-28 12:47:52.000000000 +0400 @@ -0,0 +1,150 @@ +/* + * 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/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…
creator: dev
date: 2006-08-29T14:50:18-00:00

[PATCH 4/7] 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/bc/task.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sched.h | 5 ++++ kernel/bc/Makefile | 1 kernel/bc/beancounter.c | 3 ++ kernel/bc/misc.c | 32 ++++++++++++++++++++++++++ kernel/fork.c | 17 ++++++++++++- kernel/irq/handle.c | 9 +++++++ kernel/softirq.c | 8 ++++++ 8 files changed, 132 insertions(+), 2 deletions(-) --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./include/bc/task.h 2006-08-28 12:54:48.000000000 +0400 @@ -0,0 +1,59 @@ +/* + * include/bc/task.h + * + * Copyright (C) 2006 OpenVZ. SWsoft Inc + * + */ + +#ifndef __BC_TASK_H_ +#define __BC_TASK_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 { \ + struct task_beancounter *tbc; \ + tbc = &current->task_bc; \ + BUG_ON(tbc->exec_bc != exp); \ + tbc->exec_bc = old; \ + } while (0) + +int __must_check bc_task_charge(struct task_struct *parent, + struct task_struct *new); +void bc_task_uncharge(struct task_struct *tsk); + +#else + +#define get_exec_bc() (NULL) +#define set_exec_bc(new) (NULL) +#define reset_exec_bc(new, exp) do { } while (0) + +static inline __must_check int bc_task_charge(struct task_struct *parent, + struct task_struct *new) +{ + return 0; +} + +static inline void bc_task_uncharge(struct task_struct *tsk) +{ …
creator: dev
date: 2006-08-29T14:51:35-00:00

[PATCH 5/7] 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 <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/bc/Makefile | 1 kernel/bc/sys.c | 120 +++++++++++++++++++++++++++++++++++++++ kernel/sys_ni.c | 6 + 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 d…
creator: dev
date: 2006-08-29T14:52:39-00:00

[PATCH 6/7] BC: kernel memory (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/beancounter.h | 4 + include/bc/kmem.h | 46 +++++++++++++++++ include/linux/gfp.h | 8 ++- include/linux/mm.h | 6 ++ include/linux/slab.h | 4 + include/linux/vmalloc.h | 1 kernel/bc/Makefile | 1 kernel/bc/beancounter.c | 3 + kernel/bc/kmem.c | 85 +++++++++++++++++++++++++++++++++ mm/mempool.c | 2 mm/page_alloc.c | 11 ++++ mm/slab.c | 121 ++++++++++++++++++++++++++++++++++++++--------- mm/vmalloc.c | 6 ++ 13 files changed, 273 insertions(+), 25 deletions(-) --- ./include/bc/beancounter.h.bckmem 2006-08-28 12:47:52.000000000 +0400 +++ ./include/bc/beancounter.h 2006-08-28 12:59:28.000000000 +0400 @@ -12,7 +12,9 @@ * Resource list. */ -#define BC_RESOURCES 0 +#define BC_KMEMSIZE 0 + +#define BC_RESOURCES 1 struct bc_resource_parm { unsigned long barrier; /* A barrier over which resource allocations --- /dev/null 2006-07-18 14:52:43.075228448 +0400 +++ ./include/bc/kmem.h 2006-08-28 13:00:43.000000000 +0400 @@ -0,0 +1,46 @@ +/* + * include/bc/kmem.h + * + * Copyright (C) 2006 OpenVZ. SWsoft Inc + * + */ + +#ifndef __BC_KMEM_H_ +#define __BC_KMEM_H_ + +/* + * BC_KMEMSIZE accounting + */ + +struct mm_struct; +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); …
creator: dev
date: 2006-08-29T14:55:19-00:00

[PATCH 7/7] BC: kernel memory (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.bckmemc 2006-08-28 12:20:10.000000000 +0400 +++ ./arch/i386/kernel/ldt.c 2006-08-28 13:05:46.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.bckmemc 2006-08-28 12:20:10.000000000 +0400 +++ ./arch/i386/mm/init.c 2006-08-28 13:05:46.000000000 +0400 @@ -709,7 +709,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) @@ -718,7 +718,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-29T14:56:01-00:00

Re: [PATCH 6/7] BC: kernel memory (core)
type: item
description: Kirill Korotaev wrote: > 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. > <snip> > +#define __GFP_BC_LIMIT ((__force gfp_t)0x100000u) /* Charge against BC > limit */ > What's _GFP_BC_LIMIT for, could you add the description for that flag? The comment is not very clear > +#ifdef CONFIG_BEANCOUNTERS > + union { > + struct beancounter *page_bc; > + } bc; > +#endif > }; > > +#define page_bc(page) ((page)->bc.page_bc) Minor comment - page->(bc).page_bc has too many repititions of page and bc - see the Practice of Programming by Kernighan and Pike I missed the part of why you wanted to have a union (in struct page for bc)? > const char *bc_rnames[] = { > + "kmemsize", /* 0 */ > }; > > static struct hlist_head bc_hash[BC_HASH_SIZE]; > @@ -221,6 +222,8 @@ static void init_beancounter_syslimits(s > { > int k; > > + bc->bc_parms[BC_KMEMSIZE].limit = 32 * 1024 * 1024; > + Can't this be configurable CONFIG_XXX or a #defined constant? > --- ./mm/mempool.c.bckmem 2006-04-21 11:59:36.000000000 +0400 > +++ ./mm/mempool.c 2006-08-28 12:59:28.000000000 +0400 > @@ -119,6 +119,7 @@ int mempool_resize(mempool_t *pool, int unsigned > long flags; > > BUG_ON(new_min_nr <= 0); > + gfp_mask &= ~__GFP_BC; > > spin_lock_irqsave(&pool->lock, flags); > if (new_min_nr <= pool->min_nr) { > @@ -212,6 +213,7 @@ void * mempool_alloc(mempool_t *pool, gf > gfp_mask |= __GFP_NOMEMALLOC; /* don't allocate emergency > reserves */ > gfp_mask |= __GFP_NORE…
creator: Balbir Singh
date: 2006-08-29T18:41:46-00:00

Re: [ckrm-tech] [PATCH 6/7] BC: kernel memory (core)
type: item
description: On Tue, 2006-08-29 at 18:58 +0400, Kirill Korotaev wrote: > @@ -274,8 +274,14 @@ struct page { > unsigned int gfp_mask; > unsigned long trace[8]; > #endif > +#ifdef CONFIG_BEANCOUNTERS > + union { > + struct beancounter *page_bc; > + } bc; > +#endif > }; I know you're probably saving this union for when you put some userspace stuff in here or something. But, for now, it would probably be best just to leave it as a plain struct, or even an anonymous union. You probably had to use gcc 2 when this was written and couldn't use anonymous unions, right? -- Dave
creator: Dave Hansen
date: 2006-08-29T20:29:42-00:00

Re: [PATCH 2/7] BC: kconfig
type: item
description: On Tue, 29 Aug 2006 18:51:20 +0400 Kirill Korotaev wrote: > Add kernel/bc/Kconfig file with BC options and > include it into arch Kconfigs > > --- > init/Kconfig | 2 ++ > kernel/bc/Kconfig | 25 +++++++++++++++++++++++++ > 2 files changed, 27 insertions(+) > > --- ./init/Kconfig.bckm 2006-07-10 12:39:10.000000000 +0400 > +++ ./init/Kconfig 2006-07-28 14:10:41.000000000 +0400 > @@ -222,6 +222,8 @@ source "crypto/Kconfig" > > Say N if unsure. > > +source "kernel/bc/Kconfig" > + > config SYSCTL > bool > > --- ./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 @@ > +# > +# Resource beancounters (BC) > +# > +# Copyright (C) 2006 OpenVZ. SWsoft Inc > + > +menu "User resources" > + > +config BEANCOUNTERS > + bool "Enable resource accounting/control" > + default n > + help > + When Y this option provides accounting and allows to configure "allows" needs an object after it, like "you" or "one". > + 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 an accidental drop "an" > + misbehavior of processes or malicious activity aiming to ``kill'' > + the system. It's worth to mention that resource limits configured s/to mention/mentioning/ > + by setrlimit(2) do not give an acceptable level of protection > + because they cover only small fraction of resources and work on a "only a small fraction" > + per-process basis. Per-process accounting doesn't prevent malicious > + users from spawning a lot of resource-consuming processes. > + > +endmenu and there are several lines that end with <space> (don't do that :). --- ~Randy
creator: rdunlap
date: 2006-08-30T04:02:53-00:00

Re: [PATCH 1/7] introduce atomic_dec_and_lock_irqsave()
type: item
description: Hi, On Tue, 29 Aug 2006, Kirill Korotaev wrote: > --- ./kernel/user.c.dlirq 2006-07-10 12:39:20.000000000 +0400 > +++ ./kernel/user.c 2006-08-28 11:08:56.000000000 +0400 > @@ -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); > } > } Why does this need protection against interrupts? bye, Roman
creator: Roman Zippel
date: 2006-08-30T09:59:48-00:00

Re: [PATCH 1/7] introduce atomic_dec_and_lock_irqsave()
type: item
description: Hi, On Wed, 30 Aug 2006, Oleg Nesterov wrote: > > Why does this need protection against interrupts? > > uidhash_lock can be taken from irq context. For example, delayed_put_task_struct() > does __put_task_struct()->free_uid(). AFAICT it's called via rcu, does that mean anything released via rcu has to be protected against interrupts? bye, Roman
creator: Roman Zippel
date: 2006-08-30T10:51:28-00:00

Re: [PATCH 1/7] introduce atomic_dec_and_lock_irqsave()
type: item
description: On 08/30, Roman Zippel wrote: > Hi, > > On Wed, 30 Aug 2006, Oleg Nesterov wrote: > > > > Why does this need protection against interrupts? > > > > uidhash_lock can be taken from irq context. For example, delayed_put_task_struct() > > does __put_task_struct()->free_uid(). > > AFAICT it's called via rcu, does that mean anything released via rcu has > to be protected against interrupts? RCU means softirq, probably we can use spin_lock_bh() to protect against deadlock. But free_uid() may be called with irqs disabled, we can't use local_bh_enable() in such a case. Oleg.
creator: Oleg Nesterov
date: 2006-08-30T11:27:53-00:00

Re: [PATCH 1/7] introduce atomic_dec_and_lock_irqsave()
type: item
description: Hi, On Wed, 30 Aug 2006, Oleg Nesterov wrote: > > AFAICT it's called via rcu, does that mean anything released via rcu has > > to be protected against interrupts? > > RCU means softirq, probably we can use spin_lock_bh() to protect against deadlock. > But free_uid() may be called with irqs disabled, we can't use local_bh_enable() > in such a case. Eek, I wasn't really aware of it and this would really suck. We should move things out of the interrupt context and not into it. :( I would call it a bug in the rcu system. bye, Roman
creator: Roman Zippel
date: 2006-08-30T11:54:51-00:00

Re: [PATCH 1/7] introduce atomic_dec_and_lock_irqsave()
type: item
description: On 08/30, Roman Zippel wrote: > Hi, > > On Tue, 29 Aug 2006, Kirill Korotaev wrote: > > > --- ./kernel/user.c.dlirq 2006-07-10 12:39:20.000000000 +0400 > > +++ ./kernel/user.c 2006-08-28 11:08:56.000000000 +0400 > > @@ -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); > > } > > } > > Why does this need protection against interrupts? uidhash_lock can be taken from irq context. For example, delayed_put_task_struct() does __put_task_struct()->free_uid(). Oleg.
creator: Oleg Nesterov
date: 2006-08-30T12:01:38-00:00

Re: [PATCH 1/7] introduce atomic_dec_and_lock_irqsave()
type: item
description: On Wed, Aug 30, 2006 at 12:51:28PM +0200, Roman Zippel wrote: > Hi, > > On Wed, 30 Aug 2006, Oleg Nesterov wrote: > > > > Why does this need protection against interrupts? > > > > uidhash_lock can be taken from irq context. For example, delayed_put_task_struct() > > does __put_task_struct()->free_uid(). > > AFAICT it's called via rcu, does that mean anything released via rcu has > to be protected against interrupts? No. You need protection only if you have are using some data that can also be used by the RCU callback. For example, if your RCU callback just calls kfree(), you don't have to do a spin_lock_bh(). Thanks Dipankar
creator: Dipankar Sarma
date: 2006-08-30T16:58:51-00:00

Re: [PATCH 1/7] introduce atomic_dec_and_lock_irqsave()
type: item
description: Hi, On Wed, 30 Aug 2006, Dipankar Sarma wrote: > > > uidhash_lock can be taken from irq context. For example, delayed_put_task_struct() > > > does __put_task_struct()->free_uid(). > > > > AFAICT it's called via rcu, does that mean anything released via rcu has > > to be protected against interrupts? > > No. You need protection only if you have are using some > data that can also be used by the RCU callback. For example, > if your RCU callback just calls kfree(), you don't have to > do a spin_lock_bh(). In this case kfree() does its own interrupt synchronization. I didn't realize before that rcu had this (IMO serious) limitation. I think there should be two call_rcu() variants, one that queues the callback in a soft irq and a second which queues it in a thread context. bye, Roman
creator: Roman Zippel
date: 2006-08-30T17:25:07-00:00

Re: [ckrm-tech] [PATCH 3/7] BC: beancounters core (API)
type: item
description: On Tue, 2006-08-29 at 18:53 +0400, Kirill Korotaev 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 > > Signed-off-by: Pavel Emelianov <xemul@sw.ru> > Signed-off-by: Kirill Korotaev <dev@sw.ru> > > --- > > include/bc/beancounter.h | 150 +++++++++++++++++++++++++++ > include/linux/types.h | 16 ++ > init/main.c | 4 > kernel/Makefile | 1 > kernel/bc/Makefile | 7 + > kernel/bc/beancounter.c | 256 +++++++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 434 insertions(+) > > --- /dev/null 2006-07-18 14:52:43.075228448 +0400 > +++ ./include/bc/beancounter.h 2006-08-28 12:47:52.000000000 +0400 > @@ -0,0 +1,150 @@ > +/* > + * 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/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 …
creator: Chandra Seetharaman
date: 2006-08-30T18:58:34-00:00

Re: [ckrm-tech] [PATCH 4/7] BC: context inheriting and changing
type: item
description: On Tue, 2006-08-29 at 18:54 +0400, Kirill Korotaev wrote: > 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/bc/task.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/linux/sched.h | 5 ++++ > kernel/bc/Makefile | 1 > kernel/bc/beancounter.c | 3 ++ > kernel/bc/misc.c | 32 ++++++++++++++++++++++++++ > kernel/fork.c | 17 ++++++++++++- > kernel/irq/handle.c | 9 +++++++ > kernel/softirq.c | 8 ++++++ > 8 files changed, 132 insertions(+), 2 deletions(-) > > --- /dev/null 2006-07-18 14:52:43.075228448 +0400 > +++ ./include/bc/task.h 2006-08-28 12:54:48.000000000 +0400 > @@ -0,0 +1,59 @@ > +/* > + * include/bc/task.h > + * > + * Copyright (C) 2006 OpenVZ. SWsoft Inc > + * > + */ > + > +#ifndef __BC_TASK_H_ > +#define __BC_TASK_H_ > + > +struct beancounter; we can as well include beancounter.h here and avoid including it in different files below. > + > +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 { \ > + struct task_beancounter *tbc; \ > + tbc = &current->task_bc; \ > + BUG_ON(tbc->exec_bc != exp); \ > + tbc->exec_bc = old; \ > + } while (0) > + Didn't follow why the above two macros can't be functions. > +int __must_check bc_task_charge(struct task_struct *parent, > + struct task_struct *new); > +void bc_task_unchar…
creator: Chandra Seetharaman
date: 2006-08-30T19:11:32-00:00

Re: [ckrm-tech] [PATCH 5/7] BC: user interface (syscalls)
type: item
description: On Tue, 2006-08-29 at 18:55 +0400, Kirill Korotaev wrote: > 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> > > --- <snip> > + > +asmlinkage long sys_set_bclimit(bcid_t id, unsigned long resource, > + 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; > + > + error = -EINVAL; > + if (new_limits[0] > BC_MAXVALUE || new_limits[1] > BC_MAXVALUE || > + new_limits[0] > new_limits[1]) > + goto out; > + > + error = -ENOENT; > + bc = beancounter_findcreate(id, BC_LOOKUP); > + if (bc == NULL) > + goto out; Moving this to be before copy_from_user() would be efficient. > + > + spin_lock_irqsave(&bc->bc_lock, flags); > + bc->bc_parms[resource].barrier = new_limits[0]; > + bc->bc_parms[resource].limit = new_limits[1]; > + 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-30T19:15:23-00:00

Re: [ckrm-tech] [PATCH 6/7] BC: kernel memory (core)
type: item
description: On Tue, 2006-08-29 at 18:58 +0400, Kirill Korotaev wrote: > --- ./include/bc/beancounter.h.bckmem 2006-08-28 12:47:52.000000000 +0400 > +++ ./include/bc/beancounter.h 2006-08-28 12:59:28.000000000 +0400 > @@ -12,7 +12,9 @@ > * Resource list. > */ > > -#define BC_RESOURCES 0 > +#define BC_KMEMSIZE 0 > + > +#define BC_RESOURCES 1 <snip> > --- ./kernel/bc/beancounter.c.bckmem 2006-08-28 12:52:11.000000000 +0400 > +++ ./kernel/bc/beancounter.c 2006-08-28 12:59:28.000000000 +0400 > @@ -20,6 +20,7 @@ static void init_beancounter_struct(stru > struct beancounter init_bc; > > const char *bc_rnames[] = { > + "kmemsize", /* 0 */ > }; > > static struct hlist_head bc_hash[BC_HASH_SIZE]; > @@ -221,6 +222,8 @@ static void init_beancounter_syslimits(s > { > int k; > > + bc->bc_parms[BC_KMEMSIZE].limit = 32 * 1024 * 1024; > + > for (k = 0; k < BC_RESOURCES; k++) > bc->bc_parms[k].barrier = bc->bc_parms[k].limit; > } As I mentioned in one of my earlier email ( http://marc.theaimsgroup.com/?l=linux-kernel&m=115619384 500289&w=2), IMHO, this way of defining an interface is not clean/clear (for controller writers). <snip> -- ------------------------------------------------------------ ---------- Chandra Seetharaman | Be careful what you choose.... - sekharan@us.ibm.com | .......you may get it. ------------------------------------------------------------ ----------
creator: Chandra Seetharaman
date: 2006-08-30T19:25:26-00:00

Re: [ckrm-tech] [PATCH 3/7] BC: beancounters core (API)
type: item
description: On Wed, 30 Aug 2006 11:58:34 -0700 Chandra Seetharaman <sekharan@us.ibm.com> wrote: > > +void bc_uncharge_locked(struct beancounter *bc, int res, unsigned long val); > > +void bc_uncharge(struct beancounter *bc, int res, unsigned long val); > > + > > +struct beancounter *beancounter_findcreate(bcid_t id, int mask); > > prototype do not need the parameter names, types would suffice (would > save you few characters). argh. Those few characters are useful.
creator: Andrew Morton
date: 2006-08-30T19:47:52-00:00

Re: [PATCH 1/7] introduce atomic_dec_and_lock_irqsave()
type: item
description: On Wed, Aug 30, 2006 at 07:25:07PM +0200, Roman Zippel wrote: > Hi, > > On Wed, 30 Aug 2006, Dipankar Sarma wrote: > > > > > uidhash_lock can be taken from irq context. For example, delayed_put_task_struct() > > > > does __put_task_struct()->free_uid(). > > > > > > AFAICT it's called via rcu, does that mean anything released via rcu has > > > to be protected against interrupts? > > > > No. You need protection only if you have are using some > > data that can also be used by the RCU callback. For example, > > if your RCU callback just calls kfree(), you don't have to > > do a spin_lock_bh(). > > In this case kfree() does its own interrupt synchronization. I didn't > realize before that rcu had this (IMO serious) limitation. I think there > should be two call_rcu() variants, one that queues the callback in a soft > irq and a second which queues it in a thread context. How about just using synchronize_rcu() in the second situation? This primitive blocks until the grace period completes, allowing you to do the remaining processing in thread context. As a bonus, RCU code that uses synchronize_rcu() is usually quite a bit simpler than code using call_rcu(). Using synchronize_rcu(): list_del_rcu(p); synchronize_rcu(); kfree(p); Using call_rcu(): static void rcu_callback_func(struct rcu_head *rcu) { struct foo *p = container_of(rcu, struct foo, rcu); kfree(p); } list_del_rcu(p); call_rcu(&p->rcu, rcu_callback_func); Furthermore, the call_rcu() approach requires a struct rcu_head somewhere in the data structure, so use of synchronize_rcu() saves a bit of memory, as well. But if you have a situation where neither synchronize_srcu() nor call_rcu() is working out for you, let's hear it! Thanx, Paul
creator: paulmck
date: 2006-08-31T22:58:28-00:00

Re: [PATCH 6/7] BC: kernel memory (core)
type: item
description: Balbir Singh wrote: > Kirill Korotaev wrote: > >> 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. >> > > <snip> > >> +#define __GFP_BC_LIMIT ((__force gfp_t)0x100000u) /* Charge against >> BC limit */ >> > > What's _GFP_BC_LIMIT for, could you add the description for that flag? > The comment is not very clear > >> +#ifdef CONFIG_BEANCOUNTERS >> + union { >> + struct beancounter *page_bc; >> + } bc; >> +#endif >> }; >> >> +#define page_bc(page) ((page)->bc.page_bc) > > > Minor comment - page->(bc).page_bc has too many repititions of page and > bc - see > the Practice of Programming by Kernighan and Pike > > I missed the part of why you wanted to have a union (in struct page for > bc)? because this union is used both for kernel memory accounting and user memeory tracking. >> const char *bc_rnames[] = { >> + "kmemsize", /* 0 */ >> }; >> >> static struct hlist_head bc_hash[BC_HASH_SIZE]; >> @@ -221,6 +222,8 @@ static void init_beancounter_syslimits(s >> { int k; >> >> + bc->bc_parms[BC_KMEMSIZE].limit = 32 * 1024 * 1024; >> + > > > Can't this be configurable CONFIG_XXX or a #defined constant? This is some arbitraty limited container, just to make sure it is not created unlimited. User space should initialize limits properly after creation anyway. So I don't see reasons to make it configurable, do you? >> --- ./mm/mempool.c.bckmem 2006-04-21 11:59:36.000000000 +0400 >> +++ ./mm/mempool.c 2006-08-28 12:59:28.…
creator: dev
date: 2006-09-04T12:19:10-00:00

Re: [ckrm-tech] [PATCH 6/7] BC: kernel memory (core)
type: item
description: Dave Hansen wrote: > On Tue, 2006-08-29 at 18:58 +0400, Kirill Korotaev wrote: > >>@@ -274,8 +274,14 @@ struct page { >> unsigned int gfp_mask; >> unsigned long trace[8]; >> #endif >>+#ifdef CONFIG_BEANCOUNTERS >>+ union { >>+ struct beancounter *page_bc; >>+ } bc; >>+#endif >> }; > > > I know you're probably saving this union for when you put some userspace > stuff in here or something. But, for now, it would probably be best > just to leave it as a plain struct, or even an anonymous union. > > You probably had to use gcc 2 when this was written and couldn't use > anonymous unions, right? are you suggesting the code like this: >>+#ifdef CONFIG_BEANCOUNTERS >>+ union { >>+ struct beancounter *page_bc; >>+ }; >>+#endif >> }; #define page_bc(page) ((page)->page_bc) ? > -- Dave > > > ------------------------------------------------------------ ------------- > Using Tomcat but need to do more? Need to support web services, security? > Get stuff done quickly with pre-integrated technology to make your job easier > Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo > http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&b id=263057&dat=121642 > _______________________________________________ > ckrm-tech mailing list > https://lists.sourceforge.net/lists/listinfo/ckrm-tech >
creator: dev
date: 2006-09-04T12:20:00-00:00

Re: [PATCH 6/7] BC: kernel memory (core)
type: item
description: Kirill Korotaev wrote: > Balbir Singh wrote: >> Kirill Korotaev wrote: >>> +#ifdef CONFIG_BEANCOUNTERS >>> + union { >>> + struct beancounter *page_bc; >>> + } bc; >>> +#endif >>> }; >>> >>> +#define page_bc(page) ((page)->bc.page_bc) >> >> >> Minor comment - page->(bc).page_bc has too many repititions of page >> and bc - see >> the Practice of Programming by Kernighan and Pike >> >> I missed the part of why you wanted to have a union (in struct page >> for bc)? > because this union is used both for kernel memory accounting and user > memeory tracking. Ok.. that's good. I remember seeing a user_bc sometime back in the code. I had some idea about allowing tasks to migrate across resources (bean counters), which I think can be easily done for user space pages, if the user limits are tracked separately. > >>> const char *bc_rnames[] = { >>> + "kmemsize", /* 0 */ >>> }; >>> >>> static struct hlist_head bc_hash[BC_HASH_SIZE]; >>> @@ -221,6 +222,8 @@ static void init_beancounter_syslimits(s >>> { int k; >>> >>> + bc->bc_parms[BC_KMEMSIZE].limit = 32 * 1024 * 1024; >>> + >> >> >> Can't this be configurable CONFIG_XXX or a #defined constant? > This is some arbitraty limited container, just to make sure it is not > created unlimited. User space should initialize limits properly after > creation > anyway. So I don't see reasons to make it configurable, do you? May be its not very important now but configurable limits will help a confused user. Even if we decide to use this number for now, a constant like BC_DEFAULT_MEM_LIMIT is easier to read. >> I wonder if bc_page_charge() should be called bc_page_charge_failed()? >> Does it make sense to atleast partially start reclamation here? I know >> with >> bean counters we cannot reclaim from a particular container, but for now >> we could kick off kswapd() or call shrink_all_memory() inline (Dave's >> patches do this to shrink memory from the particular cpuset). Or do >> you want to leave this >> slot open for later? > yes. my inte…
creator: Balbir Singh
date: 2006-09-04T15:45:47-00:00

Re: [ckrm-tech] [PATCH 6/7] BC: kernel memory (core)
type: item
description: On Mon, 2006-09-04 at 16:23 +0400, Kirill Korotaev wrote: > > are you suggesting the code like this: > >>+#ifdef CONFIG_BEANCOUNTERS > >>+ union { > >>+ struct beancounter *page_bc; > >>+ }; > >>+#endif > >> }; > > #define page_bc(page) ((page)->page_bc) That would be a bit better. Although, I think the "page_" bit is also superfluous. -- Dave
creator: Dave Hansen
date: 2006-09-05T16:21:34-00:00