On Wed 24-09-14 11:08:57, Johannes Weiner wrote: > When attempting to charge pages, we first charge the memory counter > and then the memory+swap counter. If one of the counters is at its > limit, we enter reclaim, but if it's the memory+swap counter, reclaim > shouldn't swap because that wouldn't change the situation. However, > if the counters have the same limits, we never get to the memory+swap > limit. To know whether reclaim should swap or not, there is a state > flag that indicates whether the limits are equal and whether hitting > the memory limit implies hitting the memory+swap limit. > > Just try the memory+swap counter first.
OK, this makes sense and makes the reclaim code little bit more readable (). I would just add that the patch shouldn't have any visible effectes because that is not apparent from the description. > > Signed-off-by: Johannes Weiner <[email protected]> Acked-by: Michal Hocko <[email protected]> > --- > mm/memcontrol.c | 47 +++++++++++++---------------------------------- > 1 file changed, 13 insertions(+), 34 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 1ec22bf380d0..89c920156c2a 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -315,9 +315,6 @@ struct mem_cgroup { > /* OOM-Killer disable */ > int oom_kill_disable; > > - /* set when res.limit == memsw.limit */ > - bool memsw_is_minimum; > - > /* protect arrays of thresholds */ > struct mutex thresholds_lock; > > @@ -1804,8 +1801,6 @@ static unsigned long mem_cgroup_reclaim(struct > mem_cgroup *memcg, > > if (flags & MEM_CGROUP_RECLAIM_NOSWAP) > noswap = true; > - if (!(flags & MEM_CGROUP_RECLAIM_SHRINK) && memcg->memsw_is_minimum) > - noswap = true; > > for (loop = 0; loop < MEM_CGROUP_MAX_RECLAIM_LOOPS; loop++) { > if (loop) > @@ -2543,16 +2538,17 @@ retry: > goto done; > > size = batch * PAGE_SIZE; > - if (!res_counter_charge(&memcg->res, size, &fail_res)) { > - if (!do_swap_account) > + if (!do_swap_account || > + !res_counter_charge(&memcg->memsw, size, &fail_res)) { > + if (!res_counter_charge(&memcg->res, size, &fail_res)) > goto done_restock; > - if (!res_counter_charge(&memcg->memsw, size, &fail_res)) > - goto done_restock; > - res_counter_uncharge(&memcg->res, size); > + if (do_swap_account) > + res_counter_uncharge(&memcg->memsw, size); > + mem_over_limit = mem_cgroup_from_res_counter(fail_res, res); > + } else { > mem_over_limit = mem_cgroup_from_res_counter(fail_res, memsw); > flags |= MEM_CGROUP_RECLAIM_NOSWAP; > - } else > - mem_over_limit = mem_cgroup_from_res_counter(fail_res, res); > + } > > if (batch > nr_pages) { > batch = nr_pages; > @@ -3615,7 +3611,6 @@ static int mem_cgroup_resize_limit(struct mem_cgroup > *memcg, > unsigned long long val) > { > int retry_count; > - u64 memswlimit, memlimit; > int ret = 0; > int children = mem_cgroup_count_children(memcg); > u64 curusage, oldusage; > @@ -3642,24 +3637,16 @@ static int mem_cgroup_resize_limit(struct mem_cgroup > *memcg, > * We have to guarantee memcg->res.limit <= memcg->memsw.limit. > */ > mutex_lock(&set_limit_mutex); > - memswlimit = res_counter_read_u64(&memcg->memsw, RES_LIMIT); > - if (memswlimit < val) { > + if (res_counter_read_u64(&memcg->memsw, RES_LIMIT) < val) { > ret = -EINVAL; > mutex_unlock(&set_limit_mutex); > break; > } > > - memlimit = res_counter_read_u64(&memcg->res, RES_LIMIT); > - if (memlimit < val) > + if (res_counter_read_u64(&memcg->res, RES_LIMIT) < val) > enlarge = 1; > > ret = res_counter_set_limit(&memcg->res, val); > - if (!ret) { > - if (memswlimit == val) > - memcg->memsw_is_minimum = true; > - else > - memcg->memsw_is_minimum = false; > - } > mutex_unlock(&set_limit_mutex); > > if (!ret) > @@ -3684,7 +3671,7 @@ static int mem_cgroup_resize_memsw_limit(struct > mem_cgroup *memcg, > unsigned long long val) > { > int retry_count; > - u64 memlimit, memswlimit, oldusage, curusage; > + u64 oldusage, curusage; > int children = mem_cgroup_count_children(memcg); > int ret = -EBUSY; > int enlarge = 0; > @@ -3703,22 +3690,14 @@ static int mem_cgroup_resize_memsw_limit(struct > mem_cgroup *memcg, > * We have to guarantee memcg->res.limit <= memcg->memsw.limit. > */ > mutex_lock(&set_limit_mutex); > - memlimit = res_counter_read_u64(&memcg->res, RES_LIMIT); > - if (memlimit > val) { > + if (res_counter_read_u64(&memcg->res, RES_LIMIT) > val) { > ret = -EINVAL; > mutex_unlock(&set_limit_mutex); > break; > } > - memswlimit = res_counter_read_u64(&memcg->memsw, RES_LIMIT); > - if (memswlimit < val) > + if (res_counter_read_u64(&memcg->memsw, RES_LIMIT) < val) > enlarge = 1; > ret = res_counter_set_limit(&memcg->memsw, val); > - if (!ret) { > - if (memlimit == val) > - memcg->memsw_is_minimum = true; > - else > - memcg->memsw_is_minimum = false; > - } > mutex_unlock(&set_limit_mutex); > > if (!ret) > -- > 2.1.0 > -- Michal Hocko SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
