Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread David Rientjes
On Wed, 4 Dec 2013, Johannes Weiner wrote: > However, the GFP_NOFS | __GFP_NOFAIL task stuck in the page allocator > may hold filesystem locks that could prevent a third party from > freeing memory and/or exiting, so we can not guarantee that only the > __GFP_NOFAIL task is getting stuck, it

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread Johannes Weiner
On Wed, Dec 04, 2013 at 03:34:17PM +1100, Dave Chinner wrote: > On Tue, Dec 03, 2013 at 10:01:01PM -0500, Johannes Weiner wrote: > > On Tue, Dec 03, 2013 at 03:40:13PM -0800, David Rientjes wrote: > > > On Tue, 3 Dec 2013, Johannes Weiner wrote: > > > I believe the page allocator would be

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread Dave Chinner
On Tue, Dec 03, 2013 at 10:01:01PM -0500, Johannes Weiner wrote: > On Tue, Dec 03, 2013 at 03:40:13PM -0800, David Rientjes wrote: > > On Tue, 3 Dec 2013, Johannes Weiner wrote: > > I believe the page allocator would be susceptible to the same deadlock if > > nothing else on the system can

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread Johannes Weiner
On Tue, Dec 03, 2013 at 03:40:13PM -0800, David Rientjes wrote: > On Tue, 3 Dec 2013, Johannes Weiner wrote: > > > > > Spin on which level? The whole point of this change was to not spin for > > > > ever because the caller might sit on top of other locks which might > > > > prevent somebody else

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread David Rientjes
On Tue, 3 Dec 2013, Johannes Weiner wrote: > > > Spin on which level? The whole point of this change was to not spin for > > > ever because the caller might sit on top of other locks which might > > > prevent somebody else to die although it has been killed. > > > > See my question about the

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread Johannes Weiner
On Mon, Dec 02, 2013 at 03:02:09PM -0800, David Rientjes wrote: > On Mon, 2 Dec 2013, Michal Hocko wrote: > > > > > What if the callers simply cannot deal with the allocation failure? > > > > 84235de394d97 (fs: buffer: move allocation failure loop into the > > > > allocator) describes one such

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread Johannes Weiner
On Mon, Dec 02, 2013 at 03:02:09PM -0800, David Rientjes wrote: On Mon, 2 Dec 2013, Michal Hocko wrote: What if the callers simply cannot deal with the allocation failure? 84235de394d97 (fs: buffer: move allocation failure loop into the allocator) describes one such case when

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread David Rientjes
On Tue, 3 Dec 2013, Johannes Weiner wrote: Spin on which level? The whole point of this change was to not spin for ever because the caller might sit on top of other locks which might prevent somebody else to die although it has been killed. See my question about the non-memcg page

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread Johannes Weiner
On Tue, Dec 03, 2013 at 03:40:13PM -0800, David Rientjes wrote: On Tue, 3 Dec 2013, Johannes Weiner wrote: Spin on which level? The whole point of this change was to not spin for ever because the caller might sit on top of other locks which might prevent somebody else to die

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread Dave Chinner
On Tue, Dec 03, 2013 at 10:01:01PM -0500, Johannes Weiner wrote: On Tue, Dec 03, 2013 at 03:40:13PM -0800, David Rientjes wrote: On Tue, 3 Dec 2013, Johannes Weiner wrote: I believe the page allocator would be susceptible to the same deadlock if nothing else on the system can reclaim

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread Johannes Weiner
On Wed, Dec 04, 2013 at 03:34:17PM +1100, Dave Chinner wrote: On Tue, Dec 03, 2013 at 10:01:01PM -0500, Johannes Weiner wrote: On Tue, Dec 03, 2013 at 03:40:13PM -0800, David Rientjes wrote: On Tue, 3 Dec 2013, Johannes Weiner wrote: I believe the page allocator would be susceptible to

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-03 Thread David Rientjes
On Wed, 4 Dec 2013, Johannes Weiner wrote: However, the GFP_NOFS | __GFP_NOFAIL task stuck in the page allocator may hold filesystem locks that could prevent a third party from freeing memory and/or exiting, so we can not guarantee that only the __GFP_NOFAIL task is getting stuck, it might

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-02 Thread David Rientjes
On Mon, 2 Dec 2013, Michal Hocko wrote: > > > What if the callers simply cannot deal with the allocation failure? > > > 84235de394d97 (fs: buffer: move allocation failure loop into the > > > allocator) describes one such case when __getblk_slow tries desperately > > > to grow buffers relying on

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-02 Thread Michal Hocko
On Fri 29-11-13 15:46:16, David Rientjes wrote: > On Thu, 28 Nov 2013, Michal Hocko wrote: > > > > Ok, so let's forget about GFP_KERNEL | __GFP_NOFAIL since anything doing > > > __GFP_FS should not be holding such locks, we have some of those in the > > > drivers code and that makes sense that

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-02 Thread Michal Hocko
On Fri 29-11-13 15:46:16, David Rientjes wrote: On Thu, 28 Nov 2013, Michal Hocko wrote: Ok, so let's forget about GFP_KERNEL | __GFP_NOFAIL since anything doing __GFP_FS should not be holding such locks, we have some of those in the drivers code and that makes sense that they are

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-12-02 Thread David Rientjes
On Mon, 2 Dec 2013, Michal Hocko wrote: What if the callers simply cannot deal with the allocation failure? 84235de394d97 (fs: buffer: move allocation failure loop into the allocator) describes one such case when __getblk_slow tries desperately to grow buffers relying on the reclaim

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-29 Thread David Rientjes
On Thu, 28 Nov 2013, Michal Hocko wrote: > > Ok, so let's forget about GFP_KERNEL | __GFP_NOFAIL since anything doing > > __GFP_FS should not be holding such locks, we have some of those in the > > drivers code and that makes sense that they are doing GFP_KERNEL. > > > > Focusing on the

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-29 Thread David Rientjes
On Thu, 28 Nov 2013, Michal Hocko wrote: Ok, so let's forget about GFP_KERNEL | __GFP_NOFAIL since anything doing __GFP_FS should not be holding such locks, we have some of those in the drivers code and that makes sense that they are doing GFP_KERNEL. Focusing on the GFP_NOFS |

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-28 Thread Michal Hocko
On Wed 27-11-13 15:34:24, David Rientjes wrote: > On Wed, 27 Nov 2013, Johannes Weiner wrote: > > > > We don't give __GFP_NOFAIL allocations access to memory reserves in the > > > page allocator and we do call the oom killer for them so that a process > > > is > > > killed so that memory is

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-28 Thread Michal Hocko
On Wed 27-11-13 15:34:24, David Rientjes wrote: On Wed, 27 Nov 2013, Johannes Weiner wrote: We don't give __GFP_NOFAIL allocations access to memory reserves in the page allocator and we do call the oom killer for them so that a process is killed so that memory is freed. Why do

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-27 Thread David Rientjes
On Wed, 27 Nov 2013, Johannes Weiner wrote: > > We don't give __GFP_NOFAIL allocations access to memory reserves in the > > page allocator and we do call the oom killer for them so that a process is > > killed so that memory is freed. Why do we have a different policy for > > memcg? > > Oh

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-27 Thread Johannes Weiner
On Wed, Nov 27, 2013 at 01:38:59PM -0800, David Rientjes wrote: > On Wed, 27 Nov 2013, Johannes Weiner wrote: > > > > Ah, this is because of 3168ecbe1c04 ("mm: memcg: use proper memcg in > > > limit > > > bypass") which just bypasses all of these allocations and charges the > > > root > > >

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-27 Thread David Rientjes
On Wed, 27 Nov 2013, Johannes Weiner wrote: > > Ah, this is because of 3168ecbe1c04 ("mm: memcg: use proper memcg in limit > > bypass") which just bypasses all of these allocations and charges the root > > memcg. So if allocations want to bypass memcg isolation they just have to > > be

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-27 Thread Johannes Weiner
On Tue, Nov 26, 2013 at 07:33:12PM -0800, David Rientjes wrote: > On Tue, 26 Nov 2013, David Rientjes wrote: > > > On Fri, 22 Nov 2013, Johannes Weiner wrote: > > > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > > index 13b9d0f..cc4f9cb 100644 > > > --- a/mm/memcontrol.c > > > +++

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-27 Thread Johannes Weiner
On Tue, Nov 26, 2013 at 07:33:12PM -0800, David Rientjes wrote: On Tue, 26 Nov 2013, David Rientjes wrote: On Fri, 22 Nov 2013, Johannes Weiner wrote: diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 13b9d0f..cc4f9cb 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-27 Thread David Rientjes
On Wed, 27 Nov 2013, Johannes Weiner wrote: Ah, this is because of 3168ecbe1c04 (mm: memcg: use proper memcg in limit bypass) which just bypasses all of these allocations and charges the root memcg. So if allocations want to bypass memcg isolation they just have to be __GFP_NOFAIL?

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-27 Thread Johannes Weiner
On Wed, Nov 27, 2013 at 01:38:59PM -0800, David Rientjes wrote: On Wed, 27 Nov 2013, Johannes Weiner wrote: Ah, this is because of 3168ecbe1c04 (mm: memcg: use proper memcg in limit bypass) which just bypasses all of these allocations and charges the root memcg. So if

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-27 Thread David Rientjes
On Wed, 27 Nov 2013, Johannes Weiner wrote: We don't give __GFP_NOFAIL allocations access to memory reserves in the page allocator and we do call the oom killer for them so that a process is killed so that memory is freed. Why do we have a different policy for memcg? Oh boy, that's

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-26 Thread David Rientjes
On Tue, 26 Nov 2013, David Rientjes wrote: > On Fri, 22 Nov 2013, Johannes Weiner wrote: > > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 13b9d0f..cc4f9cb 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -2677,6 +2677,9 @@ static int __mem_cgroup_try_charge(struct

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-26 Thread David Rientjes
On Fri, 22 Nov 2013, Johannes Weiner wrote: > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 13b9d0f..cc4f9cb 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -2677,6 +2677,9 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm, > if

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-26 Thread David Rientjes
On Fri, 22 Nov 2013, Johannes Weiner wrote: diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 13b9d0f..cc4f9cb 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2677,6 +2677,9 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm, if (unlikely(task_in_memcg_oom(current)))

Re: [patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-26 Thread David Rientjes
On Tue, 26 Nov 2013, David Rientjes wrote: On Fri, 22 Nov 2013, Johannes Weiner wrote: diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 13b9d0f..cc4f9cb 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -2677,6 +2677,9 @@ static int __mem_cgroup_try_charge(struct mm_struct

[patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-22 Thread Johannes Weiner
84235de394d9 ("fs: buffer: move allocation failure loop into the allocator") started recognizing __GFP_NOFAIL in memory cgroups but forgot to disable the OOM killer. Any task that does not fail allocation will also not enter the OOM completion path. So don't declare an OOM state in this case or

[patch] mm: memcg: do not declare OOM from __GFP_NOFAIL allocations

2013-11-22 Thread Johannes Weiner
84235de394d9 (fs: buffer: move allocation failure loop into the allocator) started recognizing __GFP_NOFAIL in memory cgroups but forgot to disable the OOM killer. Any task that does not fail allocation will also not enter the OOM completion path. So don't declare an OOM state in this case or