From: David Rientjes <rientjes@google.com>
To: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org, linux-numa@vger.kernel.org, mel@csn.ul.ie,
randy.dunlap@oracle.com, nacc@us.ibm.com, agl@us.ibm.com,
apw@canonical.com, eric.whitney@hp.com
Subject: Re: [PATCH 3/6] hugetlb: introduce alloc_nodemask_of_node
Date: Fri, 11 Sep 2009 15:38:26 -0700 (PDT) [thread overview]
Message-ID: <alpine.DEB.1.00.0909111529590.22083@chino.kir.corp.google.com> (raw)
In-Reply-To: <1252674684.4392.222.camel@useless.americas.hpqcorp.net>
On Fri, 11 Sep 2009, Lee Schermerhorn wrote:
> > It's a bit rude to assume that the caller wanted to use GFP_KERNEL.
>
> I can add a gfp_t parameter to the macro, but I'll still need to select
> value in the caller. Do you have a suggested alternative to GFP_KERNEL
> [for both here and in alloc_nodemask_of_mempolicy()]? We certainly
> don't want to loop forever, killing off tasks, as David mentioned.
> Silently failing is OK. We handle that.
>
Dynamically allocating the nodemask_t for small NODES_SHIFT and failing to
find adequate memory isn't as troublesome as I may have made it sound;
it's only a problem if we're low on memory and can't do order-0 GFP_KERNEL
allocations and the kmalloc cache for that size is full. That's going to
be extremely rare, but the first requirement, being low on memory, is one
of the reasons why people traditionally free hugepages via the tunable.
As far as the software engineering of alloc_nodemask_of_node() goes, I'd
defer back to my previous suggestion of modifying NODEMASK_ALLOC() which
has very much the same purpose. It's also only used with mempolicies
because we're frequently dealing with the same issue; this is not unique
only to hugetlb, which is probably why it was made generic in the first
place.
It has the added benefit of also incorporating my other suggestion, which
was to allocate these on the stack when NODES_SHIFT is small, which it
defaults to for all architectures other than ia64. I think it would be
nice to avoid the slab allocator for relatively small (<= 256 bytes?)
amounts of memory that could otherwise be stack allocated. That's more of
a general statement with regard to the entire kernel, but I don't think
you'll find much benefit in always allocating them from slab for code
clarity when NODEMASK_ALLOC() exists for the same purpose such as
set_mempolicy(), mbind(), etc.
So I'd ask that you reconsider using NODEMASK_ALLOC() by making it more
general (i.e. not just allocating "structs of <name>" but rather pass in
the entire type such as "nodemask_t" or "struct nodemask_scratch") and
then using it to dynamically allocate your hugetlb nodemasks when
necessary because of their size.
next prev parent reply other threads:[~2009-09-11 22:38 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-09-09 16:31 [PATCH 0/6] hugetlb: V6 constrain allocation/free based on task mempolicy Lee Schermerhorn
2009-09-09 16:31 ` [PATCH 1/6] hugetlb: rework hstate_next_node_* functions Lee Schermerhorn
2009-09-09 16:31 ` [PATCH 2/6] hugetlb: add nodemask arg to huge page alloc, free and surplus adjust fcns Lee Schermerhorn
2009-09-09 16:31 ` [PATCH 3/6] hugetlb: introduce alloc_nodemask_of_node Lee Schermerhorn
2009-09-10 23:05 ` Andrew Morton
2009-09-10 23:17 ` David Rientjes
2009-09-10 23:36 ` Andrew Morton
2009-09-10 23:43 ` David Rientjes
2009-09-11 13:11 ` Lee Schermerhorn
2009-09-11 22:38 ` David Rientjes [this message]
2009-09-09 16:31 ` [PATCH 4/6] hugetlb: derive huge pages nodes allowed from task mempolicy Lee Schermerhorn
2009-09-10 23:15 ` Andrew Morton
2009-09-11 13:12 ` Lee Schermerhorn
2009-09-09 16:31 ` [PATCH 5/6] hugetlb: add per node hstate attributes Lee Schermerhorn
2009-09-10 12:32 ` Mel Gorman
2009-09-10 14:26 ` Lee Schermerhorn
2009-09-10 19:50 ` David Rientjes
2009-09-10 19:58 ` Lee Schermerhorn
2009-09-10 23:31 ` Andrew Morton
2009-09-11 13:12 ` Lee Schermerhorn
2009-09-09 16:32 ` [PATCH 6/6] hugetlb: update hugetlb documentation for mempolicy based management Lee Schermerhorn
2009-09-09 16:32 ` [PATCH 1/3] hugetlb: use only nodes with memory for huge pages Lee Schermerhorn
2009-09-10 23:33 ` Andrew Morton
2009-09-11 13:54 ` Lee Schermerhorn
2009-09-09 16:32 ` [PATCH 2/3] hugetlb: handle memory hot-plug events Lee Schermerhorn
2009-09-09 16:32 ` [PATCH 3/3] hugetlb: offload per node attribute registrations Lee Schermerhorn
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.1.00.0909111529590.22083@chino.kir.corp.google.com \
--to=rientjes@google.com \
--cc=Lee.Schermerhorn@hp.com \
--cc=agl@us.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=apw@canonical.com \
--cc=eric.whitney@hp.com \
--cc=linux-mm@kvack.org \
--cc=linux-numa@vger.kernel.org \
--cc=mel@csn.ul.ie \
--cc=nacc@us.ibm.com \
--cc=randy.dunlap@oracle.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 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 read-only IMAP folder(s) and NNTP newsgroup(s).