linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: David Rientjes <rientjes@google.com>
To: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	mtosatti@redhat.com, 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: Sat, 15 Feb 2014 02:06:38 -0800 (PST)	[thread overview]
Message-ID: <alpine.DEB.2.02.1402150159540.28883@chino.kir.corp.google.com> (raw)
In-Reply-To: <20140214225810.57e854cb@redhat.com>

On Fri, 14 Feb 2014, Luiz Capitulino wrote:

> > Again, I think this syntax is horrendous and doesn't couple well with the 
> > other hugepage-related kernel command line options.  We already have 
> > hugepages= and hugepagesz= which you can interleave on the command line to 
> > get 100 2M hugepages and 10 1GB hugepages, for example.
> > 
> > This patchset is simply introducing another variable to the matter: the 
> > node that the hugepages should be allocated on.  So just introduce a 
> > hugepagesnode= parameter to couple with the others so you can do
> > 
> > 	hugepagesz=<size> hugepagesnode=<nid> hugepages=<#>
> 
> That was my first try but it turned out really bad. First, for every node
> you specify you need three options.

Just like you need two options today to specify a number of hugepages of a 
particular non-default size.  You only need to use hugepagesz= or 
hugepagenode= if you want a non-default size or a specify a particular 
node.

> So, if you want to setup memory for
> three nodes you'll need to specify nine options.

And you currently need six if you want to specify three different hugepage 
sizes (?).  But who really specifies three different hugepage sizes on the 
command line that are needed to be reserved at boot?

If that's really the usecase, it seems like you want the old 
CONFIG_PAGE_SHIFT patch.

> And it gets worse, because
> hugepagesz= and hugepages= have strict ordering (which is a mistake, IMHO) so
> you have to specify them in the right order otherwise things don't work as
> expected and you have no idea why (have been there myself).
> 

How is that difficult?  hugepages= is the "noun", hugepagesz= is the 
"adjective".  hugepages=100 hugepagesz=1G hugepages=4 makes perfect sense 
to me, and I actually don't allocate hugepages on the command line, nor 
have I looked at Documentation/kernel-parameters.txt to check if I'm 
constructing it correctly.  It just makes sense and once you learn it it's 
just natural.

> IMO, hugepages_node=<nid>:<nr_pages>:<size>,... is good enough. It's concise,
> and don't depend on any other option to function. Also, there are lots of other
> kernel command-line options that require you to specify multiple fields, so
> it's not like hugepages_node= is totally different in that regard.
> 

I doubt Andrew is going to want a completely different format for hugepage 
allocations that want to specify a node and have to deal with people who 
say hugepages_node=2:1:1G and constantly have to lookup if it's 2 
hugepages on node 1 or 1 hugepage on node 2.

--
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>

  reply	other threads:[~2014-02-15 10:06 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-14  1:02 [PATCH v2 0/4] " 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 [this message]
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
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=alpine.DEB.2.02.1402150159540.28883@chino.kir.corp.google.com \
    --to=rientjes@google.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=mtosatti@redhat.com \
    --cc=riel@redhat.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