All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony Liguori <anthony@codemonkey.ws>
To: john cooper <john.cooper@third-harmonic.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>,
	kvm@vger.kernel.org, john.cooper@redhat.com
Subject: Re: patch: qemu + hugetlbfs..
Date: Thu, 10 Jul 2008 16:38:38 -0500	[thread overview]
Message-ID: <4876815E.3010109@codemonkey.ws> (raw)
In-Reply-To: <48767B20.20806@third-harmonic.com>

john cooper wrote:
> Anthony Liguori wrote:
>> john cooper wrote:
>>> 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.
>
> Well, yes it will round the allocation.  But from a
> minimally sufficient 4KB boundary to that of 4MB/2MB
> relative to a 32/64 bit x86 host which is excessive.
>
>>> 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.
>
> I don't see any harm reverting the name.  But I do
> believe it is largely cosmetic as given the above,
> the current code does require some work to make it
> independent of huge page assumptions.  Update attached.
>
> -john

Looks good to me.

Acked-by: Anthony Liguori <aliguori@us.ibm.com>


  reply	other threads:[~2008-07-10 21:39 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-08 22:02 patch: qemu + hugetlbfs john cooper
2008-07-08 23:09 ` Anthony Liguori
2008-07-09  0:23   ` john cooper
2008-07-09  1:08     ` Anthony Liguori
2008-07-09 17:03       ` Marcelo Tosatti
2008-07-09 17:11         ` Anthony Liguori
2008-07-10 16:40           ` john cooper
2008-07-10 17:58             ` Anthony Liguori
2008-07-10 20:16               ` john cooper
2008-07-10 20:47                 ` Anthony Liguori
2008-07-10 21:12                   ` john cooper
2008-07-10 21:38                     ` Anthony Liguori [this message]
2008-08-25 23:05                       ` Resend: " john cooper
2008-08-26  8:11                         ` Avi Kivity
2008-08-27  4:13                           ` john cooper
2009-01-16  2:19                         ` john cooper
2009-01-20 10:29                           ` Avi Kivity
2009-01-23 21:21                             ` john cooper
2009-02-05 15:42                               ` Avi Kivity
2009-02-05 16:12                                 ` Marcelo Tosatti
2009-02-05 16:15                                   ` Avi Kivity

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4876815E.3010109@codemonkey.ws \
    --to=anthony@codemonkey.ws \
    --cc=john.cooper@redhat.com \
    --cc=john.cooper@third-harmonic.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.