Re: [ckrm-tech] [Patch 5/6] CKRM: Add config support for mem controller

[Dave Hansen <haveblue@us.ibm.com>]

There appears to still be some serious issues in the patch with respect
to per-zone accounting.  There is only accounting in each ckrm_mem_res
for each *kind* of zone, not each zone.

For instance, the accounting for a page appears to be the same no matter
which zone it came from, just which kind of zone

Then, when it comes to actually use some of the information, the kswapd
wakeup just throws a completely unrelated number into wakeup_kswapd().
ZONE_DMA (zone 0) tends to be *MUCH* smaller than ZONE_HIGHMEM, for
instance.  It doesn't make a whole lot of logical sense to me to be
waking up kswapd for a possibly 16GB zone with data from a 16MB zone.

+       for_each_zone(zone) {
+               /* This is just a number to get to wakeup kswapd */
+               order = cls->pg_total[0] -
+                       ((ckrm_mem_shrink_to * cls->pg_limit) / 100);
+               wakeup_kswapd(zone, order);
+               break; /* only once is enough */
+       }

If the number doesn't matter, why not just pass 0 into it?

Could you explain what advantages keeping a per-zone-type count has over
actually doing one count for each zone?  Also, why bother tracking it
per-zone-type anyway?  Would a single count work the same way?

-- Dave

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>