All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] When it's okay to treat OOM as fatal?
Date: Thu, 18 Oct 2018 15:54:06 +0100	[thread overview]
Message-ID: <20181018145406.GE2632@work-vm> (raw)
In-Reply-To: <87va5zjort.fsf@dusky.pond.sub.org>

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> We sometimes use g_new() & friends, which abort() on OOM, and sometimes
> >> g_try_new() & friends, which can fail, and therefore require error
> >> handling.
> >> 
> >> HACKING points out the difference, but is mum on when to use what:
> >> 
> >>     3. Low level memory management
> >> 
> >>     Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
> >>     APIs is not allowed in the QEMU codebase. Instead of these routines,
> >>     use the GLib memory allocation routines g_malloc/g_malloc0/g_new/
> >>     g_new0/g_realloc/g_free or QEMU's qemu_memalign/qemu_blockalign/qemu_vfree
> >>     APIs.
> >> 
> >>     Please note that g_malloc will exit on allocation failure, so there
> >>     is no need to test for failure (as you would have to with malloc).
> >>     Calling g_malloc with a zero size is valid and will return NULL.
> >> 
> >>     Prefer g_new(T, n) instead of g_malloc(sizeof(T) * n) for the following
> >>     reasons:
> >> 
> >>       a. It catches multiplication overflowing size_t;
> >>       b. It returns T * instead of void *, letting compiler catch more type
> >>          errors.
> >> 
> >>     Declarations like T *v = g_malloc(sizeof(*v)) are acceptable, though.
> >> 
> >>     Memory allocated by qemu_memalign or qemu_blockalign must be freed with
> >>     qemu_vfree, since breaking this will cause problems on Win32.
> >> 
> >> Now, in my personal opinion, handling OOM gracefully is worth the
> >> (commonly considerable) trouble when you're coding for an Apple II or
> >> similar.  Anything that pages commonly becomes unusable long before
> >> allocations fail.
> >
> > That's not always my experience; I've seen cases where you suddenly
> > allocate a load more memory and hit OOM fairly quickly on that hot
> > process.  Most of the time on the desktop you're right.
> >
> >> Anything that overcommits will send you a (commonly
> >> lethal) signal instead.  Anything that tries handling OOM gracefully,
> >> and manages to dodge both these bullets somehow, will commonly get it
> >> wrong and crash.
> >
> > If your qemu has maped it's main memory from hugetlbfs or similar pools
> > then we're looking at the other memory allocations; and that's a bit of
> > an interesting difference where those other allocations should be a lot
> > smaller.
> >
> >> But others are entitled to their opinions as much as I am.  I just want
> >> to know what our rules are, preferably in the form of a patch to
> >> HACKING.
> >
> > My rule is to try not to break a happily running VM by some new
> > activity; I don't worry about it during startup.
> >
> > So for example, I don't like it when starting a migration, allocates
> > some more memory and kills the VM - the user had a happy stable VM
> > upto that point.  Migration gets the blame at this point.
> 
> I don't doubt reliable OOM handling would be nice.  I do doubt it's
> practical for an application like QEMU.

Well, our use of glib certainly makes it much much harder.
I just try and make sure anywhere that I'm allocating a non-trivial
amount of memory (especially anything guest or user controlled) uses
the _try_ variants.  That should keep a lot of the larger allocations.
However, it scares me that we've got things that can return big chunks
of JSON for example, and I don't think they're being careful about it.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

  reply	other threads:[~2018-10-18 14:54 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-16 13:01 [Qemu-devel] When it's okay to treat OOM as fatal? Markus Armbruster
2018-10-16 13:20 ` Daniel P. Berrangé
2018-10-18 13:06   ` Markus Armbruster
2018-10-18 14:28     ` Paolo Bonzini
2018-10-16 13:33 ` Dr. David Alan Gilbert
2018-10-18 14:46   ` Markus Armbruster
2018-10-18 14:54     ` Dr. David Alan Gilbert [this message]
2018-10-18 17:26       ` Markus Armbruster
2018-10-18 18:01         ` Dr. David Alan Gilbert
2018-10-19  5:43           ` Markus Armbruster
2018-10-19 10:07             ` Dr. David Alan Gilbert
2018-10-22 13:40             ` Dr. David Alan Gilbert
2018-10-17 10:05 ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181018145406.GE2632@work-vm \
    --to=dgilbert@redhat.com \
    --cc=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.