From: Andrew Morton <akpm@linux-foundation.org>
To: willy@infradead.org,peterx@redhat.com,lukas.bulwahn@gmail.com,kirill.shutemov@linux.intel.com,jgg@ziepe.ca,jgg@nvidia.com,jack@suse.cz,imbrenda@linux.ibm.com,hch@lst.de,david@redhat.com,alex.williamson@redhat.com,aarcange@redhat.com,jhubbard@nvidia.com,akpm@linux-foundation.org,patches@lists.linux.dev,linux-mm@kvack.org,mm-commits@vger.kernel.org,torvalds@linux-foundation.org,akpm@linux-foundation.org
Subject: [patch 4/5] mm: change lookup_node() to use get_user_pages_fast()
Date: Fri, 11 Feb 2022 16:29:04 -0800 [thread overview]
Message-ID: <20220212002904.A1E5FC340EB@smtp.kernel.org> (raw)
In-Reply-To: <20220211162756.9f8e8baef81183041ccfc16f@linux-foundation.org>
From: John Hubbard <jhubbard@nvidia.com>
Subject: mm: change lookup_node() to use get_user_pages_fast()
The purpose of calling get_user_pages_locked() from lookup_node() was to
allow for unlocking the mmap_lock when reading a page from the disk during
a page fault (hidden behind VM_FAULT_RETRY). The idea was to reduce
contention on the heavily-used mmap_lock. (Thanks to Jan Kara for clearly
pointing that out, and in fact I've used some of his wording here.)
However, it is unlikely for lookup_node() to take a page fault. With that
in mind, change over to calling get_user_pages_fast(). This simplifies
the code, runs a little faster in the expected case, and allows removing
get_user_pages_locked() entirely, in a subsequent patch.
Link: https://lkml.kernel.org/r/20220204020010.68930-5-jhubbard@nvidia.com
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Peter Xu <peterx@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
--- a/mm/mempolicy.c~mm-change-lookup_node-to-use-get_user_pages_fast
+++ a/mm/mempolicy.c
@@ -907,17 +907,14 @@ static void get_policy_nodemask(struct m
static int lookup_node(struct mm_struct *mm, unsigned long addr)
{
struct page *p = NULL;
- int err;
+ int ret;
- int locked = 1;
- err = get_user_pages_locked(addr & PAGE_MASK, 1, 0, &p, &locked);
- if (err > 0) {
- err = page_to_nid(p);
+ ret = get_user_pages_fast(addr & PAGE_MASK, 1, 0, &p);
+ if (ret > 0) {
+ ret = page_to_nid(p);
put_page(p);
}
- if (locked)
- mmap_read_unlock(mm);
- return err;
+ return ret;
}
/* Retrieve NUMA policy */
@@ -968,14 +965,14 @@ static long do_get_mempolicy(int *policy
if (flags & MPOL_F_NODE) {
if (flags & MPOL_F_ADDR) {
/*
- * Take a refcount on the mpol, lookup_node()
- * will drop the mmap_lock, so after calling
- * lookup_node() only "pol" remains valid, "vma"
- * is stale.
+ * Take a refcount on the mpol, because we are about to
+ * drop the mmap_lock, after which only "pol" remains
+ * valid, "vma" is stale.
*/
pol_refcount = pol;
vma = NULL;
mpol_get(pol);
+ mmap_read_unlock(mm);
err = lookup_node(mm, addr);
if (err < 0)
goto out;
_
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Morton <akpm@linux-foundation.org>
To: willy@infradead.org, peterx@redhat.com, lukas.bulwahn@gmail.com,
kirill.shutemov@linux.intel.com, jgg@ziepe.ca, jgg@nvidia.com,
jack@suse.cz, imbrenda@linux.ibm.com, hch@lst.de,
david@redhat.com, alex.williamson@redhat.com,
aarcange@redhat.com, jhubbard@nvidia.com,
akpm@linux-foundation.org, patches@lists.linux.dev,
linux-mm@kvack.org, mm-commits@vger.kernel.org,
torvalds@linux-foundation.org, akpm@linux-foundation.org
Subject: [patch 4/5] mm: change lookup_node() to use get_user_pages_fast()
Date: Fri, 11 Feb 2022 16:29:04 -0800 [thread overview]
Message-ID: <20220212002904.A1E5FC340EB@smtp.kernel.org> (raw)
In-Reply-To: <20220211162756.9f8e8baef81183041ccfc16f@linux-foundation.org>
From: John Hubbard <jhubbard@nvidia.com>
Subject: mm: change lookup_node() to use get_user_pages_fast()
The purpose of calling get_user_pages_locked() from lookup_node() was to
allow for unlocking the mmap_lock when reading a page from the disk during
a page fault (hidden behind VM_FAULT_RETRY). The idea was to reduce
contention on the heavily-used mmap_lock. (Thanks to Jan Kara for clearly
pointing that out, and in fact I've used some of his wording here.)
However, it is unlikely for lookup_node() to take a page fault. With that
in mind, change over to calling get_user_pages_fast(). This simplifies
the code, runs a little faster in the expected case, and allows removing
get_user_pages_locked() entirely, in a subsequent patch.
Link: https://lkml.kernel.org/r/20220204020010.68930-5-jhubbard@nvidia.com
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Peter Xu <peterx@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
--- a/mm/mempolicy.c~mm-change-lookup_node-to-use-get_user_pages_fast
+++ a/mm/mempolicy.c
@@ -907,17 +907,14 @@ static void get_policy_nodemask(struct m
static int lookup_node(struct mm_struct *mm, unsigned long addr)
{
struct page *p = NULL;
- int err;
+ int ret;
- int locked = 1;
- err = get_user_pages_locked(addr & PAGE_MASK, 1, 0, &p, &locked);
- if (err > 0) {
- err = page_to_nid(p);
+ ret = get_user_pages_fast(addr & PAGE_MASK, 1, 0, &p);
+ if (ret > 0) {
+ ret = page_to_nid(p);
put_page(p);
}
- if (locked)
- mmap_read_unlock(mm);
- return err;
+ return ret;
}
/* Retrieve NUMA policy */
@@ -968,14 +965,14 @@ static long do_get_mempolicy(int *policy
if (flags & MPOL_F_NODE) {
if (flags & MPOL_F_ADDR) {
/*
- * Take a refcount on the mpol, lookup_node()
- * will drop the mmap_lock, so after calling
- * lookup_node() only "pol" remains valid, "vma"
- * is stale.
+ * Take a refcount on the mpol, because we are about to
+ * drop the mmap_lock, after which only "pol" remains
+ * valid, "vma" is stale.
*/
pol_refcount = pol;
vma = NULL;
mpol_get(pol);
+ mmap_read_unlock(mm);
err = lookup_node(mm, addr);
if (err < 0)
goto out;
_
next prev parent reply other threads:[~2022-02-12 0:29 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-12 0:27 incoming Andrew Morton
2022-02-12 0:28 ` [patch 1/5] mm: fix invalid page pointer returned with FOLL_PIN gups Andrew Morton
2022-02-12 0:28 ` Andrew Morton
2022-02-12 0:32 ` Andrew Morton
2022-02-12 0:28 ` [patch 2/5] mm/gup: follow_pfn_pte(): -EEXIST cleanup Andrew Morton
2022-02-12 0:28 ` Andrew Morton
2022-02-12 0:29 ` [patch 3/5] mm/gup: remove unused pin_user_pages_locked() Andrew Morton
2022-02-12 0:29 ` Andrew Morton
2022-02-12 0:29 ` Andrew Morton [this message]
2022-02-12 0:29 ` [patch 4/5] mm: change lookup_node() to use get_user_pages_fast() Andrew Morton
2022-02-12 0:29 ` [patch 5/5] mm/gup: remove unused get_user_pages_locked() Andrew Morton
2022-02-12 0:29 ` Andrew Morton
2022-02-12 0:32 ` [patch 1/5] fs/binfmt_elf: fix PT_LOAD p_align values for loaders Andrew Morton
2022-02-12 0:32 ` Andrew Morton
2022-02-12 0:32 ` [patch 2/5] fs/proc: task_mmu.c: don't read mapcount for migration entry Andrew Morton
2022-02-12 0:32 ` Andrew Morton
2022-02-12 0:32 ` [patch 3/5] mm: vmscan: remove deadlock due to throttling failing to make progress Andrew Morton
2022-02-12 0:32 ` Andrew Morton
2022-02-12 0:32 ` [patch 4/5] mm: memcg: synchronize objcg lists with a dedicated spinlock Andrew Morton
2022-02-12 0:32 ` Andrew Morton
2022-02-12 0:32 ` [patch 5/5] kfence: make test case compatible with run time set sample interval Andrew Morton
2022-02-12 0:32 ` Andrew Morton
2022-02-12 2:02 ` incoming Linus Torvalds
2022-02-12 5:24 ` incoming 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=20220212002904.A1E5FC340EB@smtp.kernel.org \
--to=akpm@linux-foundation.org \
--cc=aarcange@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=david@redhat.com \
--cc=hch@lst.de \
--cc=imbrenda@linux.ibm.com \
--cc=jack@suse.cz \
--cc=jgg@nvidia.com \
--cc=jgg@ziepe.ca \
--cc=jhubbard@nvidia.com \
--cc=kirill.shutemov@linux.intel.com \
--cc=linux-mm@kvack.org \
--cc=lukas.bulwahn@gmail.com \
--cc=mm-commits@vger.kernel.org \
--cc=patches@lists.linux.dev \
--cc=peterx@redhat.com \
--cc=torvalds@linux-foundation.org \
--cc=willy@infradead.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 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.