Re: [RFC] [PATCH] mm,oom: Offload OOM notify callback to a kernel thread.

[Michal Hocko <mhocko@kernel.org>]

[Hmm, I do not see the original patch which this has been a reply to]

On Mon 02-10-17 06:59:12, Michael S. Tsirkin wrote:
> On Sun, Oct 01, 2017 at 02:44:34PM +0900, Tetsuo Handa wrote:
> > Tetsuo Handa wrote:
> > > Michael S. Tsirkin wrote:
> > > > On Mon, Sep 11, 2017 at 07:27:19PM +0900, Tetsuo Handa wrote:
> > > > > Hello.
> > > > > 
> > > > > I noticed that virtio_balloon is using register_oom_notifier() and
> > > > > leak_balloon() from virtballoon_oom_notify() might depend on
> > > > > __GFP_DIRECT_RECLAIM memory allocation.
> > > > > 
> > > > > In leak_balloon(), mutex_lock(&vb->balloon_lock) is called in order to
> > > > > serialize against fill_balloon(). But in fill_balloon(),
> > > > > alloc_page(GFP_HIGHUSER[_MOVABLE] | __GFP_NOMEMALLOC | __GFP_NORETRY) is
> > > > > called with vb->balloon_lock mutex held. Since GFP_HIGHUSER[_MOVABLE] implies
> > > > > __GFP_DIRECT_RECLAIM | __GFP_IO | __GFP_FS, this allocation attempt might
> > > > > depend on somebody else's __GFP_DIRECT_RECLAIM | !__GFP_NORETRY memory
> > > > > allocation. Such __GFP_DIRECT_RECLAIM | !__GFP_NORETRY allocation can reach
> > > > > __alloc_pages_may_oom() and hold oom_lock mutex and call out_of_memory().
> > > > > And leak_balloon() is called by virtballoon_oom_notify() via
> > > > > blocking_notifier_call_chain() callback when vb->balloon_lock mutex is already
> > > > > held by fill_balloon(). As a result, despite __GFP_NORETRY is specified,
> > > > > fill_balloon() can indirectly get stuck waiting for vb->balloon_lock mutex
> > > > > at leak_balloon().

This is really nasty! And I would argue that this is an abuse of the oom
notifier interface from the virtio code. OOM notifiers are an ugly hack
on its own but all its users have to be really careful to not depend on
any allocation request because that is a straight deadlock situation.

I do not think that making oom notifier API more complex is the way to
go. Can we simply change the lock to try_lock? If the lock is held we
would simply fall back to the normal OOM handling. As a follow up it
would be great if virtio could use some other form of aging e.g.
shrinker.
-- 
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>