Re: [LSF/MM TOPIC ATTEND]

[Andrea Arcangeli <aarcange@redhat.com>]

Hi Michal,

On Thu, Jan 15, 2015 at 03:06:54PM +0100, Michal Hocko wrote:
> 
> > http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?id=a0fcf2323b2e4cffd750c1abc1d2c138acdefcc8
> 
> I am not sure about this one because TIF_MEMDIE is there to give an
> access to memory reserves. GFP_NOFAIL shouldn't mean the same because
> then it would be much harder to "guarantee" that the reserves wouldn't
> be depleted completely. So I do not like this much. Besides that I think
> that GFP_NOFAIL allocation blocking OOM victim is a plain bug.
> grow_dev_page is relying on GFP_NOFAIL but I am wondering whether ext4
> can do something to pre-allocate so that it doesn't have to call it.

Well this is just the longstanding GFP_NOFAIL livelock, it always
existed deep down in the buffer header allocation even before
GFP_NOFAIL existed. GFP_NOFAIL just generalized the livelocking
concept.

There's no proper fix for that other than to teach the filesystem to
deal with allocation errors and remove GFP_NOFAIL (in this case
__GFP_NOFAIL was set:

 #0 get_page_from_freelist (gfp_mask=0x20858, nodemask=0x0
  <irq_stack_union>, order=0x0, zonelist=0xffff88007fffc100, hi
  gh_zoneidx=0x2, alloc_flags=0xc0, preferred_zone=0xffff88007fffa840,
  classzone_idx=classzone_idx@entry=0x1,
  migratetype=migratetype@entry=0x2) at mm/page_alloc.c:1953

gfp_mask=0x20858  & 0x800u = 0x800

If we're OOM and GFP_NOFAIL actually fails to allocate memory, this
patch simply tries to mitigate the potential livelock by giving it a
chance to use the memory reserves that are normally used only for high
priority allocations.

If __GFP_NOFAIL hits OOM I think it's fair to say it is very high
priority (more high priority than a GFP_ATOMIC or something that can
fail totally gracefully).

So the above second patch looks quite safe to me conceptually as well:
at least we put those last 50M of ram to good use instead of
livelocking while 50M are still free.

> > http://git.kernel.org/cgit/linux/kernel/git/andrea/aa.git/commit/?id=798b7f9d549664f8c0007c6416a2568eedd75d6a
> 
> I think this should be fixed in the filesystem rather than paper over
> it.

No doubt, this third patch basically undoes the fix in the first
patch. This third patch makes __GFP_FS again not failing if invoked in
kernel thread context where TIF_MEMDIE cannot ever be set.

However there was no way I could run without this third patch on my
own production systems with a potential ext4 mounting itself readonly
on me during OOM killing (as result of the livelock fix in the first
patch).

Ideally the ext4 developer should reverse this third patch (which must
be keep separated from the first patch exactly for this reason) and
start an OOM killing loop to reproduce and fix this so then we can
reverse the third patch.

In short:

1) first patch makes !__GFP_FS not equivalent to __GFP_NOFAIL anymore
   (when invoked by kernel threads where TIF_MEMDIE cannot be set)

2) second patch deals with a genuine __GFP_NOFAIL livelock using the
   memory reserves (this is orthogonal with 1)

3) third patch undoes 1 and uses the memory reserves for !__GFP_FS too
   like patch 2 used them to mitigate the genuine __GFP_NOFAIL
   deadlock.  Undoing patch 1 is needed because patch 1 causes ext4 to
   remount itself readonly and complain about metadata corruption.

I later tested further the ext4 trouble after applying only 1 and it
seems ext4 thinks it's corrupted, but e2fsck -f shows it's actually
clean. So it's probably an in-memory issue only, but still having ext4
remounting itself readonly during a OOM killing isn't exactly
acceptable or graceful (until it is fixed). Hence the reason for 3.

Of course it took a long time before the trouble with patch 1 seen the
light, in fact first I hit the genuine __GFP_NOFAIL deadlock fixed by
2 before I could ever hit the ext4 error paths.

Let me know what you'd like me to submit, I just don't think
submitting only the first patch as you suggested is safe idea.

I also think allowing __GFP_NOFAIL to access the emergency reserves is
ok if __GFP_NOFAIL is hitting an OOM condition (what else could be
more urgent than succeeding a potentially livelocking __GFP_NOFAIL?).

I think the combination of the 3 patches is safe and in practice it
solves all OOM related livelocks I run into. It also allows ext4
developers to trivially (git reverse #ofpatch3) to fix their bugs and
then we can reverse the third patch upstream so !__GFP_FS allocations
from kernel threads becomes theoretically safe too. As opposed
__GFP_NOFAIL is never theoretically safe but that's much harder to fix
than the already existing ext4 error paths that aren't using
__GFP_NOFAIL but that haven't been properly exercised, simply because
they couldn't be exercised without patch 1 applied (kernel thread
allocations without __GFP_FS set cannot fail currently and making them
fail by applying patch1, exercises those untested error paths for the
first time).

Thanks!
Andrea

--
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>