From: Eric Paris <eparis@redhat.com>
To: linux-kernel@vger.kernel.org, akpm@osdl.org, wli@holomorphy.com
Subject: [PATCH] Fix race in set_max_huge_pages for multiple updaters of nr_huge_pages
Date: Fri, 18 Nov 2005 12:51:40 -0500 [thread overview]
Message-ID: <1132336300.5151.47.camel@localhost.localdomain> (raw)
If there are multiple updaters to /proc/sys/vm/nr_hugepages
simultaneously it is possible for the nr_huge_pages variable to become
incorrect. There is no locking in the set_max_huge_pages function
around alloc_fresh_huge_page which is able to update nr_huge_pages. Two
callers to alloc_fresh_huge_page could race against each other as could
a call to alloc_fresh_huge_page and a call to update_and_free_page.
This patch just expands the area covered by the hugetlb_lock to cover
the call into alloc_fresh_huge_page. I'm not sure how we could say that
a sysctl section is performance critical where more specific locking
would be needed.
My reproducer was to run a couple copies of the following script
simultaneously
while [ true ]; do
echo 1000 > /proc/sys/vm/nr_hugepages
echo 500 > /proc/sys/vm/nr_hugepages
echo 750 > /proc/sys/vm/nr_hugepages
echo 100 > /proc/sys/vm/nr_hugepages
echo 0 > /proc/sys/vm/nr_hugepages
done
and then watch /proc/meminfo and eventually you will see things like
HugePages_Total: 100
HugePages_Free: 109
After applying the patch all seemed well.
Signed-off-by: Eric Paris <eparis@redhat.com>
----
hugetlb.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
--- linux-2.6.14.2/mm/hugetlb.c.old
+++ linux-2.6.14.2/mm/hugetlb.c
@@ -22,6 +22,7 @@
static struct list_head hugepage_freelists[MAX_NUMNODES];
static unsigned int nr_huge_pages_node[MAX_NUMNODES];
static unsigned int free_huge_pages_node[MAX_NUMNODES];
+/* This lock protects updates to hugepage_freelists, nr_huge_pages, and free_huge_pages */
static DEFINE_SPINLOCK(hugetlb_lock);
static void enqueue_huge_page(struct page *page)
@@ -172,10 +173,13 @@
static unsigned long set_max_huge_pages(unsigned long count)
{
while (count > nr_huge_pages) {
- struct page *page = alloc_fresh_huge_page();
- if (!page)
- return nr_huge_pages;
+ struct page *page;
spin_lock(&hugetlb_lock);
+ page = alloc_fresh_huge_page();
+ if (!page) {
+ spin_unlock(&hugetlb_lock);
+ return nr_huge_pages;
+ }
enqueue_huge_page(page);
spin_unlock(&hugetlb_lock);
}
next reply other threads:[~2005-11-18 17:51 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-11-18 17:51 Eric Paris [this message]
2005-11-19 3:09 ` [PATCH] Fix race in set_max_huge_pages for multiple updaters of nr_huge_pages Andrew Morton
2005-11-19 3:24 ` William Lee Irwin III
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=1132336300.5151.47.camel@localhost.localdomain \
--to=eparis@redhat.com \
--cc=akpm@osdl.org \
--cc=linux-kernel@vger.kernel.org \
--cc=wli@holomorphy.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.