Memory Binding (0/1)
Subsystems (7)
RFC PATCH 1/1 Memory Binding (0/1)
So I've made a couple attempts at adding a Memory Binding facility to the kernel. Obviously, I've had no success as yet, so I'm stubbornly trying another approach! ;)
This patch is just a small pre-patch to the real memory binding. It just adds get_zonelist() and get_node_zonelist() calls, to make some code a little more readable. This patch is super straight-forward. The real fun is in the next patch...
[mcd@arrakis membind]$ diffstat 00-pre_membind.patch
gfp.h | 2 +- mmzone.h | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-)
-Matt
include/linux/gfp.h +1 -1
@@ -52,7 +52,7 @@if (unlikely (order >=MAX_ORDER ))return NULL ; -return __alloc_pages (gfp_mask, order,NODE_DATA (nid)->node_zonelists + (gfp_mask &GFP_ZONEMASK )); +return __alloc_pages (gfp_mask, order,get_node_zonelist (nid, gfp_mask)); }#define alloc_pages (gfp_mask, order) \
include/linux/mmzone.h +8 -0
@@ -326,6 +326,14 @@#define num_online_memblks () 1#endif /* CONFIG_DISCONTIGMEM || CONFIG_NUMA */ + +static inline struct zonelist *get_node_zonelist (int nid,int gfp_mask) +{ +return NODE_DATA (nid)->node_zonelists + (gfp_mask &GFP_ZONEMASK ); +} + +#define get_zonelist (gfp_mask) get_node_zonelist(numa_node_id(), gfp_mask) +#endif /* !__ASSEMBLY__ */ #endif /* __KERNEL__ */ #endif /* _LINUX_MMZONE_H */
Re: [patch][rfc] Memory Binding (1/1)
Okee dokee... Here's the real core of the patch. This adds a struct binding (basically, just a zonelist). It adds a pointer to a binding into the struct address_space. Then it adds sys_membind() syscall to allow user-processes to bind memory. For now, the membind call only works if the region specified (start_addr, len) is a shared memory segment. It is an easy first case, and I think it's one of the more useful types of memory to try to bind.
A binding call takes a cpumask in, translates it to a nodemask, and then ensures that all memory faulted into the region comes from memory on the nodes in the nodemask. Policy argument is ignored for now. Simple bindings only.
Pretty much all the code is hidden behind #ifdef CONFIG_NUMA. This has virtually no effect on non-NUMA kernels. The patch also is not 100% done. It does some quirky things if you exhaust a node's memory. I need to look more into the interactions of this patch and OOM code. I would like comments from anyone interested in this patch.
[mcd@arrakis membind]$ diffstat 01-membind.patch
arch/i386/kernel/entry.S | 1 fs/inode.c | 1 include/asm-i386/unistd.h | 3 include/linux/binding.h | 37 ++++++++ include/linux/fs.h | 2 include/linux/pagemap.h | 26 ++++++ mm/Makefile | 2 mm/binding.c | 199 ++++++++++++++++++++++++++++++++++++++++++++++ mm/swap_state.c | 1 9 files changed, 270 insertions(+), 2 deletions(-)
-Matt
arch/i386/kernel/entry.S +1 -0
@@ -852,6 +852,7 @@ .long sys_clock_gettime /* 265 */ .long sys_clock_getres .long sys_clock_nanosleep + .long sys_membind nr_syscalls=(.-sys_call_table)/4
fs/inode.c +1 -0
@@ -141,6 +141,7 @@ mapping->a_ops = &empty_aops; mapping->host = inode; mapping->gfp_mask =GFP_HIGHUSER ; + mapping->binding =NULL ; mapping->dirtied_when =0 ; mapping->assoc_mapping =NULL ; mapping->backing_dev_info = &default_backing_dev_info;
include/asm-i386/unistd.h +2 -1
@@ -273,8 +273,9 @@#define __NR_clock_gettime (__NR_timer_create+6)#define __NR_clock_getres (__NR_timer_create+7)#define __NR_clock_nanosleep (__NR_timer_create+8) +#define __NR_membind 268 -#define NR_syscalls 268 +#define NR_syscalls 269/* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
include/linux/binding.h +37 -0
@@ -0,0 +1,37 @@ +/* +* include/linux/binding.h +* +* Written by: Matthew Dobson, IBM Corporation +* +* Copyright (C) 2003, IBM Corp. +* +* All rights reserved. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or +* NON INFRINGEMENT. See the GNU General Public License for more +* details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +* +* Send feedback to <[email protected]> +*/ +#ifndef _LINUX_BINDING_H +#define _LINUX_BINDING_H + +#include <linux/mmzone.h> + +/* Structure to keep track of shared memory segment bindings */ +struct binding { +struct zonelist zonelist; +}; + +#endif /* _LINUX_BINDING_H */
include/linux/fs.h +2 -0
@@ -19,6 +19,7 @@#include <linux/cache.h> #include <linux/radix-tree.h> #include <linux/kobject.h> +#include <linux/binding.h> #include <asm/atomic.h> struct iovec ;
@@ -325,6 +326,7 @@struct semaphore i_shared_sem;/* and sem protecting it */ unsigned long dirtied_when;/* jiffies of first page dirtying */ int gfp_mask;/* how to allocate the pages */ +struct binding *binding;/* for memory bindings */ struct backing_dev_info *backing_dev_info;/* device readahead, etc */ spinlock_t private_lock;/* for use by the address_space */ struct list_head private_list;/* ditto */
include/linux/pagemap.h +26 -0
@@ -8,6 +8,7 @@#include <linux/fs.h> #include <linux/list.h> #include <linux/highmem.h> +#include <linux/binding.h> #include <asm/uaccess.h> /*
@@ -27,6 +28,8 @@#define page_cache_release (page) put_page(page)void release_pages (struct page **pages,int nr,int cold); +#ifndef CONFIG_NUMA +static inline struct page *page_cache_alloc (struct address_space *x) {return alloc_pages (x->gfp_mask,0 );
@@ -37,6 +40,29 @@return alloc_pages (x->gfp_mask|__GFP_COLD,0 ); } +#else /* CONFIG_NUMA */ + +static inline struct page *__page_cache_alloc (struct address_space *x,int cold) +{ +int gfp_mask; +struct zonelist *zonelist; + + gfp_mask = x->gfp_mask; +if (cold) + gfp_mask |= __GFP_COLD; +if (!x->binding) + zonelist =get_zonelist (gfp_mask); +else + zonelist = &x->binding->zonelist; + +return __alloc_pages (gfp_mask,0 , zonelist); +} + +#define page_cache_alloc (x) __page_cache_alloc((x), 0) +#define page_cache_alloc_cold (x) __page_cache_alloc((x), 1) + +#endif /* !CONFIG_NUMA */ +typedef int filler_t (void *,struct page *); extern struct page * find_get_page(struct address_space *mapping,
mm/Makefile +1 -1
@@ -7,7 +7,7 @@ mlock.o mmap.o mprotect.o mremap.o msync.o rmap.o \ shmem.o vmalloc.o -obj-y := bootmem.o filemap.o mempool.o oom_kill.o fadvise.o \ +obj-y := binding.o bootmem.o fadvise.o filemap.o mempool.o oom_kill.o \ page_alloc.o page-writeback.o pdflush.o readahead.o \ slab.o swap.o truncate.o vcache.o vmscan.o $(mmu-y)
mm/binding.c +199 -0
199 lines not on this page · raw message
mm/swap_state.c +1 -0
@@ -42,6 +42,7 @@ .host = &swapper_inode, .a_ops = &swap_aops, .backing_dev_info = &swap_backing_dev_info, + .binding = NULL, .i_mmap = LIST_HEAD_INIT(swapper_space.i_mmap), .i_mmap_shared = LIST_HEAD_INIT(swapper_space.i_mmap_shared), .i_shared_sem = __MUTEX_INITIALIZER(swapper_space.i_shared_sem),
Re: [patch][rfc] Memory Binding (1/1)
Quoted 2 lines · Matthew Dobson
Okee dokee... Here's the real core of the patch.
Looks saneish to me. I'd like to see thorough benchmark results when it is complete. And it would be nice to make address_space.binding go away if !CONFIG_NUMA.
The explicit knowledge of ZONE_DMA/ZONE_NORMAL/ZONE_HIGHMEM in get_zonetype() should not be necessary - you don't want it to explode if ZONE_DMA32 is added. It should be indexing into node_zonelists in some manner.
Will this code work if all memory is in ZONE_DMA, as some architectures do?
Re: [patch][rfc] Memory Binding (1/1)
Quoted 8 lines · Andrew Morton
Quoted 1 line · Matthew Dobson
Okee dokee... Here's the real core of the patch.
Looks saneish to me. I'd like to see thorough benchmark results when it is complete. And it would be nice to make address_space.binding go away if !CONFIG_NUMA.
Yeah... I figured the one pointer wouldn't be a big deal, and it would save a few more ifdefs, but I guess consistency is better. If I ifdef most of it, may as well ifdef it all!
Quoted 5 lines
The explicit knowledge of ZONE_DMA/ZONE_NORMAL/ZONE_HIGHMEM in
get_zonetype()should not be necessary - you don't want it to explode ifZONE_DMA32is added. It should be indexing intonode_zonelistsin some manner.Will this code work if all memory is in
ZONE_DMA, as some architectures do?
Well, what I do for now is use the gfp_mask in the address_space that is already there for the shm segments. This means I have to look at that to decide if they want DMA, NORMAL, or HIGHMEM. So it already would need to be changed if we added a new zone. If the thing we're creating a binding for only can use DMA, for example, I need to make sure I use only the DMA zones. I can't easily see a way to make sure I add only the appropriate zones to the zonelist, and also don't look at the gfp_flag to determine the zones they want. I'll ponder it more deeply tonight, though.
New patch, sans binding pointer attatched.
Cheers!
-Matt
arch/i386/kernel/entry.S +1 -0
@@ -852,6 +852,7 @@ .long sys_clock_gettime /* 265 */ .long sys_clock_getres .long sys_clock_nanosleep + .long sys_membind nr_syscalls=(.-sys_call_table)/4
fs/inode.c +1 -0
@@ -141,6 +141,7 @@ mapping->a_ops = &empty_aops; mapping->host = inode; mapping->gfp_mask =GFP_HIGHUSER ; + mapping->binding =NULL ; mapping->dirtied_when =0 ; mapping->assoc_mapping =NULL ; mapping->backing_dev_info = &default_backing_dev_info;
include/asm-i386/unistd.h +2 -1
@@ -273,8 +273,9 @@#define __NR_clock_gettime (__NR_timer_create+6)#define __NR_clock_getres (__NR_timer_create+7)#define __NR_clock_nanosleep (__NR_timer_create+8) +#define __NR_membind 268 -#define NR_syscalls 268 +#define NR_syscalls 269/* user-visible error numbers are in the range -1 - -124: see <asm-i386/errno.h> */
include/linux/binding.h +40 -0
@@ -0,0 +1,40 @@ +/* +* include/linux/binding.h +* +* Written by: Matthew Dobson, IBM Corporation +* +* Copyright (C) 2003, IBM Corp. +* +* All rights reserved. +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, but +* WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or +* NON INFRINGEMENT. See the GNU General Public License for more +* details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. +* +* Send feedback to <[email protected]> +*/ +#ifndef _LINUX_BINDING_H +#define _LINUX_BINDING_H + +#ifdef CONFIG_NUMA + +#include <linux/mmzone.h> + +/* Structure to keep track of shared memory segment bindings */ +struct binding { +struct zonelist zonelist; +}; + +#endif /* CONFIG_NUMA */ +#endif /* _LINUX_BINDING_H */
include/linux/fs.h +4 -0
@@ -19,6 +19,7 @@#include <linux/cache.h> #include <linux/radix-tree.h> #include <linux/kobject.h> +#include <linux/binding.h> #include <asm/atomic.h> struct iovec ;
@@ -329,6 +330,9 @@spinlock_t private_lock;/* for use by the address_space */ struct list_head private_list;/* ditto */ struct address_space *assoc_mapping;/* ditto */ +#ifdef CONFIG_NUMA +struct binding *binding;/* for memory bindings */ +#endif };struct block_device {
include/linux/pagemap.h +26 -0
@@ -8,6 +8,7 @@#include <linux/fs.h> #include <linux/list.h> #include <linux/highmem.h> +#include <linux/binding.h> #include <asm/uaccess.h> /*
@@ -27,6 +28,8 @@#define page_cache_release (page) put_page(page)void release_pages (struct page **pages,int nr,int cold); +#ifndef CONFIG_NUMA +static inline struct page *page_cache_alloc (struct address_space *x) {return alloc_pages (x->gfp_mask,0 );
@@ -37,6 +40,29 @@return alloc_pages (x->gfp_mask|__GFP_COLD,0 ); } +#else /* CONFIG_NUMA */ + +static inline struct page *__page_cache_alloc (struct address_space *x,int cold) +{ +int gfp_mask; +struct zonelist *zonelist; + + gfp_mask = x->gfp_mask; +if (cold) + gfp_mask |= __GFP_COLD; +if (!x->binding) + zonelist =get_zonelist (gfp_mask); +else + zonelist = &x->binding->zonelist; + +return __alloc_pages (gfp_mask,0 , zonelist); +} + +#define page_cache_alloc (x) __page_cache_alloc((x), 0) +#define page_cache_alloc_cold (x) __page_cache_alloc((x), 1) + +#endif /* !CONFIG_NUMA */ +typedef int filler_t (void *,struct page *); extern struct page * find_get_page(struct address_space *mapping,
mm/Makefile +1 -1
@@ -7,7 +7,7 @@ mlock.o mmap.o mprotect.o mremap.o msync.o rmap.o \ shmem.o vmalloc.o -obj-y := bootmem.o filemap.o mempool.o oom_kill.o fadvise.o \ +obj-y := binding.o bootmem.o fadvise.o filemap.o mempool.o oom_kill.o \ page_alloc.o page-writeback.o pdflush.o readahead.o \ slab.o swap.o truncate.o vcache.o vmscan.o $(mmu-y)
mm/binding.c +199 -0
199 lines not on this page · raw message
mm/swap_state.c +3 -0
@@ -47,6 +47,9 @@ .i_shared_sem = __MUTEX_INITIALIZER(swapper_space.i_shared_sem), .private_lock = SPIN_LOCK_UNLOCKED, .private_list = LIST_HEAD_INIT(swapper_space.private_list), +#ifdef CONFIG_NUMA + .binding = NULL, +#endif };#define INC_CACHE_INFO (x) do { swap_cache_info.x++; } while (0)
Re: [patch][rfc] Memory Binding (1/1)
Quoted 1 line · Matthew Dobson
Yeah... I figured the one pointer wouldn't be a big deal...
the road to hell is paved through indirections
-Bryan
Re: [patch][rfc] Memory Binding (1/1)
In reply to Matthew Dobsonarch/i386/kernel/entry.S line 855
Quoted 7 lines · Matthew Dobson
arch/i386/kernel/entry.S +1
-0@@ -852,6 +852,7 @@ .long sys_clock_gettime /* 265 */ .long sys_clock_getres .long sys_clock_nanosleep + .long sys_membind
Syscall slot 223 is unused, please use it instead of adding at the end.
arch/i386/kernel/entry.S
Quoted 9 lines
arch/i386/kernel/entry.S +9
-0+#ifndef CONFIG_NUMA + +asmlinkage unsigned long sys_membind(unsigned long start, unsigned long len, + unsigned long *mask_ptr, unsigned int mask_len, unsigned long policy) +{ + return -ENOSYS; +} + +#else /* CONFIG_NUMA */
Yuck! Compile this file conditional on CONFIG_NUMA and add a cond_syscall entry for sys_membind to kernel/sys.c
arch/i386/kernel/entry.S
Quoted 8 lines
arch/i386/kernel/entry.S +8
-0+#include <linux/binding.h> +#include <asm/string.h> +#include <asm/topology.h> +#include <asm/uaccess.h> + +/* Translate a cpumask to a nodemask */ +static inline void cpumask_to_nodemask(DECLARE_BITMAP(cpumask, NR_CPUS), + DECLARE_BITMAP(nodemask, MAX_NUMNODES))
Umm, this arguments looks strange. DECLARE_BITMAP always expands to an unsigned long array, so just use unsigned long * arguments.
Quoted 10 lines
arch/i386/kernel/entry.S +10
-0+static inline int add_zones(pg_data_t *pgdat, struct zonelist *zonelist, + int zone_num, int zone_type) +{ + switch (zone_type) { + struct zone *zone; + default: + BUG(); + case ZONE_HIGHMEM: + zone = pgdat->node_zones + ZONE_HIGHMEM; + if (zone->present_pages)
arch/i386/kernel/entry.S
Quoted 10 lines
arch/i386/kernel/entry.S +10
-0+ zonelist->zones[zone_num++] = zone; + case ZONE_NORMAL: + zone = pgdat->node_zones + ZONE_NORMAL; + if (zone->present_pages) + zonelist->zones[zone_num++] = zone; + case ZONE_DMA: + zone = pgdat->node_zones + ZONE_DMA; + if (zone->present_pages) + zonelist->zones[zone_num++] = zone; + }
Hardconding of zones looks like a bad idea. Do you really need to expose zones in the API? User memory should always be in ZONE_HIGHMEM (or ZONE_NORMAL for 64bit arches).
arch/i386/kernel/entry.S
Quoted 8 lines
arch/i386/kernel/entry.S +8
-0+ vma = find_vma(current->mm, start); + if (!(vma && vma->vm_file && vma->vm_ops && + vma->vm_ops->nopage == shmem_nopage)) { + /* This isn't a shm segment. For now, we bail. */ + printk("%s: Can only bind shm(em) segments for now!\n", __FUNCTION__); + error = -EINVAL; + goto out; + }
This check sounds like a bad idea. Please make this API generic.
Re: [Lse-tech] Re: [patch][rfc] Memory Binding (1/1)
In reply to Matthew DobsonOn Wed, 2003-04-02 at 00:39, Matthew Dobson wrote:
I'm not sure why you do this on shm segments only. Why not directly on vmas ?
I'm considering to add a similar thing to the mm_struct to allow processes to allocate on their "homenode" only. But I could also live with it being per VMA.
In case of a large number of such bindings or nodes it may make sense to cache and share the zone lists, but that can be probably left for a future patch.
Quoted 5 lines
include/linux/pagemap.h +5
-0+static inline struct page *__page_cache_alloc (struct address_space *x,int cold) +{ +int gfp_mask; +struct zonelist *zonelist; +
include/linux/pagemap.h line 59
Quoted 10 lines
include/linux/pagemap.h +10
-0+ gfp_mask = x->gfp_mask; +if (cold) + gfp_mask |= __GFP_COLD; +if (!x->binding) + zonelist =get_zonelist (gfp_mask); +else + zonelist = &x->binding->zonelist; + +return __alloc_pages (gfp_mask,0 , zonelist); +}
I would move a function of this size out of line. In fact i think it should even in the non NUMA case be a simple function call with zonelist getting evaluated out-of-line.
Quoted 4 lines
mm/binding.c +4
-0+asmlinkage unsigned long sys_membind (unsigned long start,unsigned long len, +unsigned long *mask_ptr,unsigned int mask_len,unsigned long policy) +{ +DECLARE_BITMAP (cpu_mask,NR_CPUS );
mm/binding.c line 170
Quoted 10 lines
mm/binding.c +10
-0+DECLARE_BITMAP (node_mask,MAX_NUMNODES ); +struct vm_area_struct *vma =NULL ; +struct address_space *mapping; +int error =0 ; + +/* Deal with getting cpu_mask from userspace & translating to node_mask */ +if (mask_len >NR_CPUS ) { + error = -EINVAL ; + goto out; + }
I don't like that check. It requires hardcoding NR_CPUs (which can be variable!) in the application. It would be better to allow arbitary length arguments, but only error when there is a bit set outside NR_CPUS. Of course arbitary would be hard regarding the copy from user, so perhaps use some very big limit (e.g. one page worth of bitmap)
mm/binding.c line 183
Quoted 13 lines
mm/binding.c +13
-0+CLEAR_BITMAP (cpu_mask,NR_CPUS ); +CLEAR_BITMAP (node_mask,MAX_NUMNODES ); +if (copy_from_user (cpu_mask, mask_ptr, (mask_len+7 )/8 )) { + error = -EFAULT ; + goto out; + } +cpumask_to_nodemask (cpu_mask, node_mask); + + vma =find_vma (current->mm, start); +if (!(vma && vma->vm_file && vma->vm_ops && + vma->vm_ops->nopage == shmem_nopage)) { +/* This isn't a shm segment. For now, we bail. */ +printk ("%s: Can only bind shm(em) segments for now!\n" , __FUNCTION__);
Instead of __FUNCTION__ i would use current->comm here.
Or better perhaps just remove the error printks.
-Andi