[PATCH] mm: memcontrol: fix the return in mem_cgroup_margin

[PATCH] mm: memcontrol: fix the return in mem_cgroup_margin

[roy.qing.li]

From: Li RongQing <roy.qing.li@gmail.com>

when memory+swap is over limit, return 0

Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
 mm/memcontrol.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index fe787f5..e9211c2 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1090,6 +1090,8 @@ static unsigned long mem_cgroup_margin(struct mem_cgroup *memcg)
 		limit = READ_ONCE(memcg->memsw.limit);
 		if (count <= limit)
 			margin = min(margin, limit - count);
+		else
+			margin = 0;
 	}
 
 	return margin;
-- 
2.1.4

--
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:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: memcontrol: fix the return in mem_cgroup_margin

[Michal Hocko]

On Wed 18-05-16 15:24:15, roy.qing.li@gmail.com wrote:
> From: Li RongQing <roy.qing.li@gmail.com>
> 
> when memory+swap is over limit, return 0
> 
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> ---
>  mm/memcontrol.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index fe787f5..e9211c2 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -1090,6 +1090,8 @@ static unsigned long mem_cgroup_margin(struct mem_cgroup *memcg)
>  		limit = READ_ONCE(memcg->memsw.limit);
>  		if (count <= limit)
>  			margin = min(margin, limit - count);
> +		else
> +			margin = 0;

count should always be smaller than memsw.limit (this is a hard limit).
Even if we have some temporary breach then the code should work as
expected because margin is initialized to 0 and memsw.limit >= limit.

Or have you seen any real problem with this code path?

-- 
Michal Hocko
SUSE Labs

--
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:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: memcontrol: fix the return in mem_cgroup_margin

[Li RongQing]

On Wed, May 18, 2016 at 3:32 PM, Michal Hocko <mhocko@kernel.org> wrote:
> Or have you seen any real problem with this code path?


not;
I read the codes, think it is more reasonable to set margin to 0 here

-Roy

--
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:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: memcontrol: fix the return in mem_cgroup_margin

[Li RongQing]

On Wed, May 18, 2016 at 3:32 PM, Michal Hocko <mhocko@kernel.org> wrote:
> count should always be smaller than memsw.limit (this is a hard limit).
> Even if we have some temporary breach then the code should work as
> expected because margin is initialized to 0 and memsw.limit >= limit.

is it possible for this case? for example

memory count is 500, memory limit is 600; the margin is set to 100 firstly,
then check memory+swap limit, its count(1100) is bigger than its limit(1000),
then the margin 100 is returned wrongly.


-Roy

--
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:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: memcontrol: fix the return in mem_cgroup_margin

[Vladimir Davydov]

On Thu, May 19, 2016 at 09:44:53AM +0800, Li RongQing wrote:
> On Wed, May 18, 2016 at 3:32 PM, Michal Hocko <mhocko@kernel.org> wrote:
> > count should always be smaller than memsw.limit (this is a hard limit).
> > Even if we have some temporary breach then the code should work as
> > expected because margin is initialized to 0 and memsw.limit >= limit.
> 
> is it possible for this case? for example
> 
> memory count is 500, memory limit is 600; the margin is set to 100 firstly,
> then check memory+swap limit, its count(1100) is bigger than its limit(1000),
> then the margin 100 is returned wrongly.

I guess it is possible, because try_charge forces charging __GFP_NOFAIL
allocations, which may result in memsw.limit excess. If we are below
memory.limit and there's nothing to reclaim to reduce memsw.usage, we
might end up looping in try_charge forever. I've never seen that happen
in practice, but I still think the patch is worth applying.

Acked-by: Vladimir Davydov <vdavydov@virtuozzo.com>

--
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:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: memcontrol: fix the return in mem_cgroup_margin

[Michal Hocko]

On Mon 23-05-16 13:37:58, Vladimir Davydov wrote:
> On Thu, May 19, 2016 at 09:44:53AM +0800, Li RongQing wrote:
> > On Wed, May 18, 2016 at 3:32 PM, Michal Hocko <mhocko@kernel.org> wrote:
> > > count should always be smaller than memsw.limit (this is a hard limit).
> > > Even if we have some temporary breach then the code should work as
> > > expected because margin is initialized to 0 and memsw.limit >= limit.
> > 
> > is it possible for this case? for example
> > 
> > memory count is 500, memory limit is 600; the margin is set to 100 firstly,
> > then check memory+swap limit, its count(1100) is bigger than its limit(1000),
> > then the margin 100 is returned wrongly.
> 
> I guess it is possible, because try_charge forces charging __GFP_NOFAIL
> allocations, which may result in memsw.limit excess. If we are below
> memory.limit and there's nothing to reclaim to reduce memsw.usage, we
> might end up looping in try_charge forever. I've never seen that happen
> in practice, but I still think the patch is worth applying.

You are right. I have completely missed a potential interaction with
__GFP_NOFAIL. We even do not seem to trigger the memcg OOM killer for
these requests to sort the situation out.

Can we have updated patch with all this useful information in the
changelog, please?

Thanks Vladimir!
-- 
Michal Hocko
SUSE Labs

--
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:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: memcontrol: fix the return in mem_cgroup_margin

[Michal Hocko]

On Mon 23-05-16 14:06:20, Michal Hocko wrote:
[...]
> I have completely missed a potential interaction with
> __GFP_NOFAIL. We even do not seem to trigger the memcg OOM killer for
> these requests to sort the situation out.

Which is intentional. a0d8b00a3381 ("mm: memcg: do not declare OOM from
__GFP_NOFAIL allocations")

-- 
Michal Hocko
SUSE Labs

--
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:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] mm: memcontrol: fix the return in mem_cgroup_margin

[Li RongQing]

On Mon, May 23, 2016 at 8:06 PM, Michal Hocko <mhocko@kernel.org> wrote:
>
> Can we have updated patch with all this useful information in the
> changelog, please?


Ok, I will update this patch

-Roy

--
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:"dont@kvack.org"> email@kvack.org </a>