Re: [PATCH] virtio: avoid possible OOM lockup at virtballoon_oom_notify()

[Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>]

Tetsuo Handa wrote:
> Michael S. Tsirkin wrote:
> > > > 
> > > > The proper fix isn't that hard - just avoid allocations under lock.
> > > > 
> > > > Patch posted, pls take a look.
> > > 
> > > Your patch allocates pages in order to inflate the balloon, but
> > > your patch will allow leak_balloon() to deflate the balloon.
> > > How deflating the balloon (i.e. calling leak_balloon()) makes sense
> > > when allocating pages for inflating the balloon (i.e. calling
> > > fill_balloon()) ?
> > 
> > The idea is that fill_balloon is allocating memory with __GFP_NORETRY
> > so it will avoid disruptive actions like the OOM killer.
> > Under pressure it will normally fail and retry in half a second or so.
> > 
> > Calling leak_balloon in that situation could benefit the system as a whole.
> > 
> > I might be misunderstanding the meaning of the relevant GFP flags,
> > pls correct me if I'm wrong.
> 
> Would you answer to below question by "yes"/"no" ?
> 
>   If leak_balloon() is called via out_of_memory(), leak_balloon()
>   will decrease "struct virtio_balloon"->num_pages.
>   But, is "struct virtio_balloon_config"->num_pages updated when
>   leak_balloon() is called via out_of_memory() ?
> 
> Below explanation assumes that your answer is "no".
> 
> I consider that fill_balloon() is using __GFP_NORETRY is a bug.

Below are my test results using 4.14-rc5.

  http://I-love.SAKURA.ne.jp/tmp/20171016-default.log.xz
  http://I-love.SAKURA.ne.jp/tmp/20171016-deflate.log.xz

20171016-default.log.xz is without VIRTIO_BALLOON_F_DEFLATE_ON_OOM and
20171016-deflate.log.xz is with VIRTIO_BALLOON_F_DEFLATE_ON_OOM. (I used
inverting virtio_has_feature(VIRTIO_BALLOON_F_DEFLATE_ON_OOM) test
because the QEMU I'm using does not support deflate-on-oom option.)

> Consider an extreme situation that guest1 is started with 8192MB
> memory and then guest1's memory is reduced to 128MB by
> 
>   virsh qemu-monitor-command --domain guest1 --hmp 'balloon 128'
> 
> when VIRTIO_BALLOON_F_DEFLATE_ON_OOM was not negotiated.
> Of course, 128MB would be too small to operate guest1 properly.
> Since update_balloon_size_func() continues calling fill_balloon()
> until guest1's memory is reduced to 128MB, you will see flooding of
> "puff" messages (and guest1 is practically unusable because all CPU
> resource will be wasted for unsuccessful memory reclaim attempts)
> unless the OOM killer is invoked.

20171016-default.log.xz continued printing "puff" messages until kernel
panic caused by somebody else killing all OOM killable processes via
GFP_KERNEL allocation requests. Although fill_balloon() was printing
a lot of noises due to __GFP_NORETRY, the system made forward progress
in the form of kernel panic triggered by no more OOM killable processes.

----------
[   88.270838] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   88.503496] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   88.730058] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   88.937971] Out of memory: Kill process 669 (dhclient) score 54 or sacrifice child
[   89.007322] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   89.234874] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   89.439735] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   89.496389] Out of memory: Kill process 853 (tuned) score 47 or sacrifice child
[   89.663808] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   89.883812] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   90.104417] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   90.251293] Out of memory: Kill process 568 (polkitd) score 36 or sacrifice child
[   90.326131] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   90.821722] Out of memory: Kill process 601 (NetworkManager) score 23 or sacrifice child
[   91.293848] Out of memory: Kill process 585 (rsyslogd) score 13 or sacrifice child
[   91.799413] Out of memory: Kill process 415 (systemd-journal) score 13 or sacrifice child
[   91.861974] Out of memory: Kill process 987 (qmgr) score 6 or sacrifice child
[   91.925297] Out of memory: Kill process 985 (master) score 6 or sacrifice child
[   92.254082] Out of memory: Kill process 985 (master) score 6 or sacrifice child
[   92.464029] Out of memory: Kill process 881 (login) score 4 or sacrifice child
[   92.467859] Out of memory: Kill process 881 (login) score 4 or sacrifice child
[   93.490928] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   93.713220] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   93.932145] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   94.147652] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   94.363826] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   94.606404] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   94.833539] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   95.053230] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   95.267805] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   95.483789] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   98.290124] Out of memory: Kill process 595 (crond) score 3 or sacrifice child
[   98.643588] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   98.864487] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   99.084685] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   99.299766] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   99.515745] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   99.758334] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   99.985175] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  100.204474] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  100.419757] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  100.635681] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  101.263840] Out of memory: Kill process 587 (systemd-logind) score 2 or sacrifice child
[  101.319432] Out of memory: Kill process 586 (irqbalance) score 1 or sacrifice child
[  101.386546] Out of memory: Kill process 569 (dbus-daemon) score 0 or sacrifice child
[  103.805754] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  104.033073] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  104.253104] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  104.467810] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  104.683792] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  104.926409] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  105.153490] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  105.372610] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  105.587751] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  105.803719] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  108.958882] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  109.185254] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  109.404393] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  109.621584] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  109.697419] Kernel panic - not syncing: Out of memory and no killable processes...
[  109.723710] ---[ end Kernel panic - not syncing: Out of memory and no killable processes...
----------

> 
> What this patch is trying to handle is a situation when
> VIRTIO_BALLOON_F_DEFLATE_ON_OOM was negotiated. Once
> update_balloon_size_func() started calling fill_balloon(),
> update_balloon_size_func() will continue calling fill_balloon()
> until guest1's memory is reduced to 128MB, won't it?
> 
> Since fill_balloon() uses __GFP_IO | __GFP_FS, fill_balloon() can
> indirectly trigger out_of_memory() despite __GFP_NORETRY is specified.
> 
> When update_balloon_size_func() is running for calling fill_balloon(),
> calling leak_balloon() will increase number of pages to fill which
> fill_balloon() is supposed to fill. Leaking some pages from leak_balloon()
> via blocking_notifier_call_chain() callback could avoid invocation of the
> OOM killer for that specific moment, but it bounces back to us later because
> number of pages to allocate later (note that update_balloon_size_func() is
> running for calling fill_balloon()) is increased by leak_balloon().

20171016-deflate.log.xz continued printing "puff" messages without any OOM
killer messages, for fill_balloon() always inflates faster than leak_balloon()
deflates.

Since the OOM killer cannot be invoked unless leak_balloon() completely
deflates faster than fill_balloon() inflates, the guest remained unusable
(e.g. unable to login via ssh) other than printing "puff" messages.
This result was worse than 20171016-default.log.xz , for the system was
not able to make any forward progress (i.e. complete OOM lockup).

----------
[   19.866938] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   20.089350] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   20.430965] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   20.652641] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   20.894680] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   21.122063] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   21.340872] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   21.556128] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   21.772473] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   22.014960] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   24.972961] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   25.201565] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   25.420875] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   25.636081] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   25.851670] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   26.095842] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   26.322284] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[   26.851691] virtio_balloon virtio3: Out of puff! Can't get 1 pages
(...snipped...)
[  211.748567] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  211.963910] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  215.157737] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  215.385239] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  215.604426] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  215.819807] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  216.036491] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  216.278718] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  216.505410] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  216.724334] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  216.940318] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  217.155560] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  220.312187] virtio_balloon virtio3: Out of puff! Can't get 1 pages
[  220.342980] sysrq: SysRq : Resetting
----------

> 
> Thus, I don't think that avoid invoking the OOM killer by calling leak_balloon()
> makes sense when update_balloon_size_func() is running for calling fill_balloon().
> And this patch tries to detect it by replacing mutex_lock() with mutex_trylock().
> 
> > Well the point of this flag is that when it's acked,
> > host knows that it's safe to inflate the balloon
> > to a large portion of guest memory and this won't
> > cause an OOM situation.
> 
> Assuming that your answer to the question is "no", I don't think it is
> safe to inflate the balloon to a large portion of guest memory, for once
> update_balloon_size_func() started calling fill_balloon(),
> update_balloon_size_func() can not stop calling fill_balloon() even when
> blocking_notifier_call_chain() callback called leak_balloon() because
> "struct virtio_balloon_config"->num_pages will not be updated when
> blocking_notifier_call_chain() callback called leak_balloon().

As I demonstrated above, VIRTIO_BALLOON_F_DEFLATE_ON_OOM can lead to complete
OOM lockup because out_of_memory() => fill_balloon() => out_of_memory() =>
fill_balloon() sequence can effectively disable the OOM killer when the host
assumed that it's safe to inflate the balloon to a large portion of guest
memory and this won't cause an OOM situation.

> > I think the assumption is that it fill back up eventually
> > when guest does have some free memory.

So, my question again because such assumption is broken.
What is the expected behavior after deflating while inflating?
How should "struct virtio_balloon_config"->num_pages be interpreted?

  struct virtio_balloon_config {
  	/* Number of pages host wants Guest to give up. */
  	__u32 num_pages;
  	/* Number of pages we've actually got in balloon. */
  	__u32 actual;
  };

If leak_balloon() from out_of_memory() should be stronger than
fill_balloon() from update_balloon_size_func(), we need to make
sure that update_balloon_size_func() stops calling fill_balloon()
when leak_balloon() was called from out_of_memory().

If fill_balloon() from update_balloon_size_func() should be
stronger than leak_balloon() from out_of_memory(), we need to make
sure that leak_balloon() from out_of_memory() is ignored when
fill_balloon() from update_balloon_size_func() is running.

Apart from vb->balloon_lock deadlock avoidance, we need to define
the expected behavior.

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