From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57843) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtH2c-0002T1-Ci for qemu-devel@nongnu.org; Mon, 02 Nov 2015 10:29:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZtH2X-0005WB-Pz for qemu-devel@nongnu.org; Mon, 02 Nov 2015 10:29:02 -0500 Received: from mga11.intel.com ([192.55.52.93]:46792) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZtH2X-0005W3-Fp for qemu-devel@nongnu.org; Mon, 02 Nov 2015 10:28:57 -0500 References: <1446455617-129562-1-git-send-email-guangrong.xiao@linux.intel.com> <1446455617-129562-9-git-send-email-guangrong.xiao@linux.intel.com> <5637787A.10504@virtuozzo.com> From: Xiao Guangrong Message-ID: <56377FA4.8080804@linux.intel.com> Date: Mon, 2 Nov 2015 23:22:12 +0800 MIME-Version: 1.0 In-Reply-To: <5637787A.10504@virtuozzo.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v7 08/35] exec: allow memory to be allocated from any kind of path List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy , pbonzini@redhat.com, imammedo@redhat.com Cc: ehabkost@redhat.com, kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, dan.j.williams@intel.com, rth@twiddle.net On 11/02/2015 10:51 PM, Vladimir Sementsov-Ogievskiy wrote: > On 02.11.2015 12:13, Xiao Guangrong wrote: >> Currently file_ram_alloc() is designed for hugetlbfs, however, the memory >> of nvdimm can come from either raw pmem device eg, /dev/pmem, or the file >> locates at DAX enabled filesystem >> >> So this patch let it work on any kind of path > > No, this patch doesn't change any logic, but only fix variable name and some error messages. Yes, it is. 'let it work' in my thought exactly was "fix variable name and some error messages"... okay, if it confused you, how about change it to: "This patch fixes variable name and some error messages to let it be aware of normal path" > >> >> Signed-off-by: Xiao Guangrong >> --- >> exec.c | 24 ++++++++++++------------ >> 1 file changed, 12 insertions(+), 12 deletions(-) >> >> diff --git a/exec.c b/exec.c >> index 9de38be..9075f4d 100644 >> --- a/exec.c >> +++ b/exec.c >> @@ -1184,25 +1184,25 @@ static void *file_ram_alloc(RAMBlock *block, >> char *c; >> void *area; >> int fd; >> - uint64_t hpagesize; >> + uint64_t pagesize; >> Error *local_err = NULL; >> - hpagesize = qemu_file_get_page_size(path, &local_err); >> + pagesize = qemu_file_get_page_size(path, &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> goto error; >> } >> - if (hpagesize == getpagesize()) { >> - fprintf(stderr, "Warning: path not on HugeTLBFS: %s\n", path); >> + if (pagesize == getpagesize()) { >> + fprintf(stderr, "Memory is not allocated from HugeTlbfs.\n"); > > Why do you remove path from error message? It is good additional information.. What if we have > several memory file backends? Good catch, will change it to: fprintf(stderr, "Memory is not allocated from HugeTlbfs on path %s.\n", path); BTW, if no other big change in the further, i will post the new version just for of this patch, > >> } >> - block->mr->align = hpagesize; >> + block->mr->align = pagesize; >> - if (memory < hpagesize) { >> + if (memory < pagesize) { >> error_setg(errp, "memory size 0x" RAM_ADDR_FMT " must be equal to " >> - "or larger than huge page size 0x%" PRIx64, >> - memory, hpagesize); >> + "or larger than page size 0x%" PRIx64, >> + memory, pagesize); >> goto error; >> } >> @@ -1226,14 +1226,14 @@ static void *file_ram_alloc(RAMBlock *block, >> fd = mkstemp(filename); >> if (fd < 0) { >> error_setg_errno(errp, errno, >> - "unable to create backing store for hugepages"); >> + "unable to create backing store for path %s", path); >> g_free(filename); >> goto error; >> } >> unlink(filename); >> g_free(filename); >> - memory = ROUND_UP(memory, hpagesize); >> + memory = ROUND_UP(memory, pagesize); >> /* >> * ftruncate is not supported by hugetlbfs in older >> @@ -1245,10 +1245,10 @@ static void *file_ram_alloc(RAMBlock *block, >> perror("ftruncate"); >> } >> - area = qemu_ram_mmap(fd, memory, hpagesize, block->flags & RAM_SHARED); >> + area = qemu_ram_mmap(fd, memory, pagesize, block->flags & RAM_SHARED); >> if (area == MAP_FAILED) { >> error_setg_errno(errp, errno, >> - "unable to map backing store for hugepages"); >> + "unable to map backing store for path %s", path); >> close(fd); >> goto error; >> } >> >