Re: [PATCH] slub: Don't throw away partial remote slabs if there is no local memory

Joonsoo Kim
2014-01-27T05:58:20Z

On Fri, Jan 24, 2014 at 05:10:42PM -0800, Nishanth Aravamudan wrote:
> On 24.01.2014 [16:25:58 -0800], David Rientjes wrote:
> > On Fri, 24 Jan 2014, Nishanth Aravamudan wrote:
> >
> > > Thank you for clarifying and providing a test patch. I ran with this on
> > > the system showing the original problem, configured to have 15GB of
> > > memory.
> > >
> > > With your patch after boot:
> > >
> > > MemTotal: 15604736 kB
> > > MemFree: 8768192 kB
> > > Slab: 3882560 kB
> > > SReclaimable: 105408 kB
> > > SUnreclaim: 3777152 kB
> > >
> > > With Anton's patch after boot:
> > >
> > > MemTotal: 15604736 kB
> > > MemFree: 11195008 kB
> > > Slab: 1427968 kB
> > > SReclaimable: 109184 kB
> > > SUnreclaim: 1318784 kB
> > >
> > >
> > > I know that's fairly unscientific, but the numbers are reproducible.
> > >

Hello,

I think that there is one mistake on David's patch although I'm not sure
that it is the reason for this result.

With David's patch, get_partial() in new_slab_objects() doesn't work properly,
because we only change node id in !node_match() case. If we meet just !freelist
case, we pass node id directly to new_slab_objects(), so we always try to allocate
new slab page regardless existence of partial pages. We should solve it.

Could you try this one?

Thanks.

--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1698,8 +1698,10 @@ static void *get_partial(struct kmem_cache *s, gfp_t flags, int node,
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;

+ if (node != NUMA_NO_NODE && !node_present_pages(node))
+ searchnode = numa_mem_id();
object = get_partial_node(s, get_node(s, searchnode), c, flags);
if (object || node != NUMA_NO_NODE)
return object;
@@ -2278,10 +2280,14 @@ redo:

if (unlikely(!node_match(page, node))) {
stat(s, ALLOC_NODE_MISMATCH);
- deactivate_slab(s, page, c->freelist);
- c->page = NULL;
- c->freelist = NULL;
- goto new_slab;
+ if (unlikely(!node_present_pages(node)))
+ node = numa_mem_id();
+ if (!node_match(page, node)) {
+ deactivate_slab(s, page, c->freelist);
+ c->page = NULL;
+ c->freelist = NULL;
+ goto new_slab;
+ }
}

/*