On Mon, 25 Jul 2011 15:47:40 +0200
Michal Hocko <mhocko@xxxxxxx> wrote:
> On Fri 22-07-11 11:17:03, Daisuke Nishimura wrote:
> > commit:22a668d7 introduced "memsw_is_minimum" flag, which becomes true when
> > mem_limit == memsw_limit. The flag is checked at the beginning of reclaim,
> > and "noswap" is set if the flag is true, because using swap is meaningless
> > in this case.
> >
> > This works well in most cases, but when we try to shrink mem_limit, which
> > is the same as memsw_limit now, we might fail to shrink mem_limit because
> > swap doesn't used.
> >
> > This patch fixes this behavior by:
> > - check MEM_CGROUP_RECLAIM_SHRINK at the begining of reclaim
> > - If it is set, don't set "noswap" flag even if memsw_is_minimum is true.
> >
> > Signed-off-by: Daisuke Nishimura <nishimura@xxxxxxxxxxxxxxxxx>
> > ---
> > mm/memcontrol.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index ce0d617..cf6bae8 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -1649,7 +1649,7 @@ static int mem_cgroup_hierarchical_reclaim(struct mem_cgroup *root_mem,
> > excess = res_counter_soft_limit_excess(&root_mem->res) >> PAGE_SHIFT;
> >
> > /* If memsw_is_minimum==1, swap-out is of-no-use. */
> > - if (!check_soft && root_mem->memsw_is_minimum)
> > + if (!check_soft && !shrink && root_mem->memsw_is_minimum)
>
> It took me a while until I understood how we can end up having both
> flags unset - because I saw them as complementary before. But this is
> the mem_cgroup_do_charge path that is affected.
>
> Btw. shouldn't we push that check into the loop. We could catch also
> memsw changes done (e.g. increased memsw limit in order to cope with the
> current workload) while we were reclaiming from a subgroup.
>
hmm, we shouldn't enable swap if the caller set MEM_CGROUP_RECLAIM_SOFT.
So, something like this ? I think it must be another patch anyway.
---
@@ -1640,7 +1640,7 @@ static int mem_cgroup_hierarchical_reclaim(struct mem_cgroup *root_m
struct mem_cgroup *victim;
int ret, total = 0;
int loop = 0;
- bool noswap = reclaim_options & MEM_CGROUP_RECLAIM_NOSWAP;
+ bool noswap;
bool shrink = reclaim_options & MEM_CGROUP_RECLAIM_SHRINK;
bool check_soft = reclaim_options & MEM_CGROUP_RECLAIM_SOFT;
unsigned long excess;
@@ -1648,11 +1648,15 @@ static int mem_cgroup_hierarchical_reclaim(struct mem_cgroup *root
excess = res_counter_soft_limit_excess(&root_mem->res) >> PAGE_SHIFT;
- /* If memsw_is_minimum==1, swap-out is of-no-use. */
- if (!check_soft && !shrink && root_mem->memsw_is_minimum)
- noswap = true;
-
while (1) {
+ if (reclaim_options & MEM_CGROUP_RECLAIM_NOSWAP)
+ noswap = true;
+ /* If memsw_is_minimum==1, swap-out is of-no-use. */
+ else if (!check_soft && !shrink && root_mem->memsw_is_minimum)
+ noswap = true;
+ else
+ noswap = false;
+
victim = mem_cgroup_select_victim(root_mem);
if (victim == root_mem) {
loop++;
---
> Anyway looks good.
> Reviewed-by: Michal Hocko <mhocko@xxxxxxx>
Thank you for your review!
Daisuke Nishimura.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. 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@xxxxxxxxx";> email@xxxxxxxxx </a>