* [PATCH 0/4] hugetlb, rmap: fixes and cleanups
@ 2010-09-10 4:23 ` Naoya Horiguchi
0 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 4:23 UTC (permalink / raw)
To: LKML
Cc: Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
Hi,
These are fix and cleanup patches for hugepage rmapping.
All these were pointed out in the following thread (last 4 messages.)
http://thread.gmane.org/gmane.linux.kernel.mm/52334
Summary:
[PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
[PATCH 2/4] hugetlb, rmap: use hugepage_add_new_anon_rmap() in hugetlb_cow()
[PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
[PATCH 4/4] hugetlb, rmap: add BUG_ON(!PageLocked) in hugetlb_add_anon_rmap()
Thanks,
Naoya Horiguchi
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 4:23 ` Naoya Horiguchi
-1 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 4:23 UTC (permalink / raw)
To: LKML
Cc: Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
This patch applies Andrea's fix given by the following patch into hugepage
rmapping code:
commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
Author: Andrea Arcangeli <aarcange@redhat.com>
Date: Mon Aug 9 17:19:09 2010 -0700
This patch uses anon_vma->root and avoids unnecessary overwriting when
anon_vma is already set up.
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
mm/rmap.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git v2.6.36-rc3/mm/rmap.c v2.6.36-rc3/mm/rmap.c
index f6f0d2d..2854857 100644
--- v2.6.36-rc3/mm/rmap.c
+++ v2.6.36-rc3/mm/rmap.c
@@ -1564,13 +1564,14 @@ static void __hugepage_set_anon_rmap(struct page *page,
struct vm_area_struct *vma, unsigned long address, int exclusive)
{
struct anon_vma *anon_vma = vma->anon_vma;
+
BUG_ON(!anon_vma);
- if (!exclusive) {
- struct anon_vma_chain *avc;
- avc = list_entry(vma->anon_vma_chain.prev,
- struct anon_vma_chain, same_vma);
- anon_vma = avc->anon_vma;
- }
+
+ if (PageAnon(page))
+ return;
+ if (!exclusive)
+ anon_vma = anon_vma->root;
+
anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON;
page->mapping = (struct address_space *) anon_vma;
page->index = linear_page_index(vma, address);
--
1.7.2.2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
@ 2010-09-10 4:23 ` Naoya Horiguchi
0 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 4:23 UTC (permalink / raw)
To: LKML
Cc: Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
This patch applies Andrea's fix given by the following patch into hugepage
rmapping code:
commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
Author: Andrea Arcangeli <aarcange@redhat.com>
Date: Mon Aug 9 17:19:09 2010 -0700
This patch uses anon_vma->root and avoids unnecessary overwriting when
anon_vma is already set up.
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
mm/rmap.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git v2.6.36-rc3/mm/rmap.c v2.6.36-rc3/mm/rmap.c
index f6f0d2d..2854857 100644
--- v2.6.36-rc3/mm/rmap.c
+++ v2.6.36-rc3/mm/rmap.c
@@ -1564,13 +1564,14 @@ static void __hugepage_set_anon_rmap(struct page *page,
struct vm_area_struct *vma, unsigned long address, int exclusive)
{
struct anon_vma *anon_vma = vma->anon_vma;
+
BUG_ON(!anon_vma);
- if (!exclusive) {
- struct anon_vma_chain *avc;
- avc = list_entry(vma->anon_vma_chain.prev,
- struct anon_vma_chain, same_vma);
- anon_vma = avc->anon_vma;
- }
+
+ if (PageAnon(page))
+ return;
+ if (!exclusive)
+ anon_vma = anon_vma->root;
+
anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON;
page->mapping = (struct address_space *) anon_vma;
page->index = linear_page_index(vma, address);
--
1.7.2.2
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 14:37 ` Andrea Arcangeli
-1 siblings, 0 replies; 42+ messages in thread
From: Andrea Arcangeli @ 2010-09-10 14:37 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Hugh Dickins, Christoph Lameter, Linus Torvalds,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
On Fri, Sep 10, 2010 at 01:23:03PM +0900, Naoya Horiguchi wrote:
> This patch applies Andrea's fix given by the following patch into hugepage
> rmapping code:
>
> commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
> Author: Andrea Arcangeli <aarcange@redhat.com>
> Date: Mon Aug 9 17:19:09 2010 -0700
>
> This patch uses anon_vma->root and avoids unnecessary overwriting when
> anon_vma is already set up.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
> mm/rmap.c | 13 +++++++------
> 1 files changed, 7 insertions(+), 6 deletions(-)
Acked-by: Andrea Arcangeli <aarcange@redhat.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
@ 2010-09-10 14:37 ` Andrea Arcangeli
0 siblings, 0 replies; 42+ messages in thread
From: Andrea Arcangeli @ 2010-09-10 14:37 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Hugh Dickins, Christoph Lameter, Linus Torvalds,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
On Fri, Sep 10, 2010 at 01:23:03PM +0900, Naoya Horiguchi wrote:
> This patch applies Andrea's fix given by the following patch into hugepage
> rmapping code:
>
> commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
> Author: Andrea Arcangeli <aarcange@redhat.com>
> Date: Mon Aug 9 17:19:09 2010 -0700
>
> This patch uses anon_vma->root and avoids unnecessary overwriting when
> anon_vma is already set up.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
> mm/rmap.c | 13 +++++++------
> 1 files changed, 7 insertions(+), 6 deletions(-)
Acked-by: Andrea Arcangeli <aarcange@redhat.com>
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 14:40 ` Rik van Riel
-1 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-10 14:40 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Peter Zijlstra, Andi Kleen,
linux-mm
On 09/10/2010 12:23 AM, Naoya Horiguchi wrote:
> This patch applies Andrea's fix given by the following patch into hugepage
> rmapping code:
>
> commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
> Author: Andrea Arcangeli<aarcange@redhat.com>
> Date: Mon Aug 9 17:19:09 2010 -0700
>
> This patch uses anon_vma->root and avoids unnecessary overwriting when
> anon_vma is already set up.
>
> Signed-off-by: Naoya Horiguchi<n-horiguchi@ah.jp.nec.com>
Reviewed-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
@ 2010-09-10 14:40 ` Rik van Riel
0 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-10 14:40 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Peter Zijlstra, Andi Kleen,
linux-mm
On 09/10/2010 12:23 AM, Naoya Horiguchi wrote:
> This patch applies Andrea's fix given by the following patch into hugepage
> rmapping code:
>
> commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
> Author: Andrea Arcangeli<aarcange@redhat.com>
> Date: Mon Aug 9 17:19:09 2010 -0700
>
> This patch uses anon_vma->root and avoids unnecessary overwriting when
> anon_vma is already set up.
>
> Signed-off-by: Naoya Horiguchi<n-horiguchi@ah.jp.nec.com>
Reviewed-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 17:19 ` Linus Torvalds
-1 siblings, 0 replies; 42+ messages in thread
From: Linus Torvalds @ 2010-09-10 17:19 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
On Thu, Sep 9, 2010 at 9:23 PM, Naoya Horiguchi
<n-horiguchi@ah.jp.nec.com> wrote:
> This patch applies Andrea's fix given by the following patch into hugepage
> rmapping code:
>
> commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
> Author: Andrea Arcangeli <aarcange@redhat.com>
> Date: Mon Aug 9 17:19:09 2010 -0700
>
> This patch uses anon_vma->root and avoids unnecessary overwriting when
> anon_vma is already set up.
Btw, why isn't the code in __page_set_anon_rmap() also doing this
cleaner version (ie a single "if (PageAnon(page)) return;" up front)?
The comments in that function are also some alien language translated
to english by some broken automatic translation service. Could
somebody clean up that function and come up with a comment that
actually parses as English and makes sense?
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
@ 2010-09-10 17:19 ` Linus Torvalds
0 siblings, 0 replies; 42+ messages in thread
From: Linus Torvalds @ 2010-09-10 17:19 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
On Thu, Sep 9, 2010 at 9:23 PM, Naoya Horiguchi
<n-horiguchi@ah.jp.nec.com> wrote:
> This patch applies Andrea's fix given by the following patch into hugepage
> rmapping code:
>
> commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
> Author: Andrea Arcangeli <aarcange@redhat.com>
> Date: Mon Aug 9 17:19:09 2010 -0700
>
> This patch uses anon_vma->root and avoids unnecessary overwriting when
> anon_vma is already set up.
Btw, why isn't the code in __page_set_anon_rmap() also doing this
cleaner version (ie a single "if (PageAnon(page)) return;" up front)?
The comments in that function are also some alien language translated
to english by some broken automatic translation service. Could
somebody clean up that function and come up with a comment that
actually parses as English and makes sense?
Linus
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
2010-09-10 17:19 ` Linus Torvalds
@ 2010-09-10 21:50 ` Andi Kleen
-1 siblings, 0 replies; 42+ messages in thread
From: Andi Kleen @ 2010-09-10 21:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: Naoya Horiguchi, LKML, Andrea Arcangeli, Hugh Dickins,
Christoph Lameter, Andrew Morton, Rik van Riel, Peter Zijlstra,
linux-mm
On Fri, 10 Sep 2010 10:19:24 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Sep 9, 2010 at 9:23 PM, Naoya Horiguchi
> <n-horiguchi@ah.jp.nec.com> wrote:
> > This patch applies Andrea's fix given by the following patch into
> > hugepage rmapping code:
> >
> > commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
> > Author: Andrea Arcangeli <aarcange@redhat.com>
> > Date: Mon Aug 9 17:19:09 2010 -0700
> >
> > This patch uses anon_vma->root and avoids unnecessary overwriting
> > when anon_vma is already set up.
>
> Btw, why isn't the code in __page_set_anon_rmap() also doing this
> cleaner version (ie a single "if (PageAnon(page)) return;" up front)?
Perhaps I misunderstand the question, but __page_set_anon_rmap
should handle Anon pages, shouldn't it?
>
> The comments in that function are also some alien language translated
> to english by some broken automatic translation service. Could
> somebody clean up that function and come up with a comment that
> actually parses as English and makes sense?
I'll do that.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
@ 2010-09-10 21:50 ` Andi Kleen
0 siblings, 0 replies; 42+ messages in thread
From: Andi Kleen @ 2010-09-10 21:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: Naoya Horiguchi, LKML, Andrea Arcangeli, Hugh Dickins,
Christoph Lameter, Andrew Morton, Rik van Riel, Peter Zijlstra,
linux-mm
On Fri, 10 Sep 2010 10:19:24 -0700
Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Sep 9, 2010 at 9:23 PM, Naoya Horiguchi
> <n-horiguchi@ah.jp.nec.com> wrote:
> > This patch applies Andrea's fix given by the following patch into
> > hugepage rmapping code:
> >
> > commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
> > Author: Andrea Arcangeli <aarcange@redhat.com>
> > Date: Mon Aug 9 17:19:09 2010 -0700
> >
> > This patch uses anon_vma->root and avoids unnecessary overwriting
> > when anon_vma is already set up.
>
> Btw, why isn't the code in __page_set_anon_rmap() also doing this
> cleaner version (ie a single "if (PageAnon(page)) return;" up front)?
Perhaps I misunderstand the question, but __page_set_anon_rmap
should handle Anon pages, shouldn't it?
>
> The comments in that function are also some alien language translated
> to english by some broken automatic translation service. Could
> somebody clean up that function and come up with a comment that
> actually parses as English and makes sense?
I'll do that.
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
2010-09-10 21:50 ` Andi Kleen
@ 2010-09-10 22:07 ` Linus Torvalds
-1 siblings, 0 replies; 42+ messages in thread
From: Linus Torvalds @ 2010-09-10 22:07 UTC (permalink / raw)
To: Andi Kleen
Cc: Naoya Horiguchi, LKML, Andrea Arcangeli, Hugh Dickins,
Christoph Lameter, Andrew Morton, Rik van Riel, Peter Zijlstra,
linux-mm
On Fri, Sep 10, 2010 at 2:50 PM, Andi Kleen <andi@firstfloor.org> wrote:
>> Btw, why isn't the code in __page_set_anon_rmap() also doing this
>> cleaner version (ie a single "if (PageAnon(page)) return;" up front)?
>
> Perhaps I misunderstand the question, but __page_set_anon_rmap
> should handle Anon pages, shouldn't it?
I'm talking about this:
if (!exclusive) {
if (PageAnon(page))
return;
anon_vma = anon_vma->root;
} else {
.. big bad comment ...
if (PageAnon(page))
return;
}
where both sides of the if-statement start off doing the same thing.
It would be much cleaner to just do
... big _comprehensible_ comment ...
if (PageAnon(page))
return;
if (!exclusive)
anon_vma = anon_vma->root;
which avoids that silly else that just does something that we always do.
The reason I reacted is that Naoya-san's patch did that cleaner
version for the hugetlb case. So when I compared it to the non-hugetlb
case I just went "Ewww..."
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
@ 2010-09-10 22:07 ` Linus Torvalds
0 siblings, 0 replies; 42+ messages in thread
From: Linus Torvalds @ 2010-09-10 22:07 UTC (permalink / raw)
To: Andi Kleen
Cc: Naoya Horiguchi, LKML, Andrea Arcangeli, Hugh Dickins,
Christoph Lameter, Andrew Morton, Rik van Riel, Peter Zijlstra,
linux-mm
On Fri, Sep 10, 2010 at 2:50 PM, Andi Kleen <andi@firstfloor.org> wrote:
>> Btw, why isn't the code in __page_set_anon_rmap() also doing this
>> cleaner version (ie a single "if (PageAnon(page)) return;" up front)?
>
> Perhaps I misunderstand the question, but __page_set_anon_rmap
> should handle Anon pages, shouldn't it?
I'm talking about this:
if (!exclusive) {
if (PageAnon(page))
return;
anon_vma = anon_vma->root;
} else {
.. big bad comment ...
if (PageAnon(page))
return;
}
where both sides of the if-statement start off doing the same thing.
It would be much cleaner to just do
... big _comprehensible_ comment ...
if (PageAnon(page))
return;
if (!exclusive)
anon_vma = anon_vma->root;
which avoids that silly else that just does something that we always do.
The reason I reacted is that Naoya-san's patch did that cleaner
version for the hugetlb case. So when I compared it to the non-hugetlb
case I just went "Ewww..."
Linus
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
2010-09-10 21:50 ` Andi Kleen
@ 2010-09-11 2:09 ` Rik van Riel
-1 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-11 2:09 UTC (permalink / raw)
To: Andi Kleen
Cc: Linus Torvalds, Naoya Horiguchi, LKML, Andrea Arcangeli,
Hugh Dickins, Christoph Lameter, Andrew Morton, Peter Zijlstra,
linux-mm
On 09/10/2010 05:50 PM, Andi Kleen wrote:
> On Fri, 10 Sep 2010 10:19:24 -0700
> Linus Torvalds<torvalds@linux-foundation.org> wrote:
>
>> On Thu, Sep 9, 2010 at 9:23 PM, Naoya Horiguchi
>> <n-horiguchi@ah.jp.nec.com> wrote:
>>> This patch applies Andrea's fix given by the following patch into
>>> hugepage rmapping code:
>>>
>>> commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
>>> Author: Andrea Arcangeli<aarcange@redhat.com>
>>> Date: Mon Aug 9 17:19:09 2010 -0700
>>>
>>> This patch uses anon_vma->root and avoids unnecessary overwriting
>>> when anon_vma is already set up.
>>
>> Btw, why isn't the code in __page_set_anon_rmap() also doing this
>> cleaner version (ie a single "if (PageAnon(page)) return;" up front)?
>
> Perhaps I misunderstand the question, but __page_set_anon_rmap
> should handle Anon pages, shouldn't it?
__page_set_anon_rmap sets the page->mapping to be
a pointer to the anon_vma & PAGE_MAPPING_ANON.
PageAnon tests for page->mapping & PAGE_MAPPING_ANON,
ie. whether page->mapping is already pointing to an
anon_vma.
If it is, __page_set_anon_rmap should leave the page
mapping pointer alone.
--
All rights reversed
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 1/4] hugetlb, rmap: always use anon_vma root pointer
@ 2010-09-11 2:09 ` Rik van Riel
0 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-11 2:09 UTC (permalink / raw)
To: Andi Kleen
Cc: Linus Torvalds, Naoya Horiguchi, LKML, Andrea Arcangeli,
Hugh Dickins, Christoph Lameter, Andrew Morton, Peter Zijlstra,
linux-mm
On 09/10/2010 05:50 PM, Andi Kleen wrote:
> On Fri, 10 Sep 2010 10:19:24 -0700
> Linus Torvalds<torvalds@linux-foundation.org> wrote:
>
>> On Thu, Sep 9, 2010 at 9:23 PM, Naoya Horiguchi
>> <n-horiguchi@ah.jp.nec.com> wrote:
>>> This patch applies Andrea's fix given by the following patch into
>>> hugepage rmapping code:
>>>
>>> commit 288468c334e98aacbb7e2fb8bde6bc1adcd55e05
>>> Author: Andrea Arcangeli<aarcange@redhat.com>
>>> Date: Mon Aug 9 17:19:09 2010 -0700
>>>
>>> This patch uses anon_vma->root and avoids unnecessary overwriting
>>> when anon_vma is already set up.
>>
>> Btw, why isn't the code in __page_set_anon_rmap() also doing this
>> cleaner version (ie a single "if (PageAnon(page)) return;" up front)?
>
> Perhaps I misunderstand the question, but __page_set_anon_rmap
> should handle Anon pages, shouldn't it?
__page_set_anon_rmap sets the page->mapping to be
a pointer to the anon_vma & PAGE_MAPPING_ANON.
PageAnon tests for page->mapping & PAGE_MAPPING_ANON,
ie. whether page->mapping is already pointing to an
anon_vma.
If it is, __page_set_anon_rmap should leave the page
mapping pointer alone.
--
All rights reversed
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 2/4] hugetlb, rmap: use hugepage_add_new_anon_rmap() in hugetlb_cow()
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 4:23 ` Naoya Horiguchi
-1 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 4:23 UTC (permalink / raw)
To: LKML
Cc: Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
Obviously, setting anon_vma for COWed hugepage should be done
by hugepage_add_new_anon_rmap() to scan vmas faster.
This patch fixes it.
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
mm/hugetlb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git v2.6.36-rc3/mm/hugetlb.c v2.6.36-rc3/mm/hugetlb.c
index cc5be78..9519f3f 100644
--- v2.6.36-rc3/mm/hugetlb.c
+++ v2.6.36-rc3/mm/hugetlb.c
@@ -2404,7 +2404,7 @@ retry_avoidcopy:
set_huge_pte_at(mm, address, ptep,
make_huge_pte(vma, new_page, 1));
page_remove_rmap(old_page);
- hugepage_add_anon_rmap(new_page, vma, address);
+ hugepage_add_new_anon_rmap(new_page, vma, address);
/* Make the old page be freed below */
new_page = old_page;
mmu_notifier_invalidate_range_end(mm,
--
1.7.2.2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 2/4] hugetlb, rmap: use hugepage_add_new_anon_rmap() in hugetlb_cow()
@ 2010-09-10 4:23 ` Naoya Horiguchi
0 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 4:23 UTC (permalink / raw)
To: LKML
Cc: Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
Obviously, setting anon_vma for COWed hugepage should be done
by hugepage_add_new_anon_rmap() to scan vmas faster.
This patch fixes it.
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
mm/hugetlb.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git v2.6.36-rc3/mm/hugetlb.c v2.6.36-rc3/mm/hugetlb.c
index cc5be78..9519f3f 100644
--- v2.6.36-rc3/mm/hugetlb.c
+++ v2.6.36-rc3/mm/hugetlb.c
@@ -2404,7 +2404,7 @@ retry_avoidcopy:
set_huge_pte_at(mm, address, ptep,
make_huge_pte(vma, new_page, 1));
page_remove_rmap(old_page);
- hugepage_add_anon_rmap(new_page, vma, address);
+ hugepage_add_new_anon_rmap(new_page, vma, address);
/* Make the old page be freed below */
new_page = old_page;
mmu_notifier_invalidate_range_end(mm,
--
1.7.2.2
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 2/4] hugetlb, rmap: use hugepage_add_new_anon_rmap() in hugetlb_cow()
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 14:39 ` Andrea Arcangeli
-1 siblings, 0 replies; 42+ messages in thread
From: Andrea Arcangeli @ 2010-09-10 14:39 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Hugh Dickins, Christoph Lameter, Linus Torvalds,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
On Fri, Sep 10, 2010 at 01:23:04PM +0900, Naoya Horiguchi wrote:
> Obviously, setting anon_vma for COWed hugepage should be done
> by hugepage_add_new_anon_rmap() to scan vmas faster.
> This patch fixes it.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
> mm/hugetlb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Acked-by: Andrea Arcangeli <aarcange@redhat.com>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 2/4] hugetlb, rmap: use hugepage_add_new_anon_rmap() in hugetlb_cow()
@ 2010-09-10 14:39 ` Andrea Arcangeli
0 siblings, 0 replies; 42+ messages in thread
From: Andrea Arcangeli @ 2010-09-10 14:39 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Hugh Dickins, Christoph Lameter, Linus Torvalds,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
On Fri, Sep 10, 2010 at 01:23:04PM +0900, Naoya Horiguchi wrote:
> Obviously, setting anon_vma for COWed hugepage should be done
> by hugepage_add_new_anon_rmap() to scan vmas faster.
> This patch fixes it.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
> mm/hugetlb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Acked-by: Andrea Arcangeli <aarcange@redhat.com>
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 2/4] hugetlb, rmap: use hugepage_add_new_anon_rmap() in hugetlb_cow()
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 14:40 ` Rik van Riel
-1 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-10 14:40 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Peter Zijlstra, Andi Kleen,
linux-mm
On 09/10/2010 12:23 AM, Naoya Horiguchi wrote:
> Obviously, setting anon_vma for COWed hugepage should be done
> by hugepage_add_new_anon_rmap() to scan vmas faster.
> This patch fixes it.
>
> Signed-off-by: Naoya Horiguchi<n-horiguchi@ah.jp.nec.com>
> ---
> mm/hugetlb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Reviewed-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 2/4] hugetlb, rmap: use hugepage_add_new_anon_rmap() in hugetlb_cow()
@ 2010-09-10 14:40 ` Rik van Riel
0 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-10 14:40 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Peter Zijlstra, Andi Kleen,
linux-mm
On 09/10/2010 12:23 AM, Naoya Horiguchi wrote:
> Obviously, setting anon_vma for COWed hugepage should be done
> by hugepage_add_new_anon_rmap() to scan vmas faster.
> This patch fixes it.
>
> Signed-off-by: Naoya Horiguchi<n-horiguchi@ah.jp.nec.com>
> ---
> mm/hugetlb.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
Reviewed-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 4:23 ` Naoya Horiguchi
-1 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 4:23 UTC (permalink / raw)
To: LKML
Cc: Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
if(!trylock_page) block in avoidcopy path of hugetlb_cow() looks confusing
and is buggy. Originally this trylock_page() is intended to make sure
that old_page is locked even when old_page != pagecache_page, because then
only pagecache_page is locked.
This patch fixes it by moving page locking into hugetlb_fault().
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
mm/hugetlb.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git v2.6.36-rc3/mm/hugetlb.c v2.6.36-rc3/mm/hugetlb.c
index 9519f3f..2e17e0e 100644
--- v2.6.36-rc3/mm/hugetlb.c
+++ v2.6.36-rc3/mm/hugetlb.c
@@ -2324,11 +2324,8 @@ retry_avoidcopy:
* and just make the page writable */
avoidcopy = (page_mapcount(old_page) == 1);
if (avoidcopy) {
- if (!trylock_page(old_page)) {
- if (PageAnon(old_page))
- page_move_anon_rmap(old_page, vma, address);
- } else
- unlock_page(old_page);
+ if (PageAnon(old_page))
+ page_move_anon_rmap(old_page, vma, address);
set_huge_ptep_writable(vma, address, ptep);
return 0;
}
@@ -2631,10 +2628,14 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
vma, address);
}
- if (!pagecache_page) {
- page = pte_page(entry);
+ /*
+ * hugetlb_cow() requires page locks of pte_page(entry) and
+ * pagecache_page, so here we need take the former one
+ * when page != pagecache_page or !pagecache_page.
+ */
+ page = pte_page(entry);
+ if (page != pagecache_page)
lock_page(page);
- }
spin_lock(&mm->page_table_lock);
/* Check for a racing update before calling hugetlb_cow */
@@ -2661,9 +2662,8 @@ out_page_table_lock:
if (pagecache_page) {
unlock_page(pagecache_page);
put_page(pagecache_page);
- } else {
- unlock_page(page);
}
+ unlock_page(page);
out_mutex:
mutex_unlock(&hugetlb_instantiation_mutex);
--
1.7.2.2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
@ 2010-09-10 4:23 ` Naoya Horiguchi
0 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 4:23 UTC (permalink / raw)
To: LKML
Cc: Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
if(!trylock_page) block in avoidcopy path of hugetlb_cow() looks confusing
and is buggy. Originally this trylock_page() is intended to make sure
that old_page is locked even when old_page != pagecache_page, because then
only pagecache_page is locked.
This patch fixes it by moving page locking into hugetlb_fault().
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
mm/hugetlb.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git v2.6.36-rc3/mm/hugetlb.c v2.6.36-rc3/mm/hugetlb.c
index 9519f3f..2e17e0e 100644
--- v2.6.36-rc3/mm/hugetlb.c
+++ v2.6.36-rc3/mm/hugetlb.c
@@ -2324,11 +2324,8 @@ retry_avoidcopy:
* and just make the page writable */
avoidcopy = (page_mapcount(old_page) == 1);
if (avoidcopy) {
- if (!trylock_page(old_page)) {
- if (PageAnon(old_page))
- page_move_anon_rmap(old_page, vma, address);
- } else
- unlock_page(old_page);
+ if (PageAnon(old_page))
+ page_move_anon_rmap(old_page, vma, address);
set_huge_ptep_writable(vma, address, ptep);
return 0;
}
@@ -2631,10 +2628,14 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
vma, address);
}
- if (!pagecache_page) {
- page = pte_page(entry);
+ /*
+ * hugetlb_cow() requires page locks of pte_page(entry) and
+ * pagecache_page, so here we need take the former one
+ * when page != pagecache_page or !pagecache_page.
+ */
+ page = pte_page(entry);
+ if (page != pagecache_page)
lock_page(page);
- }
spin_lock(&mm->page_table_lock);
/* Check for a racing update before calling hugetlb_cow */
@@ -2661,9 +2662,8 @@ out_page_table_lock:
if (pagecache_page) {
unlock_page(pagecache_page);
put_page(pagecache_page);
- } else {
- unlock_page(page);
}
+ unlock_page(page);
out_mutex:
mutex_unlock(&hugetlb_instantiation_mutex);
--
1.7.2.2
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 14:41 ` Rik van Riel
-1 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-10 14:41 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Peter Zijlstra, Andi Kleen,
linux-mm
On 09/10/2010 12:23 AM, Naoya Horiguchi wrote:
> if(!trylock_page) block in avoidcopy path of hugetlb_cow() looks confusing
> and is buggy. Originally this trylock_page() is intended to make sure
> that old_page is locked even when old_page != pagecache_page, because then
> only pagecache_page is locked.
> This patch fixes it by moving page locking into hugetlb_fault().
>
> Signed-off-by: Naoya Horiguchi<n-horiguchi@ah.jp.nec.com>
Acked-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
@ 2010-09-10 14:41 ` Rik van Riel
0 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-10 14:41 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Peter Zijlstra, Andi Kleen,
linux-mm
On 09/10/2010 12:23 AM, Naoya Horiguchi wrote:
> if(!trylock_page) block in avoidcopy path of hugetlb_cow() looks confusing
> and is buggy. Originally this trylock_page() is intended to make sure
> that old_page is locked even when old_page != pagecache_page, because then
> only pagecache_page is locked.
> This patch fixes it by moving page locking into hugetlb_fault().
>
> Signed-off-by: Naoya Horiguchi<n-horiguchi@ah.jp.nec.com>
Acked-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 17:15 ` Linus Torvalds
-1 siblings, 0 replies; 42+ messages in thread
From: Linus Torvalds @ 2010-09-10 17:15 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
On Fri, 10 Sep 2010, Naoya Horiguchi wrote:
>
> - if (!pagecache_page) {
> - page = pte_page(entry);
> + /*
> + * hugetlb_cow() requires page locks of pte_page(entry) and
> + * pagecache_page, so here we need take the former one
> + * when page != pagecache_page or !pagecache_page.
> + */
> + page = pte_page(entry);
> + if (page != pagecache_page)
> lock_page(page);
Why isn't this a potential deadlock? You have two pages, and lock them
both. Is there some ordering guarantee that says that 'pagecache_page' and
'page' will always be in a certain relationship so that you cannot get
A->B and B->A lock ordering?
Please document that ordering rule if so.
Linus
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
@ 2010-09-10 17:15 ` Linus Torvalds
0 siblings, 0 replies; 42+ messages in thread
From: Linus Torvalds @ 2010-09-10 17:15 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
On Fri, 10 Sep 2010, Naoya Horiguchi wrote:
>
> - if (!pagecache_page) {
> - page = pte_page(entry);
> + /*
> + * hugetlb_cow() requires page locks of pte_page(entry) and
> + * pagecache_page, so here we need take the former one
> + * when page != pagecache_page or !pagecache_page.
> + */
> + page = pte_page(entry);
> + if (page != pagecache_page)
> lock_page(page);
Why isn't this a potential deadlock? You have two pages, and lock them
both. Is there some ordering guarantee that says that 'pagecache_page' and
'page' will always be in a certain relationship so that you cannot get
A->B and B->A lock ordering?
Please document that ordering rule if so.
Linus
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
2010-09-10 17:15 ` Linus Torvalds
@ 2010-09-23 23:50 ` Naoya Horiguchi
-1 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-23 23:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
Very sorry for late reply.
(Thank you for letting me know, Andi-san.)
On Fri, Sep 10, 2010 at 10:15:46AM -0700, Linus Torvalds wrote:
>
>
> On Fri, 10 Sep 2010, Naoya Horiguchi wrote:
> >
> > - if (!pagecache_page) {
> > - page = pte_page(entry);
> > + /*
> > + * hugetlb_cow() requires page locks of pte_page(entry) and
> > + * pagecache_page, so here we need take the former one
> > + * when page != pagecache_page or !pagecache_page.
> > + */
> > + page = pte_page(entry);
> > + if (page != pagecache_page)
> > lock_page(page);
>
> Why isn't this a potential deadlock? You have two pages, and lock them
> both. Is there some ordering guarantee that says that 'pagecache_page' and
> 'page' will always be in a certain relationship so that you cannot get
> A->B and B->A lock ordering?
Locking order is always pagecache -> page, so we are free from deadlock.
> Please document that ordering rule if so.
Yes. I comment it.
Please replace this patch by the following one.
Thanks,
Naoya Horiguchi
---
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Date: Thu, 9 Sep 2010 16:39:33 +0900
Subject: [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
if(!trylock_page) block in avoidcopy path of hugetlb_cow() looks confusing
and is buggy. Originally this trylock_page() is intended to make sure
that old_page is locked even when old_page != pagecache_page, because then
only pagecache_page is locked.
This patch fixes it by moving page locking into hugetlb_fault().
ChangeLog:
- add comment about deadlock.
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Acked-by: Rik van Riel <riel@redhat.com>
---
mm/hugetlb.c | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 9519f3f..c032738 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2324,11 +2324,8 @@ retry_avoidcopy:
* and just make the page writable */
avoidcopy = (page_mapcount(old_page) == 1);
if (avoidcopy) {
- if (!trylock_page(old_page)) {
- if (PageAnon(old_page))
- page_move_anon_rmap(old_page, vma, address);
- } else
- unlock_page(old_page);
+ if (PageAnon(old_page))
+ page_move_anon_rmap(old_page, vma, address);
set_huge_ptep_writable(vma, address, ptep);
return 0;
}
@@ -2631,10 +2628,16 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
vma, address);
}
- if (!pagecache_page) {
- page = pte_page(entry);
+ /*
+ * hugetlb_cow() requires page locks of pte_page(entry) and
+ * pagecache_page, so here we need take the former one
+ * when page != pagecache_page or !pagecache_page.
+ * Note that locking order is always pagecache_page -> page,
+ * so no worry about deadlock.
+ */
+ page = pte_page(entry);
+ if (page != pagecache_page)
lock_page(page);
- }
spin_lock(&mm->page_table_lock);
/* Check for a racing update before calling hugetlb_cow */
@@ -2661,9 +2664,8 @@ out_page_table_lock:
if (pagecache_page) {
unlock_page(pagecache_page);
put_page(pagecache_page);
- } else {
- unlock_page(page);
}
+ unlock_page(page);
out_mutex:
mutex_unlock(&hugetlb_instantiation_mutex);
--
1.7.2.3
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
@ 2010-09-23 23:50 ` Naoya Horiguchi
0 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-23 23:50 UTC (permalink / raw)
To: Linus Torvalds
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
Very sorry for late reply.
(Thank you for letting me know, Andi-san.)
On Fri, Sep 10, 2010 at 10:15:46AM -0700, Linus Torvalds wrote:
>
>
> On Fri, 10 Sep 2010, Naoya Horiguchi wrote:
> >
> > - if (!pagecache_page) {
> > - page = pte_page(entry);
> > + /*
> > + * hugetlb_cow() requires page locks of pte_page(entry) and
> > + * pagecache_page, so here we need take the former one
> > + * when page != pagecache_page or !pagecache_page.
> > + */
> > + page = pte_page(entry);
> > + if (page != pagecache_page)
> > lock_page(page);
>
> Why isn't this a potential deadlock? You have two pages, and lock them
> both. Is there some ordering guarantee that says that 'pagecache_page' and
> 'page' will always be in a certain relationship so that you cannot get
> A->B and B->A lock ordering?
Locking order is always pagecache -> page, so we are free from deadlock.
> Please document that ordering rule if so.
Yes. I comment it.
Please replace this patch by the following one.
Thanks,
Naoya Horiguchi
---
From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Date: Thu, 9 Sep 2010 16:39:33 +0900
Subject: [PATCH 3/4] hugetlb, rmap: fix confusing page locking in hugetlb_cow()
if(!trylock_page) block in avoidcopy path of hugetlb_cow() looks confusing
and is buggy. Originally this trylock_page() is intended to make sure
that old_page is locked even when old_page != pagecache_page, because then
only pagecache_page is locked.
This patch fixes it by moving page locking into hugetlb_fault().
ChangeLog:
- add comment about deadlock.
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
Acked-by: Rik van Riel <riel@redhat.com>
---
mm/hugetlb.c | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 9519f3f..c032738 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2324,11 +2324,8 @@ retry_avoidcopy:
* and just make the page writable */
avoidcopy = (page_mapcount(old_page) == 1);
if (avoidcopy) {
- if (!trylock_page(old_page)) {
- if (PageAnon(old_page))
- page_move_anon_rmap(old_page, vma, address);
- } else
- unlock_page(old_page);
+ if (PageAnon(old_page))
+ page_move_anon_rmap(old_page, vma, address);
set_huge_ptep_writable(vma, address, ptep);
return 0;
}
@@ -2631,10 +2628,16 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
vma, address);
}
- if (!pagecache_page) {
- page = pte_page(entry);
+ /*
+ * hugetlb_cow() requires page locks of pte_page(entry) and
+ * pagecache_page, so here we need take the former one
+ * when page != pagecache_page or !pagecache_page.
+ * Note that locking order is always pagecache_page -> page,
+ * so no worry about deadlock.
+ */
+ page = pte_page(entry);
+ if (page != pagecache_page)
lock_page(page);
- }
spin_lock(&mm->page_table_lock);
/* Check for a racing update before calling hugetlb_cow */
@@ -2661,9 +2664,8 @@ out_page_table_lock:
if (pagecache_page) {
unlock_page(pagecache_page);
put_page(pagecache_page);
- } else {
- unlock_page(page);
}
+ unlock_page(page);
out_mutex:
mutex_unlock(&hugetlb_instantiation_mutex);
--
1.7.2.3
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 4/4] hugetlb, rmap: add BUG_ON(!PageLocked) in hugetlb_add_anon_rmap()
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 4:23 ` Naoya Horiguchi
-1 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 4:23 UTC (permalink / raw)
To: LKML
Cc: Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
Confirming page lock is held in hugetlb_add_anon_rmap() may be useful
to detect possible future problems.
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
mm/rmap.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git v2.6.36-rc3/mm/rmap.c v2.6.36-rc3/mm/rmap.c
index 2854857..9d2ba01 100644
--- v2.6.36-rc3/mm/rmap.c
+++ v2.6.36-rc3/mm/rmap.c
@@ -1582,6 +1582,8 @@ void hugepage_add_anon_rmap(struct page *page,
{
struct anon_vma *anon_vma = vma->anon_vma;
int first;
+
+ BUG_ON(!PageLocked(page));
BUG_ON(!anon_vma);
BUG_ON(address < vma->vm_start || address >= vma->vm_end);
first = atomic_inc_and_test(&page->_mapcount);
--
1.7.2.2
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 4/4] hugetlb, rmap: add BUG_ON(!PageLocked) in hugetlb_add_anon_rmap()
@ 2010-09-10 4:23 ` Naoya Horiguchi
0 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 4:23 UTC (permalink / raw)
To: LKML
Cc: Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
Andi Kleen, linux-mm
Confirming page lock is held in hugetlb_add_anon_rmap() may be useful
to detect possible future problems.
Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
---
mm/rmap.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git v2.6.36-rc3/mm/rmap.c v2.6.36-rc3/mm/rmap.c
index 2854857..9d2ba01 100644
--- v2.6.36-rc3/mm/rmap.c
+++ v2.6.36-rc3/mm/rmap.c
@@ -1582,6 +1582,8 @@ void hugepage_add_anon_rmap(struct page *page,
{
struct anon_vma *anon_vma = vma->anon_vma;
int first;
+
+ BUG_ON(!PageLocked(page));
BUG_ON(!anon_vma);
BUG_ON(address < vma->vm_start || address >= vma->vm_end);
first = atomic_inc_and_test(&page->_mapcount);
--
1.7.2.2
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 4/4] hugetlb, rmap: add BUG_ON(!PageLocked) in hugetlb_add_anon_rmap()
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 14:42 ` Rik van Riel
-1 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-10 14:42 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Peter Zijlstra, Andi Kleen,
linux-mm
On 09/10/2010 12:23 AM, Naoya Horiguchi wrote:
> Confirming page lock is held in hugetlb_add_anon_rmap() may be useful
> to detect possible future problems.
>
> Signed-off-by: Naoya Horiguchi<n-horiguchi@ah.jp.nec.com>
Acked-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/4] hugetlb, rmap: add BUG_ON(!PageLocked) in hugetlb_add_anon_rmap()
@ 2010-09-10 14:42 ` Rik van Riel
0 siblings, 0 replies; 42+ messages in thread
From: Rik van Riel @ 2010-09-10 14:42 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Peter Zijlstra, Andi Kleen,
linux-mm
On 09/10/2010 12:23 AM, Naoya Horiguchi wrote:
> Confirming page lock is held in hugetlb_add_anon_rmap() may be useful
> to detect possible future problems.
>
> Signed-off-by: Naoya Horiguchi<n-horiguchi@ah.jp.nec.com>
Acked-by: Rik van Riel <riel@redhat.com>
--
All rights reversed
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/4] hugetlb, rmap: add BUG_ON(!PageLocked) in hugetlb_add_anon_rmap()
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 14:44 ` Andrea Arcangeli
-1 siblings, 0 replies; 42+ messages in thread
From: Andrea Arcangeli @ 2010-09-10 14:44 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Hugh Dickins, Christoph Lameter, Linus Torvalds,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
On Fri, Sep 10, 2010 at 01:23:06PM +0900, Naoya Horiguchi wrote:
> Confirming page lock is held in hugetlb_add_anon_rmap() may be useful
> to detect possible future problems.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
> mm/rmap.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
Acked-by: Andrea Arcangeli <aarcange@redhat.com>
It should probably be a VM_BUG_ON like do_page_add_anon_rmap, but
given the tiny hugetlbfs userbase it's ok.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/4] hugetlb, rmap: add BUG_ON(!PageLocked) in hugetlb_add_anon_rmap()
@ 2010-09-10 14:44 ` Andrea Arcangeli
0 siblings, 0 replies; 42+ messages in thread
From: Andrea Arcangeli @ 2010-09-10 14:44 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Hugh Dickins, Christoph Lameter, Linus Torvalds,
Andrew Morton, Rik van Riel, Peter Zijlstra, Andi Kleen,
linux-mm
On Fri, Sep 10, 2010 at 01:23:06PM +0900, Naoya Horiguchi wrote:
> Confirming page lock is held in hugetlb_add_anon_rmap() may be useful
> to detect possible future problems.
>
> Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>
> ---
> mm/rmap.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
Acked-by: Andrea Arcangeli <aarcange@redhat.com>
It should probably be a VM_BUG_ON like do_page_add_anon_rmap, but
given the tiny hugetlbfs userbase it's ok.
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 0/4] hugetlb, rmap: fixes and cleanups
2010-09-10 4:23 ` Naoya Horiguchi
@ 2010-09-10 9:04 ` Andi Kleen
-1 siblings, 0 replies; 42+ messages in thread
From: Andi Kleen @ 2010-09-10 9:04 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
linux-mm
On Fri, 10 Sep 2010 13:23:02 +0900
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
> Hi,
>
> These are fix and cleanup patches for hugepage rmapping.
> All these were pointed out in the following thread (last 4 messages.)
>
> http://thread.gmane.org/gmane.linux.kernel.mm/52334
Looks all good to me. It's not strictly hwpoison related
though, so I assume they are better with Andrew than my tree.
I assume they do not depend on the earlier patchkit?
-Andi
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 0/4] hugetlb, rmap: fixes and cleanups
@ 2010-09-10 9:04 ` Andi Kleen
0 siblings, 0 replies; 42+ messages in thread
From: Andi Kleen @ 2010-09-10 9:04 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: LKML, Andrea Arcangeli, Hugh Dickins, Christoph Lameter,
Linus Torvalds, Andrew Morton, Rik van Riel, Peter Zijlstra,
linux-mm
On Fri, 10 Sep 2010 13:23:02 +0900
Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
> Hi,
>
> These are fix and cleanup patches for hugepage rmapping.
> All these were pointed out in the following thread (last 4 messages.)
>
> http://thread.gmane.org/gmane.linux.kernel.mm/52334
Looks all good to me. It's not strictly hwpoison related
though, so I assume they are better with Andrew than my tree.
I assume they do not depend on the earlier patchkit?
-Andi
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 0/4] hugetlb, rmap: fixes and cleanups
2010-09-10 9:04 ` Andi Kleen
@ 2010-09-10 11:47 ` Naoya Horiguchi
-1 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 11:47 UTC (permalink / raw)
To: Andi Kleen
Cc: Naoya Horiguchi, LKML, Andrea Arcangeli, Hugh Dickins,
Christoph Lameter, Linus Torvalds, Andrew Morton, Rik van Riel,
Peter Zijlstra, linux-mm
On Fri, Sep 10, 2010 at 11:04:38AM +0200, Andi Kleen wrote:
> On Fri, 10 Sep 2010 13:23:02 +0900
> Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
>
> > Hi,
> >
> > These are fix and cleanup patches for hugepage rmapping.
> > All these were pointed out in the following thread (last 4 messages.)
> >
> > http://thread.gmane.org/gmane.linux.kernel.mm/52334
>
> Looks all good to me. It's not strictly hwpoison related
> though, so I assume they are better with Andrew than my tree.
Agreed.
> I assume they do not depend on the earlier patchkit?
No, all changes on this patchset update code merged with
"HWPOISON for hugepage" patchset.
Thanks,
Naoya Horiguchi
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 0/4] hugetlb, rmap: fixes and cleanups
@ 2010-09-10 11:47 ` Naoya Horiguchi
0 siblings, 0 replies; 42+ messages in thread
From: Naoya Horiguchi @ 2010-09-10 11:47 UTC (permalink / raw)
To: Andi Kleen
Cc: Naoya Horiguchi, LKML, Andrea Arcangeli, Hugh Dickins,
Christoph Lameter, Linus Torvalds, Andrew Morton, Rik van Riel,
Peter Zijlstra, linux-mm
On Fri, Sep 10, 2010 at 11:04:38AM +0200, Andi Kleen wrote:
> On Fri, 10 Sep 2010 13:23:02 +0900
> Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
>
> > Hi,
> >
> > These are fix and cleanup patches for hugepage rmapping.
> > All these were pointed out in the following thread (last 4 messages.)
> >
> > http://thread.gmane.org/gmane.linux.kernel.mm/52334
>
> Looks all good to me. It's not strictly hwpoison related
> though, so I assume they are better with Andrew than my tree.
Agreed.
> I assume they do not depend on the earlier patchkit?
No, all changes on this patchset update code merged with
"HWPOISON for hugepage" patchset.
Thanks,
Naoya Horiguchi
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 0/4] hugetlb, rmap: fixes and cleanups
2010-09-10 11:47 ` Naoya Horiguchi
@ 2010-09-10 14:30 ` Andi Kleen
-1 siblings, 0 replies; 42+ messages in thread
From: Andi Kleen @ 2010-09-10 14:30 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: Naoya Horiguchi, LKML, Andrea Arcangeli, Hugh Dickins,
Christoph Lameter, Linus Torvalds, Andrew Morton, Rik van Riel,
Peter Zijlstra, linux-mm
On Fri, 10 Sep 2010 20:47:59 +0900
Naoya Horiguchi <nao.horiguchi@gmail.com> wrote:
> On Fri, Sep 10, 2010 at 11:04:38AM +0200, Andi Kleen wrote:
> > On Fri, 10 Sep 2010 13:23:02 +0900
> > Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
> >
> > > Hi,
> > >
> > > These are fix and cleanup patches for hugepage rmapping.
> > > All these were pointed out in the following thread (last 4
> > > messages.)
> > >
> > > http://thread.gmane.org/gmane.linux.kernel.mm/52334
> >
> > Looks all good to me. It's not strictly hwpoison related
> > though, so I assume they are better with Andrew than my tree.
>
> Agreed.
>
> > I assume they do not depend on the earlier patchkit?
>
> No, all changes on this patchset update code merged with
> "HWPOISON for hugepage" patchset.
Ok then it's better for me to carry it anyways, otherwise Andrew
will be in dependency hell.
I will need some acks from MM hackers though. Anyone?
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 0/4] hugetlb, rmap: fixes and cleanups
@ 2010-09-10 14:30 ` Andi Kleen
0 siblings, 0 replies; 42+ messages in thread
From: Andi Kleen @ 2010-09-10 14:30 UTC (permalink / raw)
To: Naoya Horiguchi
Cc: Naoya Horiguchi, LKML, Andrea Arcangeli, Hugh Dickins,
Christoph Lameter, Linus Torvalds, Andrew Morton, Rik van Riel,
Peter Zijlstra, linux-mm
On Fri, 10 Sep 2010 20:47:59 +0900
Naoya Horiguchi <nao.horiguchi@gmail.com> wrote:
> On Fri, Sep 10, 2010 at 11:04:38AM +0200, Andi Kleen wrote:
> > On Fri, 10 Sep 2010 13:23:02 +0900
> > Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote:
> >
> > > Hi,
> > >
> > > These are fix and cleanup patches for hugepage rmapping.
> > > All these were pointed out in the following thread (last 4
> > > messages.)
> > >
> > > http://thread.gmane.org/gmane.linux.kernel.mm/52334
> >
> > Looks all good to me. It's not strictly hwpoison related
> > though, so I assume they are better with Andrew than my tree.
>
> Agreed.
>
> > I assume they do not depend on the earlier patchkit?
>
> No, all changes on this patchset update code merged with
> "HWPOISON for hugepage" patchset.
Ok then it's better for me to carry it anyways, otherwise Andrew
will be in dependency hell.
I will need some acks from MM hackers though. Anyone?
-Andi
--
ak@linux.intel.com -- Speaking for myself only.
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 42+ messages in thread