From: Benjamin Herrenschmidt <benh@kernel•crashing.org>
To: linuxppc-dev@ozlabs•org
Subject: [PATCH 3/5] powerpc/mm: Fix bugs in huge page hashing
Date: Fri, 23 Jul 2010 10:41:42 +1000 [thread overview]
Message-ID: <1279845704-14848-3-git-send-email-benh@kernel.crashing.org> (raw)
In-Reply-To: <1279845704-14848-2-git-send-email-benh@kernel.crashing.org>
There's a couple of nasty bugs lurking in our huge page hashing code.
First, we don't check the access permission atomically with setting
the _PAGE_BUSY bit, which means that the PTE value we end up using
for the hashing might be different than the one we have checked
the access permissions for.
We've seen cases where that leads us to try to use an invalidated
PTE for hashing, causing all sort of "interesting" issues.
Then, we also failed to set _PAGE_DIRTY on a write access.
This fixes both, while also simplifying the code a bit.
Signed-off-by: Benjamin Herrenschmidt <benh@kernel•crashing.org>
---
---
arch/powerpc/mm/hugetlbpage-hash64.c | 31 +++++++++++++------------------
1 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/mm/hugetlbpage-hash64.c b/arch/powerpc/mm/hugetlbpage-hash64.c
index c9acd79..fd0a15e 100644
--- a/arch/powerpc/mm/hugetlbpage-hash64.c
+++ b/arch/powerpc/mm/hugetlbpage-hash64.c
@@ -21,21 +21,13 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
unsigned long old_pte, new_pte;
unsigned long va, rflags, pa, sz;
long slot;
- int err = 1;
BUG_ON(shift != mmu_psize_defs[mmu_psize].shift);
/* Search the Linux page table for a match with va */
va = hpt_va(ea, vsid, ssize);
- /*
- * Check the user's access rights to the page. If access should be
- * prevented then send the problem up to do_page_fault.
- */
- if (unlikely(access & ~pte_val(*ptep)))
- goto out;
- /*
- * At this point, we have a pte (old_pte) which can be used to build
+ /* At this point, we have a pte (old_pte) which can be used to build
* or update an HPTE. There are 2 cases:
*
* 1. There is a valid (present) pte with no associated HPTE (this is
@@ -49,9 +41,17 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
do {
old_pte = pte_val(*ptep);
- if (old_pte & _PAGE_BUSY)
- goto out;
+ /* If PTE busy, retry the access */
+ if (unlikely(old_pte & _PAGE_BUSY))
+ return 0;
+ /* If PTE permissions don't match, take page fault */
+ if (unlikely(access & ~pte_val(*ptep)))
+ return 1;
+ /* Try to lock the PTE, add ACCESSED and DIRTY if it was
+ * a write access */
new_pte = old_pte | _PAGE_BUSY | _PAGE_ACCESSED;
+ if (access & _PAGE_RW)
+ new_pte |= _PAGE_DIRTY;
} while(old_pte != __cmpxchg_u64((unsigned long *)ptep,
old_pte, new_pte));
@@ -127,8 +127,7 @@ repeat:
*/
if (unlikely(slot == -2)) {
*ptep = __pte(old_pte);
- err = -1;
- goto out;
+ return -1;
}
new_pte |= (slot << 12) & (_PAGE_F_SECOND | _PAGE_F_GIX);
@@ -138,9 +137,5 @@ repeat:
* No need to use ldarx/stdcx here
*/
*ptep = __pte(new_pte & ~_PAGE_BUSY);
-
- err = 0;
-
- out:
- return err;
+ return 0;
}
--
1.6.3.3
next prev parent reply other threads:[~2010-07-23 0:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-23 0:41 [PATCH 1/5] powerpc/mm: Handle hypervisor pte insert failure in __hash_page_huge Benjamin Herrenschmidt
2010-07-23 0:41 ` [PATCH 2/5] powerpc/mm: Move around testing of _PAGE_PRESENT in hash code Benjamin Herrenschmidt
2010-07-23 0:41 ` Benjamin Herrenschmidt [this message]
2010-07-23 0:41 ` [PATCH 4/5] powerpc/mm: Add some debug output when hash insertion fails Benjamin Herrenschmidt
2010-07-23 0:41 ` [PATCH 5/5] powerpc: Fix erroneous lmb->memblock conversions Benjamin Herrenschmidt
2010-07-23 3:03 ` [PATCH 3/5] powerpc/mm: Fix bugs in huge page hashing Benjamin Herrenschmidt
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=1279845704-14848-3-git-send-email-benh@kernel.crashing.org \
--to=benh@kernel$(echo .)crashing.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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