Messages in this thread | | | Date | Tue, 26 Apr 2011 14:59:40 +0100 | From | Mel Gorman <> | Subject | Re: [PATCH 02/13] mm: sl[au]b: Add knowledge of PFMEMALLOC reserve pages |
| |
On Tue, Apr 26, 2011 at 09:37:58PM +1000, NeilBrown wrote: > On Tue, 26 Apr 2011 08:36:43 +0100 Mel Gorman <mgorman@suse.de> wrote: > > > + /* > > + * If there are full empty slabs and we were not forced to > > + * allocate a slab, mark this one !pfmemalloc > > + */ > > + l3 = cachep->nodelists[numa_mem_id()]; > > + if (!list_empty(&l3->slabs_free) && force_refill) { > > + struct slab *slabp = virt_to_slab(objp); > > + slabp->pfmemalloc = false; > > + clear_obj_pfmemalloc(&objp); > > + check_ac_pfmemalloc(cachep, ac); > > + return objp; > > + } > > The comment doesn't match the code. I think you need to remove the words > "full" and "not" assuming the code is correct which it probably is... >
I'll fix up the comment, you're right, it's confusing.
> But the code seems to be much more complex than Peter's original, and I don't > see the gain. >
You're right, it is more complex.
> Peter's code had only one 'reserved' flag for each kmem_cache.
The reserve was set in a per-cpu structure so there was a "lag" time before that information was available to other CPUs. Fine on smaller machines but a bit more of a problem today.
> You seem to > have one for every slab. I don't see the point. > It is true that yours is in some sense more fair - but I'm not sure the > complexity is worth it. >
More fairness was one of the objects.
> Was there some particular reason you made the change? >
This version survives under considerably more stress than Peter's original version did without requiring the additional complexity of memory reserves.
-- Mel Gorman SUSE Labs
| |