All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: Andrea Arcangeli <aarcange@redhat.com>
Cc: "Blue Swirl" <blauwirbel@gmail.com>,
	"Andreas Färber" <andreas.faerber@web.de>,
	"qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>,
	"Anthony Liguori" <aliguori@us.ibm.com>,
	"Alexander Graf" <agraf@suse.de>
Subject: Re: [Qemu-devel] [PATCH] add MADV_DONTFORK to guest physical memory
Date: Thu, 06 Jan 2011 14:49:06 -0600	[thread overview]
Message-ID: <4D262AC2.9000202@linux.vnet.ibm.com> (raw)
In-Reply-To: <20110106174925.GK15823@random.random>

On 01/06/2011 11:49 AM, Andrea Arcangeli wrote:
> On Wed, Jan 05, 2011 at 03:27:37PM -0600, Michael Roth wrote:
>> On 01/05/2011 02:35 PM, Andrea Arcangeli wrote:
>>> On Wed, Jan 05, 2011 at 02:26:19PM -0600, Michael Roth wrote:
>>>> Yah you're right, but I've seen several discussions about using mempath
>>>> for tmpfs/ram-backed files for things like numa/zram/etc so tend to
>>>> think of it as something potentially more than just a hook for
>>>> hugetlbfs, which is becoming less and less useful. But the MADV_DONTFORK
>>>> stuff should still be immediately applicable.
>>>
>>> Yes, MADV_DONTFORK should be set all on all guest physical memory
>>> without options so I hope the new patch I just posted is fine to stop
>>> the spurious -ENOMEM failures in fork.
>>
>> The patch in this thread? A couple paths still aren't covered when using
>> -mem-path. Something like this should get them all:
>
> Well the reason of MADV_DONTFORK is to avoid accounting issues with
> anonymous memory, mem-path don't have that issue as hugetlbfs skips
> the accounting (it has to because hugetlbfs are not always taken from
> the regular page allocator). It could be however considered a minor
> performance optimization.

That's one case, but there's also a wonky fallback in that path that 
defaults to the normal qemu_vmalloc():

if (mem_path) {
#if defined (__linux__) && !defined(TARGET_S390X)
             new_block->host = file_ram_alloc(new_block, size, mem_path);
             if (!new_block->host) {
                 new_block->host = qemu_vmalloc(size);
                 qemu_madvise(new_block->host, size, QEMU_MADV_MERGEABLE);
             }

May make sense to only add coverage for that specific case though. If 
file_ram_alloc() is generalized we could deal with it then.

> Now you mention that you want to use -mem-path for other things too,
> so maybe that's why you need it there too. BTW, if you ever need it
> for more than hugetlbfs, I'm afraid this MAP_PRIVATE I see when
> mem_prealloc isn't set, is going to screw any other potential useful
> usage besides hugetlbfs, not exactly sure why it makes any sense to
> use MAP_PRIVATE there and not only MAP_SHARED.

Not sure either...but if it's another hugetlbfs-ism it shouldn't matter 
since using mem-path for something other than hugetlbfs would ideally be 
configurable with a -mem path=/dev/shm/vm0.mem,hugetlbfs=off or 
something along that line, which wouldn't necessarily set MAP_PRIVATE.

  reply	other threads:[~2011-01-06 20:49 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-15 17:08 [Qemu-devel] [PATCH] add MADV_DONTFORK to guest physical memory Andrea Arcangeli
2010-09-15 17:34 ` Alexander Graf
2010-09-15 17:37 ` Andreas Färber
2011-01-05 15:10   ` Andrea Arcangeli
2011-01-05 18:02     ` Michael Roth
2011-01-05 19:44       ` Alexander Graf
2011-01-05 19:54         ` Andrea Arcangeli
2011-01-05 20:00           ` Alexander Graf
2011-01-05 20:12             ` Anthony Liguori
2011-01-05 20:15             ` Andrea Arcangeli
2011-01-05 20:26           ` Michael Roth
2011-01-05 20:35             ` Andrea Arcangeli
2011-01-05 21:27               ` Michael Roth
2011-01-06 17:49                 ` Andrea Arcangeli
2011-01-06 20:49                   ` Michael Roth [this message]
2011-01-05 20:10         ` Anthony Liguori
2010-09-15 21:03 ` [Qemu-devel] " Michael S. Tsirkin
2010-09-16  6:51   ` Gleb Natapov

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=4D262AC2.9000202@linux.vnet.ibm.com \
    --to=mdroth@linux.vnet.ibm.com \
    --cc=aarcange@redhat.com \
    --cc=agraf@suse.de \
    --cc=aliguori@us.ibm.com \
    --cc=andreas.faerber@web.de \
    --cc=blauwirbel@gmail.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.