From: Marcelo Tosatti <mtosatti@redhat.com>
To: David Rientjes <rientjes@google.com>
Cc: Luiz Capitulino <lcapitulino@redhat.com>,
Andrew Morton <akpm@linux-foundation.org>,
Mel Gorman <mgorman@suse.de>,
Andrea Arcangeli <aarcange@redhat.com>,
Andi Kleen <andi@firstfloor.org>, Rik van Riel <riel@redhat.com>,
davidlohr@hp.com, isimatu.yasuaki@jp.fujitsu.com,
yinghai@kernel.org, linux-kernel@vger.kernel.org,
linux-mm@kvack.org
Subject: Re: [PATCH 4/4] hugetlb: add hugepages_node= command-line option
Date: Thu, 20 Feb 2014 18:34:07 -0300 [thread overview]
Message-ID: <20140220213407.GA11048@amt.cnet> (raw)
In-Reply-To: <alpine.DEB.2.02.1402191941330.29913@chino.kir.corp.google.com>
On Wed, Feb 19, 2014 at 07:46:41PM -0800, David Rientjes wrote:
> On Wed, 19 Feb 2014, Marcelo Tosatti wrote:
>
> > We agree that, in the future, we'd like to provide the ability to
> > dynamically allocate and free 1GB pages at runtime.
> >
> > Extending the kernel command line interface is a first step.
> >
> > Do you have a concrete objection to that first step ?
> >
>
> Yes, my concrete objection is that the command line interface is
> unnecessary if you can dynamically allocate and free 1GB pages at runtime
> unless memory will be so fragmented that it cannot be done when userspace
> is brought up. That is not your use case, thus this support is not
> needed. I think Mel also brought up this point.
>
> There's no "first step" about it, this is unnecessary for your use case if
> you can do it at runtime. I'm not sure what's so surprising about this.
Mel has clearly has no objection to the command line. You can also
allocate 2M pages at runtime, and that is no reason for "hugepages="
interface to not exist.
There is a number of parameters that are modifiable via the kernel
command line, so following your reasoning, they should all be removed,
because it can be done at runtime.
> > > You can't specify an interleave behavior with Luiz's command line
> > > interface so now we'd have two different interfaces for allocating
> > > hugepage sizes depending on whether you're specifying a node or not.
> > > It's "hugepagesz=1G hugepages=16" vs "hugepage_node=1:16:1G" (and I'd have
> > > to look at previous messages in this thread to see if that means 16 1GB
> > > pages on node 1 or 1 1GB pages on node 16.)
> >
> > What syntax do you prefer and why ?
> >
>
> I'm not sure it's interesting to talk about since this patchset is
> unnecessary if you can do it at runtime, but since "hugepagesz=" and
> "hugepages=" have existed for many kernel releases, we must maintain
> backwards compatibility.
Yes, we'd like to maintain backwards compatibility.
> Thus, it seems, the easiest addition would have
> been "hugepagesnode=" which I've mentioned several times, there's no
> reason to implement yet another command line option purely as a shorthand
> which hugepage_node=1:2:1G is and in a very cryptic way.
Can you state your suggestion clearly (or point to such messages), and
list the advantages of it versus the proposed patch ?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2014-02-20 22:08 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-14 1:02 [PATCH v2 0/4] hugetlb: add hugepages_node= command-line option Luiz Capitulino
2014-02-14 1:02 ` [PATCH 1/4] memblock: memblock_virt_alloc_internal(): add __GFP_THISNODE flag support Luiz Capitulino
2014-02-14 1:02 ` [PATCH 2/4] memblock: add memblock_virt_alloc_nid_nopanic() Luiz Capitulino
2014-02-14 1:02 ` [PATCH 3/4] hugetlb: add parse_pagesize_str() Luiz Capitulino
2014-02-14 1:02 ` [PATCH 4/4] hugetlb: add hugepages_node= command-line option Luiz Capitulino
2014-02-14 23:14 ` David Rientjes
2014-02-15 3:58 ` Luiz Capitulino
2014-02-15 10:06 ` David Rientjes
2014-02-17 13:56 ` Luiz Capitulino
2014-02-17 23:23 ` David Rientjes
2014-02-18 12:30 ` Marcelo Tosatti
2014-02-18 22:16 ` David Rientjes
2014-02-20 2:22 ` Marcelo Tosatti
2014-02-20 3:46 ` David Rientjes
2014-02-20 4:42 ` Luiz Capitulino
2014-02-20 4:51 ` David Rientjes
2014-02-20 15:06 ` Luiz Capitulino
2014-02-20 21:34 ` Marcelo Tosatti [this message]
2014-02-20 23:15 ` David Rientjes
2014-02-21 2:28 ` Marcelo Tosatti
2014-02-21 10:07 ` David Rientjes
2014-02-21 19:10 ` Marcelo Tosatti
2014-02-21 22:04 ` David Rientjes
2014-02-21 22:36 ` Andi Kleen
2014-02-21 22:44 ` David Rientjes
2014-02-21 22:55 ` Andi Kleen
2014-02-21 3:35 ` Luiz Capitulino
2014-02-20 21:38 ` Marcelo Tosatti
2014-02-20 23:17 ` David Rientjes
2014-02-18 5:47 ` Davidlohr Bueso
2014-02-21 23:54 ` Andrew Morton
2014-02-22 4:03 ` Andi Kleen
2014-02-22 4:31 ` Davidlohr Bueso
2014-02-22 4:40 ` Andrew Morton
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=20140220213407.GA11048@amt.cnet \
--to=mtosatti@redhat.com \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=andi@firstfloor.org \
--cc=davidlohr@hp.com \
--cc=isimatu.yasuaki@jp.fujitsu.com \
--cc=lcapitulino@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--cc=riel@redhat.com \
--cc=rientjes@google.com \
--cc=yinghai@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).