From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35521) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WIXWV-0002yO-QG for qemu-devel@nongnu.org; Wed, 26 Feb 2014 00:59:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WIXWR-0000cb-7N for qemu-devel@nongnu.org; Wed, 26 Feb 2014 00:59:15 -0500 Received: from [222.73.24.84] (port=19647 helo=song.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WIXWQ-0000Hr-P3 for qemu-devel@nongnu.org; Wed, 26 Feb 2014 00:59:11 -0500 Date: Wed, 26 Feb 2014 13:57:06 +0800 From: Hu Tao Message-ID: <20140226055706.GA5090@G08FNSTD100614.fnst.cn.fujitsu.com> References: <53047351.80300@redhat.com> <20140219103657.4daed264@nial.usersys.redhat.com> MIME-Version: 1.0 In-Reply-To: <20140219103657.4daed264@nial.usersys.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Subject: Re: [Qemu-devel] [PATCH v18 13/14] memory backend: fill memory backend ram fields List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Paolo Bonzini , lersek@redhat.com, qemu-devel@nongnu.org, Wanlong Gao On Wed, Feb 19, 2014 at 10:36:57AM +0100, Igor Mammedov wrote: > On Wed, 19 Feb 2014 10:03:13 +0100 > Paolo Bonzini wrote: > > > 19/02/2014 08:54, Hu Tao ha scritto: > > > Thus makes user control how to allocate memory for ram backend. > > > > > > Signed-off-by: Hu Tao > > > --- > > > backends/hostmem-ram.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++++ > > > include/sysemu/sysemu.h | 2 + > > > 2 files changed, 160 insertions(+) > > > > > > diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c > [...] > > > > static int > > > ram_backend_memory_init(HostMemoryBackend *backend, Error **errp) > > > { > > > + HostMemoryBackendRam *ram_backend = MEMORY_BACKEND_RAM(backend); > > > + int mode = ram_backend->policy; > > > + void *p; > > > + unsigned long maxnode; > > > + > > > if (!memory_region_size(&backend->mr)) { > > > memory_region_init_ram(&backend->mr, OBJECT(backend), > > > object_get_canonical_path(OBJECT(backend)), > > > backend->size); > > > + > > > + p = memory_region_get_ram_ptr(&backend->mr); > > > + maxnode = find_last_bit(ram_backend->host_nodes, MAX_NODES); > > > + > > > + mode |= ram_backend->relative ? MPOL_F_RELATIVE_NODES : > > > + MPOL_F_STATIC_NODES; > > > + /* This is a workaround for a long standing bug in Linux' > > > + * mbind implementation, which cuts off the last specified > > > + * node. To stay compatible should this bug be fixed, we > > > + * specify one more node and zero this one out. > > > + */ > > > + if (syscall(SYS_mbind, p, backend->size, mode, > > > + ram_backend->host_nodes, maxnode + 2, 0)) { > > > > This does not compile on non-Linux; also, does libnuma include the > > workaround? If so, this is a hint that we should be using libnuma > > instead... > > > > Finally, all this code should be in hostmem.c, not hostmem-ram.c, > > because the same policies can be applied to hugepage-backed memory. > > > > Currently host_memory_backend_get_memory is calling bc->memory_init. > > Probably the call should be replaced by something like > I've pushed to github updated version of memdev, where > host_memory_backend_get_memory() is just convenience wrapper to get > access to memdev's internal MemoryRegion. > > All initialization now is done in user_creatable->complete() method > which calls ram_backend_memory_init() so leaving it as is should be fine. If lines about memory polices are moved up to hostmem.c, the only thing left in ram_backend_memory_init() is calling memory_region_init_ram() to allocate memory. Then it comes a problem that when to apply memory polices? Choices: 1. apply memory polices in hostmem.c since this is the place user sets memory polices. But user_creatable_complete() seems not to support this.( but fix me) 2. cast to HostMemoryBackend in ram_backend_memory_init() (or in other memory backends) and add lines to apply memory polices. 3. provide an interface in HostMemoryBackendClass to do the thing and call it in subclasses. (this is basically the same as 2 except that we can reuse code) Opinions?