If you configure THP in addition to HUGETLB_PAGE on x86_32 without PAE,
the p?d-folding works out that munlock_vma_pages_range() can crash to
follow_page()'s pud_huge() BUG_ON(flags & FOLL_GET): it needs the same
VM_HUGETLB check already there on the pmd_huge() line. Conveniently,
openSUSE provides a "blogd" which tests this out at startup!
Signed-off-by: Hugh Dickins <hughd@google.com>
---
This massive rework belongs just after thp-transparent-hugepage-core.patch
mm/memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Andrea Arcangeli <hidden> Date: 2011-01-11 01:57:46
Hi Hugh,
On Mon, Jan 10, 2011 at 04:55:53PM -0800, Hugh Dickins wrote:
quoted hunk
If you configure THP in addition to HUGETLB_PAGE on x86_32 without PAE,
the p?d-folding works out that munlock_vma_pages_range() can crash to
follow_page()'s pud_huge() BUG_ON(flags & FOLL_GET): it needs the same
VM_HUGETLB check already there on the pmd_huge() line. Conveniently,
openSUSE provides a "blogd" which tests this out at startup!
Signed-off-by: Hugh Dickins <hughd@google.com>
---
This massive rework belongs just after thp-transparent-hugepage-core.patch
mm/memory.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
How is THP related to this? pud_trans_huge doesn't exist, if pud_huge
is true, vma is already guaranteed to belong to hugetlbfs without
requiring the additional check.
I added the check to pmd_huge already, there it is needed, but for
pud_huge it isn't as far as I can tell.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Mon, Jan 10, 2011 at 5:57 PM, Andrea Arcangeli [off-list ref] wrote:
On Mon, Jan 10, 2011 at 04:55:53PM -0800, Hugh Dickins wrote:
quoted
If you configure THP in addition to HUGETLB_PAGE on x86_32 without PAE,
the p?d-folding works out that munlock_vma_pages_range() can crash to
follow_page()'s pud_huge() BUG_ON(flags & FOLL_GET): it needs the same
VM_HUGETLB check already there on the pmd_huge() line. Conveniently,
openSUSE provides a "blogd" which tests this out at startup!
How is THP related to this? pud_trans_huge doesn't exist, if pud_huge
is true, vma is already guaranteed to belong to hugetlbfs without
requiring the additional check.
THP puts in pmds that are huge. In this configuration the "folding" is
such that the puds are the pmds. So the pud_huge test passes and
the BUG_ON hits. I hope I've explained that correctly, agreed that
it's confusing!
I added the check to pmd_huge already, there it is needed, but for
pud_huge it isn't as far as I can tell.
Crashing on that BUG_ON suggests otherwise ;)
Hugh
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Andrea Arcangeli <hidden> Date: 2011-01-11 14:04:26
On Mon, Jan 10, 2011 at 06:29:29PM -0800, Hugh Dickins wrote:
On Mon, Jan 10, 2011 at 5:57 PM, Andrea Arcangeli [off-list ref] wrote:
quoted
On Mon, Jan 10, 2011 at 04:55:53PM -0800, Hugh Dickins wrote:
quoted
If you configure THP in addition to HUGETLB_PAGE on x86_32 without PAE,
the p?d-folding works out that munlock_vma_pages_range() can crash to
follow_page()'s pud_huge() BUG_ON(flags & FOLL_GET): it needs the same
VM_HUGETLB check already there on the pmd_huge() line. Conveniently,
openSUSE provides a "blogd" which tests this out at startup!
How is THP related to this? pud_trans_huge doesn't exist, if pud_huge
is true, vma is already guaranteed to belong to hugetlbfs without
requiring the additional check.
THP puts in pmds that are huge. In this configuration the "folding" is
such that the puds are the pmds. So the pud_huge test passes and
the BUG_ON hits. I hope I've explained that correctly, agreed that
it's confusing!
quoted
I added the check to pmd_huge already, there it is needed, but for
pud_huge it isn't as far as I can tell.
Crashing on that BUG_ON suggests otherwise ;)
I think I see what you mean, pgd=pud=pmd with 2 levels only, but if
pud_huge can return 1 on x86_32 without PAE, that sounds like an
architectural bug to me. Why can't pud_huge simply return 0 for
x86_32? Any other place dealing with hugepages and calling pud_huge on
x86 noPAE would be at risk, otherwise, no?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Andrea Arcangeli <hidden> Date: 2011-01-11 16:31:28
On Tue, Jan 11, 2011 at 03:04:21PM +0100, Andrea Arcangeli wrote:
architectural bug to me. Why can't pud_huge simply return 0 for
x86_32? Any other place dealing with hugepages and calling pud_huge on
x86 noPAE would be at risk, otherwise, no?
Isn't this better solution?
======
Subject: avoid confusing hugetlbfs code when pmd_trans_huge is set
From: Andrea Arcangeli <redacted>
If pmd is set huge by THP, pud_huge shouldn't return 1 when pud doesn't exist
and it's just a 1:1 bypass over the pmd (like it happens on 32bit x86 because
there are at most 2 or 3 level of pagetables). Only pmd_huge can return 1.
Signed-off-by: Andrea Arcangeli <redacted>
---
@@ -227,7 +227,15 @@ int pmd_huge(pmd_t pmd)intpud_huge(pud_tpud){+#ifdef CONFIG_X86_64return!!(pud_val(pud)&_PAGE_PSE);+#else+/*+*pudisabypasswith2or3levelpagetables,onlypmd_huge+*canreturn1.+*/+return0;+#endif}structpage*--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
On Tue, Jan 11, 2011 at 03:04:21PM +0100, Andrea Arcangeli wrote:
quoted
architectural bug to me. Why can't pud_huge simply return 0 for
x86_32? Any other place dealing with hugepages and calling pud_huge on
x86 noPAE would be at risk, otherwise, no?
Isn't this better solution?
[Better solution than my patch to follow_page() in mmotm, to fix crash
with Transparent Huge Pages by duplicating Andrea's pmd_huge VM_HUGETLB
check to the pud_huge line too.]
The truth is, I'm sure one of the solutions is better than the other,
but I'm too confused by p?d folding to know which is which ;)
Certainly I don't oppose your patch as a replacement for mine,
if you're sure yours is better.
There are only two places which are using pud_huge() anyway:
follow_page() and apply_to_pmd_range(). Is the latter's
BUG_ON(pud_huge) safe? Safe in the THP world?
And I never quite understood why we have both pmd_huge and pmd_large,
pud_huge and pud_large.
There are answers to these questions, but it would take me hours and
hours of easily-confused research (across several arches) to decide.
I'm hoping someone else has a surer grasp: Andi introduced pud_huge(),
and Jeremy is the most active in the pagetable layers nowadays -
perhaps they can tell us more quickly.
Hugh
quoted hunk
======
Subject: avoid confusing hugetlbfs code when pmd_trans_huge is set
From: Andrea Arcangeli <redacted>
If pmd is set huge by THP, pud_huge shouldn't return 1 when pud doesn't exist
and it's just a 1:1 bypass over the pmd (like it happens on 32bit x86 because
there are at most 2 or 3 level of pagetables). Only pmd_huge can return 1.
Signed-off-by: Andrea Arcangeli <redacted>
---
@@ -227,7 +227,15 @@ int pmd_huge(pmd_t pmd)intpud_huge(pud_tpud){+#ifdef CONFIG_X86_64return!!(pud_val(pud)&_PAGE_PSE);+#else+/*+*pudisabypasswith2or3levelpagetables,onlypmd_huge+*canreturn1.+*/+return0;+#endif}structpage*
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
From: Andrea Arcangeli <hidden> Date: 2011-01-12 02:03:55
On Tue, Jan 11, 2011 at 02:59:43PM -0800, Hugh Dickins wrote:
On Tue, 11 Jan 2011, Andrea Arcangeli wrote:
quoted
On Tue, Jan 11, 2011 at 03:04:21PM +0100, Andrea Arcangeli wrote:
quoted
architectural bug to me. Why can't pud_huge simply return 0 for
x86_32? Any other place dealing with hugepages and calling pud_huge on
x86 noPAE would be at risk, otherwise, no?
Isn't this better solution?
[Better solution than my patch to follow_page() in mmotm, to fix crash
with Transparent Huge Pages by duplicating Andrea's pmd_huge VM_HUGETLB
check to the pud_huge line too.]
The truth is, I'm sure one of the solutions is better than the other,
but I'm too confused by p?d folding to know which is which ;)
Certainly I don't oppose your patch as a replacement for mine,
if you're sure yours is better.
There are only two places which are using pud_huge() anyway:
follow_page() and apply_to_pmd_range(). Is the latter's
BUG_ON(pud_huge) safe? Safe in the THP world?
The latter BUG_ON should be safe in THP world, there's a pmd_huge bug
on too so it can't be a problem in THP world.
And I never quite understood why we have both pmd_huge and pmd_large,
pud_huge and pud_large.
There are answers to these questions, but it would take me hours and
hours of easily-confused research (across several arches) to decide.
I'm hoping someone else has a surer grasp: Andi introduced pud_huge(),
and Jeremy is the most active in the pagetable layers nowadays -
perhaps they can tell us more quickly.
I'd like their opinion too but for exactly the same reason why you
asked yourself if the latter BUG_ON is safe, I think my patch from
practical prospective reduces the risk.
When THP uses pmd_mkhuge it's counter intuitive that pud_huge returns
1, and there's no benefit to that at all other than risking troubles
like this one. In fact a branch and a block of follow_page is
eliminated at compile time by my patch (as opposed your patch adds one
more branch and can't eliminate a block of code if the second branch
would be taken but we know it can't).
I consider this an arch bug, not common code issue. This is the THP
modifications to the code and I didn't expect having to alter the
pud_huge check in addition to the below one. I thought this shall be
enough if the arch is correct (and optimal).
I think the x86 3level should work ok with follow_page_pmd (it's
basically identical to follow_page_pud so it won't notice the
difference) so I hope it doesn't break anything, and it will speedup
follow_page too (even when THP is off).
Across the whole tree if you grep for pmd_offset, you'll find all the
places that you've to care for THP, I'd like to still not having to
care about the result of pud_offset (having to care for pmd_offset is
more than enough ;).
Other archs implementing pud_huge should also return 0 if there are
only 3 levels, if they introduce THP, this will have the benefit of
optimizing follow_page when THP is off as well for them.
Your patch is ok if this will not be considered an arch bug (I think
to avoid mistakes pud_huge should be implemented by pgtable-nopud.h
but that's a little bigger cleanup I didn't do myself yet).
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>