Re: [patch 2/3] memcg: prevent endless loop when charging huge pages to near-limit group
From: Andrew Morton <akpm@linux-foundation.org>
Date: 2011-01-31 22:42:21
Also in:
lkml
On Mon, 31 Jan 2011 15:03:54 +0100 Johannes Weiner [off-list ref] wrote:
quoted hunk ↗ jump to hunk
+static inline bool res_counter_check_margin(struct res_counter *cnt, + unsigned long bytes) +{ + bool ret; + unsigned long flags; + + spin_lock_irqsave(&cnt->lock, flags); + ret = cnt->limit - cnt->usage >= bytes; + spin_unlock_irqrestore(&cnt->lock, flags); + return ret; +} + static inline bool res_counter_check_under_soft_limit(struct res_counter *cnt) { bool ret;diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 73ea323..c28072f 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c@@ -1111,6 +1111,15 @@ static bool mem_cgroup_check_under_limit(struct mem_cgroup *mem) return false; } +static bool mem_cgroup_check_margin(struct mem_cgroup *mem, unsigned long bytes) +{ + if (!res_counter_check_margin(&mem->res, bytes)) + return false; + if (do_swap_account && !res_counter_check_margin(&mem->memsw, bytes)) + return false; + return true; +}
argh. If you ever have a function with the string "check" in its name, it's a good sign that you did something wrong. Check what? Against what? Returning what? mem_cgroup_check_under_limit() isn't toooo bad - the name tells you what's being checked and tells you what to expect the return value to mean. But "res_counter_check_margin" and "mem_cgroup_check_margin" are just awful. Something like bool res_counter_may_charge(counter, bytes) would be much clearer. If we really want to stick with the "check" names (perhaps as an ironic reference to res_counter's past mistakes) then please at least document the sorry things? -- 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 policy in Canada: sign http://dissolvethecrtc.ca/ Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>