qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrange" <berrange@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: qemu-devel@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
	Zack Cornelius <zack.cornelius@kove.net>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 0/5] hostmem-file: Add "persistent" option
Date: Fri, 11 Aug 2017 17:44:55 +0100	[thread overview]
Message-ID: <20170811164455.GA2554@redhat.com> (raw)
In-Reply-To: <20170811163300.GA29509@localhost.localdomain>

On Fri, Aug 11, 2017 at 01:33:00PM -0300, Eduardo Habkost wrote:
> CCing Zack Cornelius.
> 
> On Wed, Jun 14, 2017 at 05:29:55PM -0300, Eduardo Habkost wrote:
> > This series adds a new "persistent" option to
> > memory-backend-file.  The new option it will be useful if
> > somebody is sharing RAM contents on a file using share=on, but
> > don't need it to be flushed to disk when QEMU exits.
> > 
> > Internally, it will trigger a madvise(MADV_REMOVE) or
> > fallocate(FALLOC_FL_PUNCH_HOLE) call when the memory backend is
> > destroyed.
> > 
> > To make we actually trigger the new code when QEMU exits, the
> > first patch in the series ensures we destroy all user-created
> > objects when exiting QEMU.
> 
> So, before sending a new version of this, we need to clarify one
> thing: why exactly unlink()+close() wouldn't be enough to avoid
> having data unnecessarily flushed to the backing store and make
> the new option unnecessary?

If the backend file is shared between processes, unlinking
it feels bad - you're assuming no /future/ process wants to
attach to the file. Also if QEMU aborts for any reason, the
cleanup code is never going to run

> I would expect close() to not write any data unnecessarily if
> there are no remaining references to the file.  Why/when this is
> not the case?

Isn't the unlink() delayed until such time as *all* open handles
on that file are closed. If so, it seems that if 2 processes
have the file open, and one closes it, it is still valid for the
kernel to want to flush data out to the backing store if it needs
to free up working memory consumed by i/o cache.

If this wasn't the case, then one process could write 20 GB of data,
unlink + close the file, and that 20 GB would never be able to be
purge from I/O cache for as long as another process had that FD
open. That would be pretty bad denial of sevice for memory management
system.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

  reply	other threads:[~2017-08-11 16:45 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-14 20:29 [Qemu-devel] [PATCH 0/5] hostmem-file: Add "persistent" option Eduardo Habkost
2017-06-14 20:29 ` [Qemu-devel] [PATCH 1/5] vl: Clean up user-creatable objects when exiting Eduardo Habkost
2017-06-14 20:29 ` [Qemu-devel] [PATCH 2/5] memory: Allow RAM up to block->max_length to be discarded Eduardo Habkost
2017-06-22 11:47   ` Dr. David Alan Gilbert
2017-06-14 20:29 ` [Qemu-devel] [PATCH 3/5] memory: Add RAM_NONPERSISTENT flag Eduardo Habkost
2017-06-22 12:14   ` Dr. David Alan Gilbert
2017-06-22 17:27     ` Eduardo Habkost
2017-06-22 18:56       ` Dr. David Alan Gilbert
2017-06-14 20:29 ` [Qemu-devel] [PATCH 4/5] memory: Add 'persistent' parameter to memory_region_init_ram_from_file() Eduardo Habkost
2017-06-22 12:26   ` Dr. David Alan Gilbert
2017-06-22 12:41     ` Eduardo Habkost
2017-06-14 20:30 ` [Qemu-devel] [PATCH 5/5] hostmem-file: Add "persistent" option Eduardo Habkost
2017-06-14 21:50 ` [Qemu-devel] [PATCH 0/5] " no-reply
2017-07-06 18:47 ` Eduardo Habkost
2017-08-11 16:33 ` Eduardo Habkost
2017-08-11 16:44   ` Daniel P. Berrange [this message]
2017-08-11 18:15     ` Eduardo Habkost
2017-08-14  9:39       ` Daniel P. Berrange
2017-08-14 11:40         ` Eduardo Habkost
2017-08-14 18:33           ` Zack Cornelius

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=20170811164455.GA2554@redhat.com \
    --to=berrange@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=zack.cornelius@kove.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).