From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41501) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dO1CO-0004ZO-06 for qemu-devel@nongnu.org; Thu, 22 Jun 2017 08:27:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dO1CI-0000gO-W1 for qemu-devel@nongnu.org; Thu, 22 Jun 2017 08:27:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41246) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dO1CI-0000gD-MY for qemu-devel@nongnu.org; Thu, 22 Jun 2017 08:26:54 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 966E3C0587D8 for ; Thu, 22 Jun 2017 12:26:53 +0000 (UTC) Date: Thu, 22 Jun 2017 13:26:49 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20170622122649.GD2624@work-vm> References: <20170614203000.19984-1-ehabkost@redhat.com> <20170614203000.19984-5-ehabkost@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170614203000.19984-5-ehabkost@redhat.com> Subject: Re: [Qemu-devel] [PATCH 4/5] memory: Add 'persistent' parameter to memory_region_init_ram_from_file() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Paolo Bonzini , Igor Mammedov * Eduardo Habkost (ehabkost@redhat.com) wrote: > Make it possible to set the RAM_NONPERSISTENT flag on the RAMBlock when > mapping a file. > > Signed-off-by: Eduardo Habkost Reviewed-by: Dr. David Alan Gilbert A little confusing having a persistent flag passed in but setting a NONPERSISTENT flag. One thing to watch out for is the other effect, for example I think doing an MADV_REMOVE will zero the file; if that's a ROM file that would probably be bad; but your description of persistent=false might make sense for a ROM file since it wont be changed. Dave > --- > include/exec/memory.h | 4 ++++ > include/exec/ram_addr.h | 4 ++-- > backends/hostmem-file.c | 2 +- > exec.c | 7 +++++-- > memory.c | 4 +++- > numa.c | 2 +- > 6 files changed, 16 insertions(+), 7 deletions(-) > > diff --git a/include/exec/memory.h b/include/exec/memory.h > index 80e605a96a..f47c534179 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -446,6 +446,9 @@ void memory_region_init_resizeable_ram(MemoryRegion *mr, > * must be unique within any device > * @size: size of the region. > * @share: %true if memory must be mmaped with the MAP_SHARED flag > + * @persistent: %false if RAM contents can be discarded and don't > + * need to be flushed to disk when the memory region > + * is freed. > * @path: the path in which to allocate the RAM. > * @errp: pointer to Error*, to store an error if it happens. > */ > @@ -454,6 +457,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, > const char *name, > uint64_t size, > bool share, > + bool persistent, > const char *path, > Error **errp); > #endif > diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h > index 140efa840c..67dc099355 100644 > --- a/include/exec/ram_addr.h > +++ b/include/exec/ram_addr.h > @@ -63,8 +63,8 @@ static inline void *ramblock_ptr(RAMBlock *block, ram_addr_t offset) > long qemu_getrampagesize(void); > unsigned long last_ram_page(void); > RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, > - bool share, const char *mem_path, > - Error **errp); > + bool share, bool persistent, > + const char *mem_path, Error **errp); > RAMBlock *qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, > MemoryRegion *mr, Error **errp); > RAMBlock *qemu_ram_alloc(ram_addr_t size, MemoryRegion *mr, Error **errp); > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c > index fc4ef46d11..d078775b4b 100644 > --- a/backends/hostmem-file.c > +++ b/backends/hostmem-file.c > @@ -57,7 +57,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp) > path = object_get_canonical_path(OBJECT(backend)); > memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), > path, > - backend->size, fb->share, > + backend->size, fb->share, true, > fb->mem_path, errp); > g_free(path); > } > diff --git a/exec.c b/exec.c > index a6e9ed4ece..77734198d0 100644 > --- a/exec.c > +++ b/exec.c > @@ -1937,8 +1937,8 @@ static void ram_block_add(RAMBlock *new_block, Error **errp) > > #ifdef __linux__ > RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, > - bool share, const char *mem_path, > - Error **errp) > + bool share, bool persistent, > + const char *mem_path, Error **errp) > { > RAMBlock *new_block; > Error *local_err = NULL; > @@ -1965,6 +1965,9 @@ RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr, > new_block->used_length = size; > new_block->max_length = size; > new_block->flags = share ? RAM_SHARED : 0; > + if (!persistent) { > + new_block->flags |= RAM_NONPERSISTENT; > + } > new_block->host = file_ram_alloc(new_block, size, > mem_path, errp); > if (!new_block->host) { > diff --git a/memory.c b/memory.c > index 0ddc4cc28d..3c0c0ff141 100644 > --- a/memory.c > +++ b/memory.c > @@ -1387,6 +1387,7 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, > const char *name, > uint64_t size, > bool share, > + bool persistent, > const char *path, > Error **errp) > { > @@ -1394,7 +1395,8 @@ void memory_region_init_ram_from_file(MemoryRegion *mr, > mr->ram = true; > mr->terminates = true; > mr->destructor = memory_region_destructor_ram; > - mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, path, errp); > + mr->ram_block = qemu_ram_alloc_from_file(size, mr, share, persistent, > + path, errp); > mr->dirty_log_mask = tcg_enabled() ? (1 << DIRTY_MEMORY_CODE) : 0; > } > #endif > diff --git a/numa.c b/numa.c > index 65701cb6c8..825d5933ca 100644 > --- a/numa.c > +++ b/numa.c > @@ -531,7 +531,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner, > if (mem_path) { > #ifdef __linux__ > Error *err = NULL; > - memory_region_init_ram_from_file(mr, owner, name, ram_size, false, > + memory_region_init_ram_from_file(mr, owner, name, ram_size, false, true, > mem_path, &err); > if (err) { > error_report_err(err); > -- > 2.11.0.259.g40922b1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK