From: Mike Galbraith <umgwanakikbuti@gmail.com>
To: Mel Gorman <mgorman@suse.de>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
Thomas Gleixner <tglx@linutronix.de>,
linux-rt-users <linux-rt-users@vger.kernel.org>
Subject: RT trees fail ltp futex_wait04 - culprit=65d8fc777f6d (futex: Remove requirement for lock_page() in get_futex_key())
Date: Sat, 04 Jun 2016 16:53:47 +0200 [thread overview]
Message-ID: <1465052027.4104.19.camel@gmail.com> (raw)
Hi Mel,
Doing some RT tree consolidation this weekend to see what I should try
to upstream, I found that 4.[67]-rt trees fail ltp futex_wait04, and
tracked it down to commit in $subject.
Looking at that commit, I recalled it being in SLE, and SLERT working
just fine, so I went to see what a futex.c diff looked like. Taking
what was left after tossing THP bits (irrelevant to RT) and applying
that to my RT trees fixes the testcase failure.
Snarfed-and-munged-by: Mike Galbraith <umgwanakikbuti@gmail.com>
---
kernel/futex.c | 39 ++++++++++++++++++++++-----------------
1 file changed, 22 insertions(+), 17 deletions(-)
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -469,7 +469,7 @@ get_futex_key(u32 __user *uaddr, int fsh
{
unsigned long address = (unsigned long)uaddr;
struct mm_struct *mm = current->mm;
- struct page *page;
+ struct page *page, *page_head;
struct address_space *mapping;
int err, ro = 0;
@@ -520,6 +520,12 @@ get_futex_key(u32 __user *uaddr, int fsh
else
err = 0;
+ page_head = compound_head(page);
+ if (page != page_head) {
+ get_page(page_head);
+ put_page(page);
+ }
+
/*
* The treatment of mapping from this point on is critical. The page
* lock protects many things but in this context the page lock
@@ -531,11 +537,10 @@ get_futex_key(u32 __user *uaddr, int fsh
* From this point on, mapping will be re-verified if necessary and
* page lock will be acquired only if it is unavoidable
*/
- page = compound_head(page);
- mapping = READ_ONCE(page->mapping);
+ mapping = READ_ONCE(page_head->mapping);
/*
- * If page->mapping is NULL, then it cannot be a PageAnon
+ * If mapping is NULL, then it cannot be a PageAnon
* page; but it might be the ZERO_PAGE or in the gate area or
* in a special mapping (all cases which we are happy to fail);
* or it may have been a good file page when get_user_pages_fast
@@ -547,7 +552,7 @@ get_futex_key(u32 __user *uaddr, int fsh
*
* The case we do have to guard against is when memory pressure made
* shmem_writepage move it from filecache to swapcache beneath us:
- * an unlikely race, but we do need to retry for page->mapping.
+ * an unlikely race, but we do need to retry for page_head->mapping.
*/
if (unlikely(!mapping)) {
int shmem_swizzled;
@@ -557,10 +562,10 @@ get_futex_key(u32 __user *uaddr, int fsh
* applies. If this is really a shmem page then the page lock
* will prevent unexpected transitions.
*/
- lock_page(page);
- shmem_swizzled = PageSwapCache(page) || page->mapping;
- unlock_page(page);
- put_page(page);
+ lock_page(page_head);
+ shmem_swizzled = PageSwapCache(page_head) || page_head->mapping;
+ unlock_page(page_head);
+ put_page(page_head);
if (shmem_swizzled)
goto again;
@@ -578,7 +583,7 @@ get_futex_key(u32 __user *uaddr, int fsh
* it's a read-only handle, it's expected that futexes attach to
* the object not the particular process.
*/
- if (PageAnon(page)) {
+ if (PageAnon(page_head)) {
/*
* A RO anonymous page will never change and thus doesn't make
* sense for futex operations.
@@ -599,8 +604,8 @@ get_futex_key(u32 __user *uaddr, int fsh
/*
* The associated futex object in this case is the inode and
- * the page->mapping must be traversed. Ordinarily this should
- * be stabilised under page lock but it's not strictly
+ * the page_head->mapping must be traversed. Ordinarily this
+ * should be stabilised under page lock but it's not strictly
* necessary in this case as we just want to pin the inode, not
* update the radix tree or anything like that.
*
@@ -610,9 +615,9 @@ get_futex_key(u32 __user *uaddr, int fsh
*/
rcu_read_lock();
- if (READ_ONCE(page->mapping) != mapping) {
+ if (READ_ONCE(page_head->mapping) != mapping) {
rcu_read_unlock();
- put_page(page);
+ put_page(page_head);
goto again;
}
@@ -620,7 +625,7 @@ get_futex_key(u32 __user *uaddr, int fsh
inode = READ_ONCE(mapping->host);
if (!inode) {
rcu_read_unlock();
- put_page(page);
+ put_page(page_head);
goto again;
}
@@ -638,7 +643,7 @@ get_futex_key(u32 __user *uaddr, int fsh
*/
if (WARN_ON_ONCE(!atomic_inc_not_zero(&inode->i_count))) {
rcu_read_unlock();
- put_page(page);
+ put_page(page_head);
goto again;
}
@@ -659,7 +664,7 @@ get_futex_key(u32 __user *uaddr, int fsh
}
out:
- put_page(page);
+ put_page(page_head);
return err;
}
next reply other threads:[~2016-06-04 14:53 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-04 14:53 Mike Galbraith [this message]
2016-06-04 16:07 ` RT trees fail ltp futex_wait04 - culprit=65d8fc777f6d (futex: Remove requirement for lock_page() in get_futex_key()) Mike Galbraith
2016-06-04 16:16 ` Mike Galbraith
2016-06-09 12:04 ` Sebastian Andrzej Siewior
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=1465052027.4104.19.camel@gmail.com \
--to=umgwanakikbuti@gmail.com \
--cc=bigeasy@linutronix.de \
--cc=linux-rt-users@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=tglx@linutronix.de \
/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).