From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37523) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ztz33-0006b1-7m for qemu-devel@nongnu.org; Wed, 04 Nov 2015 09:28:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ztz30-0005KM-EP for qemu-devel@nongnu.org; Wed, 04 Nov 2015 09:28:25 -0500 Received: from mga09.intel.com ([134.134.136.24]:43230) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ztz30-0005Jl-8V for qemu-devel@nongnu.org; Wed, 04 Nov 2015 09:28:22 -0500 References: <1446455617-129562-1-git-send-email-guangrong.xiao@linux.intel.com> <1446455617-129562-9-git-send-email-guangrong.xiao@linux.intel.com> <20151103230026.GG4180@thinpad.lan.raisama.net> <563977A9.3000103@linux.intel.com> <20151104124007.GI4180@thinpad.lan.raisama.net> From: Xiao Guangrong Message-ID: <563A1491.2090408@linux.intel.com> Date: Wed, 4 Nov 2015 22:22:09 +0800 MIME-Version: 1.0 In-Reply-To: <20151104124007.GI4180@thinpad.lan.raisama.net> 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: Eduardo Habkost Cc: vsementsov@virtuozzo.com, kvm@vger.kernel.org, mst@redhat.com, gleb@kernel.org, mtosatti@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, imammedo@redhat.com, pbonzini@redhat.com, dan.j.williams@intel.com, rth@twiddle.net On 11/04/2015 08:40 PM, Eduardo Habkost wrote: > On Wed, Nov 04, 2015 at 11:12:41AM +0800, Xiao Guangrong wrote: >> On 11/04/2015 07:00 AM, Eduardo Habkost wrote: >>> On Mon, Nov 02, 2015 at 05:13:10PM +0800, 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 >>>> >>>> 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"); >>> >>> If the point of this patch is to allow file_ram_alloc() to not be >>> specific to hugetlbfs anymore, this warning can simply go away. >>> >>> (And in case if you really want to keep the warning, I don't see the >>> point of the changes you made to it.) >>> >> >> This is the history why we did it like this: >> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02862.html > > The rule I am trying to follow is simple: if there are valid use cases > (e.g. nvdimm, tmpfs) where the warning will be printed every single time > QEMU runs, the warning is not appropriate. > > If you really want to keep a warning, please make it not be printed on > all other valid use cases (nvdimm and tmpfs). Personally, I don't think > adding those additional checks would be worth the trouble, that's why I > suggest removing the warning. > >> >> Q: >> | What this *actually* is trying to warn against is that >> | mapping a regular file (as opposed to hugetlbfs) >> | means transparent huge pages don't work. > > I don't think the author of that warning even thought about transparent > huge pages (did THP even existed when it was written?). I believe they > just assumed that the only reason for using -mem-path would be hugetlbfs > and wanted to warn about it. That assumption isn't true anymore. Michael, your idea? If Michael will not beat me, i will drop this. :)