From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: patch: qemu + hugetlbfs.. Date: Thu, 10 Jul 2008 15:47:20 -0500 Message-ID: <48767558.50301@codemonkey.ws> References: <4873E400.4000409@third-harmonic.com> <4873F395.6030209@codemonkey.ws> <4874051A.8000802@third-harmonic.com> <48740F86.3050306@codemonkey.ws> <20080709170301.GA11439@dmt.cnet> <4874F156.2010708@codemonkey.ws> <48763B86.6060402@third-harmonic.com> <48764DAF.6060502@codemonkey.ws> <48766E03.4090901@third-harmonic.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm@vger.kernel.org, john.cooper@redhat.com To: john cooper Return-path: Received: from py-out-1112.google.com ([64.233.166.182]:54323 "EHLO py-out-1112.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753382AbYGJUrv (ORCPT ); Thu, 10 Jul 2008 16:47:51 -0400 Received: by py-out-1112.google.com with SMTP id p76so1959521pyb.10 for ; Thu, 10 Jul 2008 13:47:50 -0700 (PDT) In-Reply-To: <48766E03.4090901@third-harmonic.com> Sender: kvm-owner@vger.kernel.org List-ID: john cooper wrote: > Anthony Liguori wrote: > >>> +#include >>> >> >> I don't think this is necessary anymore. Depending on a Linux >> headers breaks the QEMU build on other unices so it's a bad thing. > > It is no longer required, but see below. > >> hpage is a misnomer too as we aren't actually dependent on huge pages >> (this code should work equally well for tmpfs). > > As it currently exists alloc_hpage_mem() is tied to > the notion of huge page allocation as it will reference > gethugepagesize() irrespective of *mem_path. So even > in the case of tmpfs backed files, if the host kernel > has been configured with CONFIG_HUGETLBFS we will wind > up doing allocations of /dev/shm mapped files at > /proc/meminfo:Hugepagesize granularity. Which is fine. It just means we round -m values up to even numbers. > Otherwise if > HUGETLBFS is not configured gethugepagesize() returns > zero and alloc_hpage_mem() itself will not perform the > allocation. That sounds like a bug. > > Probably not what was intended but probably not too > much of a concern as "-mem-path /dev/shm" is likely > only used in debug of this flag and associated logic. > I don't see it currently being worth the trouble to > correct from a squeaky clean POV, and doing so may > drag in far more than the header file we've just > booted above to deal with this architecture/config > dependency. Renaming a function to a name that's less accurate seems bad to me. I don't mean to be pedantic, but it seems like a strange thing to do. I prefer it the way it was before. Regards, Anthony Liguori > An updated patch is attached. > > -john >