Re: [patch 7/8] vmscan: memcg-aware unevictable page rescue scanner

On Mon 13-06-11 12:30:32, Johannes Weiner wrote:

On Mon, Jun 13, 2011 at 11:42:03AM +0200, Michal Hocko wrote:

quoted

On Wed 01-06-11 08:25:18, Johannes Weiner wrote:

quoted

Once the per-memcg lru lists are exclusive, the unevictable page
rescue scanner can no longer work on the global zone lru lists.

This converts it to go through all memcgs and scan their respective
unevictable lists instead.

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>

Just a minor naming thing.

Other than that looks good to me.
Reviewed-by: Michal Hocko <redacted>

quoted

--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h

[...]

quoted

+struct page *mem_cgroup_lru_to_page(struct zone *zone, struct mem_cgroup *mem,
+				    enum lru_list lru)
+{
+	struct mem_cgroup_per_zone *mz;
+	struct page_cgroup *pc;
+
+	mz = mem_cgroup_zoneinfo(mem, zone_to_nid(zone), zone_idx(zone));
+	pc = list_entry(mz->lists[lru].prev, struct page_cgroup, lru);
+	return lookup_cgroup_page(pc);
+}
+

[...]

quoted

--- a/mm/vmscan.c
+++ b/mm/vmscan.c

@@ -3233,6 +3233,14 @@ void scan_mapping_unevictable_pages(struct address_space *mapping)
 
 }
 
+static struct page *lru_tailpage(struct zone *zone, struct mem_cgroup *mem,
+				 enum lru_list lru)
+{
+	if (mem)
+		return mem_cgroup_lru_to_page(zone, mem, lru);
+	return lru_to_page(&zone->lru[lru].list);
+}

Wouldn't it better to have those names consistent?
mem_cgroup_lru_tailpage vs lru_tailpage?

It's bad naming alright, but what is the wrapper for both of them
supposed to be called then?

Note that this function is only temporary, though, that's why I did
not spent much time on looking for a better name.

When the per-memcg lru lists finally become exclusive, this is removed
and the function converted to work on lruvecs.

Would you be okay with just adding an /* XXX */ to the function in
this patch that mentions that it's only temporary?

Sure. No biggie about the naming. It just hit my eyes during reading the
patch because while lru_tailpage is clear about which end of the list is
returned the memcg counterpart is not that clear.

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>