| Message ID | alpine.DEB.2.10.1402121612270.8183@nuc (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 12.02.2014 [16:16:11 -0600], Christoph Lameter wrote: > Here is another patch with some fixes. The additional logic is only > compiled in if CONFIG_HAVE_MEMORYLESS_NODES is set. > > Subject: slub: Memoryless node support > > Support memoryless nodes by tracking which allocations are failing. > Allocations targeted to the nodes without memory fall back to the > current available per cpu objects and if that is not available will > create a new slab using the page allocator to fallback from the > memoryless node to some other node. I'll try and retest this once the LPAR in question comes free. Hopefully in the next day or two. Thanks, Nish > Signed-off-by: Christoph Lameter <cl@linux.com> > > Index: linux/mm/slub.c > =================================================================== > --- linux.orig/mm/slub.c 2014-02-12 16:07:48.957869570 -0600 > +++ linux/mm/slub.c 2014-02-12 16:09:22.198928260 -0600 > @@ -134,6 +134,10 @@ static inline bool kmem_cache_has_cpu_pa > #endif > } > > +#ifdef CONFIG_HAVE_MEMORYLESS_NODES > +static nodemask_t empty_nodes; > +#endif > + > /* > * Issues still to be resolved: > * > @@ -1405,16 +1409,28 @@ static struct page *new_slab(struct kmem > void *last; > void *p; > int order; > + int alloc_node; > > BUG_ON(flags & GFP_SLAB_BUG_MASK); > > page = allocate_slab(s, > flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node); > - if (!page) > + if (!page) { > +#ifdef CONFIG_HAVE_MEMORYLESS_NODES > + if (node != NUMA_NO_NODE) > + node_set(node, empty_nodes); > +#endif > goto out; > + } > > order = compound_order(page); > - inc_slabs_node(s, page_to_nid(page), page->objects); > + alloc_node = page_to_nid(page); > +#ifdef CONFIG_HAVE_MEMORYLESS_NODES > + node_clear(alloc_node, empty_nodes); > + if (node != NUMA_NO_NODE && alloc_node != node) > + node_set(node, empty_nodes); > +#endif > + inc_slabs_node(s, alloc_node, page->objects); > memcg_bind_pages(s, order); > page->slab_cache = s; > __SetPageSlab(page); > @@ -1722,7 +1738,7 @@ static void *get_partial(struct kmem_cac > struct kmem_cache_cpu *c) > { > void *object; > - int searchnode = (node == NUMA_NO_NODE) ? numa_node_id() : node; > + int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node; > > object = get_partial_node(s, get_node(s, searchnode), c, flags); > if (object || node != NUMA_NO_NODE) > @@ -2117,8 +2133,19 @@ static void flush_all(struct kmem_cache > static inline int node_match(struct page *page, int node) > { > #ifdef CONFIG_NUMA > - if (!page || (node != NUMA_NO_NODE && page_to_nid(page) != node)) > + int page_node = page_to_nid(page); > + > + if (!page) > return 0; > + > + if (node != NUMA_NO_NODE) { > +#ifdef CONFIG_HAVE_MEMORYLESS_NODES > + if (node_isset(node, empty_nodes)) > + return 1; > +#endif > + if (page_node != node) > + return 0; > + } > #endif > return 1; > } >
On Wed, Feb 12, 2014 at 04:16:11PM -0600, Christoph Lameter wrote: > Here is another patch with some fixes. The additional logic is only > compiled in if CONFIG_HAVE_MEMORYLESS_NODES is set. > > Subject: slub: Memoryless node support > > Support memoryless nodes by tracking which allocations are failing. I still don't understand why this tracking is needed. All we need for allcation targeted to memoryless node is to fallback proper node, that it, numa_mem_id() node of targeted node. My previous patch implements it and use proper fallback node on every allocation code path. Why this tracking is needed? Please elaborate more on this. > Allocations targeted to the nodes without memory fall back to the > current available per cpu objects and if that is not available will > create a new slab using the page allocator to fallback from the > memoryless node to some other node. > > Signed-off-by: Christoph Lameter <cl@linux.com> > > @@ -1722,7 +1738,7 @@ static void *get_partial(struct kmem_cac > struct kmem_cache_cpu *c) > { > void *object; > - int searchnode = (node == NUMA_NO_NODE) ? numa_node_id() : node; > + int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node; > > object = get_partial_node(s, get_node(s, searchnode), c, flags); > if (object || node != NUMA_NO_NODE) This isn't enough. Consider that allcation targeted to memoryless node. get_partial_node() always fails even if there are some partial slab on memoryless node's neareast node. We should fallback to some proper node in this case, since there is no slab on memoryless node. Thanks.
On Mon, 17 Feb 2014, Joonsoo Kim wrote: > On Wed, Feb 12, 2014 at 04:16:11PM -0600, Christoph Lameter wrote: > > Here is another patch with some fixes. The additional logic is only > > compiled in if CONFIG_HAVE_MEMORYLESS_NODES is set. > > > > Subject: slub: Memoryless node support > > > > Support memoryless nodes by tracking which allocations are failing. > > I still don't understand why this tracking is needed. Its an optimization to avoid calling the page allocator to figure out if there is memory available on a particular node. > All we need for allcation targeted to memoryless node is to fallback proper > node, that it, numa_mem_id() node of targeted node. My previous patch > implements it and use proper fallback node on every allocation code path. > Why this tracking is needed? Please elaborate more on this. Its too slow to do that on every alloc. One needs to be able to satisfy most allocations without switching percpu slabs for optimal performance. > > Allocations targeted to the nodes without memory fall back to the > > current available per cpu objects and if that is not available will > > create a new slab using the page allocator to fallback from the > > memoryless node to some other node. And what about the next alloc? Assuem there are N allocs from a memoryless node this means we push back the partial slab on each alloc and then fall back? > > { > > void *object; > > - int searchnode = (node == NUMA_NO_NODE) ? numa_node_id() : node; > > + int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node; > > > > object = get_partial_node(s, get_node(s, searchnode), c, flags); > > if (object || node != NUMA_NO_NODE) > > This isn't enough. > Consider that allcation targeted to memoryless node. It will not common get there because of the tracking. Instead a per cpu object will be used. > get_partial_node() always fails even if there are some partial slab on > memoryless node's neareast node. Correct and that leads to a page allocator action whereupon the node will be marked as empty. > We should fallback to some proper node in this case, since there is no slab > on memoryless node. NUMA is about optimization of memory allocations. It is often *not* about correctness but heuristics are used in many cases. F.e. see the zone reclaim logic, zone reclaim mode, fallback scenarios in the page allocator etc etc.
On 12.02.2014 [16:16:11 -0600], Christoph Lameter wrote: > Here is another patch with some fixes. The additional logic is only > compiled in if CONFIG_HAVE_MEMORYLESS_NODES is set. > > Subject: slub: Memoryless node support > > Support memoryless nodes by tracking which allocations are failing. > Allocations targeted to the nodes without memory fall back to the > current available per cpu objects and if that is not available will > create a new slab using the page allocator to fallback from the > memoryless node to some other node. > > Signed-off-by: Christoph Lameter <cl@linux.com> Tested-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com> Acked-by: Nishanth Aravamudan <nacc@linux.vnet.ibm.com> > Index: linux/mm/slub.c > =================================================================== > --- linux.orig/mm/slub.c 2014-02-12 16:07:48.957869570 -0600 > +++ linux/mm/slub.c 2014-02-12 16:09:22.198928260 -0600 > @@ -134,6 +134,10 @@ static inline bool kmem_cache_has_cpu_pa > #endif > } > > +#ifdef CONFIG_HAVE_MEMORYLESS_NODES > +static nodemask_t empty_nodes; > +#endif > + > /* > * Issues still to be resolved: > * > @@ -1405,16 +1409,28 @@ static struct page *new_slab(struct kmem > void *last; > void *p; > int order; > + int alloc_node; > > BUG_ON(flags & GFP_SLAB_BUG_MASK); > > page = allocate_slab(s, > flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node); > - if (!page) > + if (!page) { > +#ifdef CONFIG_HAVE_MEMORYLESS_NODES > + if (node != NUMA_NO_NODE) > + node_set(node, empty_nodes); > +#endif > goto out; > + } > > order = compound_order(page); > - inc_slabs_node(s, page_to_nid(page), page->objects); > + alloc_node = page_to_nid(page); > +#ifdef CONFIG_HAVE_MEMORYLESS_NODES > + node_clear(alloc_node, empty_nodes); > + if (node != NUMA_NO_NODE && alloc_node != node) > + node_set(node, empty_nodes); > +#endif > + inc_slabs_node(s, alloc_node, page->objects); > memcg_bind_pages(s, order); > page->slab_cache = s; > __SetPageSlab(page); > @@ -1722,7 +1738,7 @@ static void *get_partial(struct kmem_cac > struct kmem_cache_cpu *c) > { > void *object; > - int searchnode = (node == NUMA_NO_NODE) ? numa_node_id() : node; > + int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node; > > object = get_partial_node(s, get_node(s, searchnode), c, flags); > if (object || node != NUMA_NO_NODE) > @@ -2117,8 +2133,19 @@ static void flush_all(struct kmem_cache > static inline int node_match(struct page *page, int node) > { > #ifdef CONFIG_NUMA > - if (!page || (node != NUMA_NO_NODE && page_to_nid(page) != node)) > + int page_node = page_to_nid(page); > + > + if (!page) > return 0; > + > + if (node != NUMA_NO_NODE) { > +#ifdef CONFIG_HAVE_MEMORYLESS_NODES > + if (node_isset(node, empty_nodes)) > + return 1; > +#endif > + if (page_node != node) > + return 0; > + } > #endif > return 1; > } >
On Tue, 18 Feb 2014, Christoph Lameter wrote: > Its an optimization to avoid calling the page allocator to figure out if > there is memory available on a particular node. > Thus this patch breaks with memory hot-add for a memoryless node.
On Wed, 19 Feb 2014, David Rientjes wrote: > On Tue, 18 Feb 2014, Christoph Lameter wrote: > > > Its an optimization to avoid calling the page allocator to figure out if > > there is memory available on a particular node. > Thus this patch breaks with memory hot-add for a memoryless node. As soon as the per cpu slab is exhausted the node number of the so far "empty" node will be used for allocation. That will be sucessfull and the node will no longer be marked as empty.
On Tue, Feb 18, 2014 at 10:38:01AM -0600, Christoph Lameter wrote: > On Mon, 17 Feb 2014, Joonsoo Kim wrote: > > > On Wed, Feb 12, 2014 at 04:16:11PM -0600, Christoph Lameter wrote: > > > Here is another patch with some fixes. The additional logic is only > > > compiled in if CONFIG_HAVE_MEMORYLESS_NODES is set. > > > > > > Subject: slub: Memoryless node support > > > > > > Support memoryless nodes by tracking which allocations are failing. > > > > I still don't understand why this tracking is needed. > > Its an optimization to avoid calling the page allocator to figure out if > there is memory available on a particular node. > > > All we need for allcation targeted to memoryless node is to fallback proper > > node, that it, numa_mem_id() node of targeted node. My previous patch > > implements it and use proper fallback node on every allocation code path. > > Why this tracking is needed? Please elaborate more on this. > > Its too slow to do that on every alloc. One needs to be able to satisfy > most allocations without switching percpu slabs for optimal performance. I don't think that we need to switch percpu slabs on every alloc. Allocation targeted to specific node is rare. And most of these allocations may be targeted to either numa_node_id() or numa_mem_id(). My patch considers these cases, so most of allocations are processed by percpu slabs. There is no suboptimal performance. > > > > Allocations targeted to the nodes without memory fall back to the > > > current available per cpu objects and if that is not available will > > > create a new slab using the page allocator to fallback from the > > > memoryless node to some other node. > > And what about the next alloc? Assuem there are N allocs from a memoryless > node this means we push back the partial slab on each alloc and then fall > back? > > > > { > > > void *object; > > > - int searchnode = (node == NUMA_NO_NODE) ? numa_node_id() : node; > > > + int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node; > > > > > > object = get_partial_node(s, get_node(s, searchnode), c, flags); > > > if (object || node != NUMA_NO_NODE) > > > > This isn't enough. > > Consider that allcation targeted to memoryless node. > > It will not common get there because of the tracking. Instead a per cpu > object will be used. > > get_partial_node() always fails even if there are some partial slab on > > memoryless node's neareast node. > > Correct and that leads to a page allocator action whereupon the node will > be marked as empty. Why do we need to request to a page allocator if there is partial slab? Checking whether node is memoryless or not is really easy, so we don't need to skip this. To skip this is suboptimal solution. > > We should fallback to some proper node in this case, since there is no slab > > on memoryless node. > > NUMA is about optimization of memory allocations. It is often *not* about > correctness but heuristics are used in many cases. F.e. see the zone > reclaim logic, zone reclaim mode, fallback scenarios in the page allocator > etc etc. Okay. But, 'do our best' is preferable to me. Thanks.
On Mon, 24 Feb 2014, Joonsoo Kim wrote: > > It will not common get there because of the tracking. Instead a per cpu > > object will be used. > > > get_partial_node() always fails even if there are some partial slab on > > > memoryless node's neareast node. > > > > Correct and that leads to a page allocator action whereupon the node will > > be marked as empty. > > Why do we need to request to a page allocator if there is partial slab? > Checking whether node is memoryless or not is really easy, so we don't need > to skip this. To skip this is suboptimal solution. The page allocator action is also used to determine to which other node we should fall back if the node is empty. So we need to call the page allocator when the per cpu slab is exhaused with the node of the memoryless node to get memory from the proper fallback node.
On 24.02.2014 [13:54:35 -0600], Christoph Lameter wrote: > On Mon, 24 Feb 2014, Joonsoo Kim wrote: > > > > It will not common get there because of the tracking. Instead a per cpu > > > object will be used. > > > > get_partial_node() always fails even if there are some partial slab on > > > > memoryless node's neareast node. > > > > > > Correct and that leads to a page allocator action whereupon the node will > > > be marked as empty. > > > > Why do we need to request to a page allocator if there is partial slab? > > Checking whether node is memoryless or not is really easy, so we don't need > > to skip this. To skip this is suboptimal solution. > > The page allocator action is also used to determine to which other node we > should fall back if the node is empty. So we need to call the page > allocator when the per cpu slab is exhaused with the node of the > memoryless node to get memory from the proper fallback node. Where do we stand with these patches? I feel like no resolution was really found... Thanks, Nish
Index: linux/mm/slub.c =================================================================== --- linux.orig/mm/slub.c 2014-02-12 16:07:48.957869570 -0600 +++ linux/mm/slub.c 2014-02-12 16:09:22.198928260 -0600 @@ -134,6 +134,10 @@ static inline bool kmem_cache_has_cpu_pa #endif } +#ifdef CONFIG_HAVE_MEMORYLESS_NODES +static nodemask_t empty_nodes; +#endif + /* * Issues still to be resolved: * @@ -1405,16 +1409,28 @@ static struct page *new_slab(struct kmem void *last; void *p; int order; + int alloc_node; BUG_ON(flags & GFP_SLAB_BUG_MASK); page = allocate_slab(s, flags & (GFP_RECLAIM_MASK | GFP_CONSTRAINT_MASK), node); - if (!page) + if (!page) { +#ifdef CONFIG_HAVE_MEMORYLESS_NODES + if (node != NUMA_NO_NODE) + node_set(node, empty_nodes); +#endif goto out; + } order = compound_order(page); - inc_slabs_node(s, page_to_nid(page), page->objects); + alloc_node = page_to_nid(page); +#ifdef CONFIG_HAVE_MEMORYLESS_NODES + node_clear(alloc_node, empty_nodes); + if (node != NUMA_NO_NODE && alloc_node != node) + node_set(node, empty_nodes); +#endif + inc_slabs_node(s, alloc_node, page->objects); memcg_bind_pages(s, order); page->slab_cache = s; __SetPageSlab(page); @@ -1722,7 +1738,7 @@ static void *get_partial(struct kmem_cac struct kmem_cache_cpu *c) { void *object; - int searchnode = (node == NUMA_NO_NODE) ? numa_node_id() : node; + int searchnode = (node == NUMA_NO_NODE) ? numa_mem_id() : node; object = get_partial_node(s, get_node(s, searchnode), c, flags); if (object || node != NUMA_NO_NODE) @@ -2117,8 +2133,19 @@ static void flush_all(struct kmem_cache static inline int node_match(struct page *page, int node) { #ifdef CONFIG_NUMA - if (!page || (node != NUMA_NO_NODE && page_to_nid(page) != node)) + int page_node = page_to_nid(page); + + if (!page) return 0; + + if (node != NUMA_NO_NODE) { +#ifdef CONFIG_HAVE_MEMORYLESS_NODES + if (node_isset(node, empty_nodes)) + return 1; +#endif + if (page_node != node) + return 0; + } #endif return 1; }
Here is another patch with some fixes. The additional logic is only compiled in if CONFIG_HAVE_MEMORYLESS_NODES is set. Subject: slub: Memoryless node support Support memoryless nodes by tracking which allocations are failing. Allocations targeted to the nodes without memory fall back to the current available per cpu objects and if that is not available will create a new slab using the page allocator to fallback from the memoryless node to some other node. Signed-off-by: Christoph Lameter <cl@linux.com>