linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault.
@ 2012-10-25 16:44 Will Deacon
  2012-10-25 19:51 ` Johannes Weiner
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Will Deacon @ 2012-10-25 16:44 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-kernel, linux-arch, mhocko, peterz, akpm, Will Deacon,
	Chris Metcalf, Kirill A. Shutemov, Andrea Arcangeli

On x86 memory accesses to pages without the ACCESSED flag set result in the
ACCESSED flag being set automatically. With the ARM architecture a page access
fault is raised instead (and it will continue to be raised until the ACCESSED
flag is set for the appropriate PTE/PMD).

For normal memory pages, handle_pte_fault will call pte_mkyoung (effectively
setting the ACCESSED flag). For transparent huge pages, pmd_mkyoung will only
be called for a write fault.

This patch ensures that faults on transparent hugepages which do not result
in a CoW update the access flags for the faulting pmd.

Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Will Deacon <will.deacon@arm.com>
---

Ok chaps, I rebased this thing onto today's next (which basically
necessitated a rewrite) so I've reluctantly dropped my acks and kindly
ask if you could eyeball the new code, especially where the locking is
concerned. In the numa code (do_huge_pmd_prot_none), Peter checks again
that the page is not splitting, but I can't see why that is required.

Cheers,

Will

 include/linux/huge_mm.h |    4 ++++
 mm/huge_memory.c        |   22 ++++++++++++++++++++++
 mm/memory.c             |    7 ++++++-
 3 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
index 4f0f948..766fb27 100644
--- a/include/linux/huge_mm.h
+++ b/include/linux/huge_mm.h
@@ -8,6 +8,10 @@ extern int do_huge_pmd_anonymous_page(struct mm_struct *mm,
 extern int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
 			 pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long addr,
 			 struct vm_area_struct *vma);
+extern void huge_pmd_set_accessed(struct mm_struct *mm,
+				  struct vm_area_struct *vma,
+				  unsigned long address, pmd_t *pmd,
+				  pmd_t orig_pmd, int dirty);
 extern int do_huge_pmd_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
 			       unsigned long address, pmd_t *pmd,
 			       pmd_t orig_pmd);
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 3c14a96..f024d98 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -932,6 +932,28 @@ out:
 	return ret;
 }
 
+void huge_pmd_set_accessed(struct mm_struct *mm,
+			   struct vm_area_struct *vma,
+			   unsigned long address,
+			   pmd_t *pmd, pmd_t orig_pmd,
+			   int dirty)
+{
+	pmd_t entry;
+	unsigned long haddr;
+
+	spin_lock(&mm->page_table_lock);
+	if (unlikely(!pmd_same(*pmd, orig_pmd)))
+		goto unlock;
+
+	entry = pmd_mkyoung(orig_pmd);
+	haddr = address & HPAGE_PMD_MASK;
+	if (pmdp_set_access_flags(vma, haddr, pmd, entry, dirty))
+		update_mmu_cache_pmd(vma, address, pmd);
+
+unlock:
+	spin_unlock(&mm->page_table_lock);
+}
+
 static int do_huge_pmd_wp_page_fallback(struct mm_struct *mm,
 					struct vm_area_struct *vma,
 					unsigned long address,
diff --git a/mm/memory.c b/mm/memory.c
index f21ac1c..bcbc084 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3650,12 +3650,14 @@ retry:
 
 		barrier();
 		if (pmd_trans_huge(orig_pmd) && !pmd_trans_splitting(orig_pmd)) {
+			unsigned int dirty = flags & FAULT_FLAG_WRITE;
+
 			if (pmd_numa(vma, orig_pmd)) {
 				do_huge_pmd_numa_page(mm, vma, address, pmd,
 						      flags, orig_pmd);
 			}
 
-			if ((flags & FAULT_FLAG_WRITE) && !pmd_write(orig_pmd)) {
+			if (dirty && !pmd_write(orig_pmd)) {
 				ret = do_huge_pmd_wp_page(mm, vma, address, pmd,
 							  orig_pmd);
 				/*
@@ -3665,6 +3667,9 @@ retry:
 				 */
 				if (unlikely(ret & VM_FAULT_OOM))
 					goto retry;
+			} else {
+				huge_pmd_set_accessed(mm, vma, address, pmd,
+						      orig_pmd, dirty);
 			}
 
 			return ret;
-- 
1.7.4.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault.
  2012-10-25 16:44 [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault Will Deacon
@ 2012-10-25 19:51 ` Johannes Weiner
  2012-10-26  3:07   ` Ni zhan Chen
  2012-10-26  6:19 ` Ni zhan Chen
  2012-10-26  7:44 ` Kirill A. Shutemov
  2 siblings, 1 reply; 9+ messages in thread
From: Johannes Weiner @ 2012-10-25 19:51 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-mm, linux-kernel, linux-arch, mhocko, peterz, akpm,
	Chris Metcalf, Kirill A. Shutemov, Andrea Arcangeli

On Thu, Oct 25, 2012 at 05:44:31PM +0100, Will Deacon wrote:
> On x86 memory accesses to pages without the ACCESSED flag set result in the
> ACCESSED flag being set automatically. With the ARM architecture a page access
> fault is raised instead (and it will continue to be raised until the ACCESSED
> flag is set for the appropriate PTE/PMD).
> 
> For normal memory pages, handle_pte_fault will call pte_mkyoung (effectively
> setting the ACCESSED flag). For transparent huge pages, pmd_mkyoung will only
> be called for a write fault.
> 
> This patch ensures that faults on transparent hugepages which do not result
> in a CoW update the access flags for the faulting pmd.
> 
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

> Ok chaps, I rebased this thing onto today's next (which basically
> necessitated a rewrite) so I've reluctantly dropped my acks and kindly
> ask if you could eyeball the new code, especially where the locking is
> concerned. In the numa code (do_huge_pmd_prot_none), Peter checks again
> that the page is not splitting, but I can't see why that is required.

I don't either.  If the thing was splitting when the fault happened,
that path is not taken.  And the locked pmd_same() check should rule
out splitting setting in after testing pmd_trans_huge_splitting().

Peter?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault.
  2012-10-25 19:51 ` Johannes Weiner
@ 2012-10-26  3:07   ` Ni zhan Chen
  0 siblings, 0 replies; 9+ messages in thread
From: Ni zhan Chen @ 2012-10-26  3:07 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: Will Deacon, linux-mm, linux-kernel, linux-arch, mhocko, peterz,
	akpm, Chris Metcalf, Kirill A. Shutemov, Andrea Arcangeli

On 10/26/2012 03:51 AM, Johannes Weiner wrote:
> On Thu, Oct 25, 2012 at 05:44:31PM +0100, Will Deacon wrote:
>> On x86 memory accesses to pages without the ACCESSED flag set result in the
>> ACCESSED flag being set automatically. With the ARM architecture a page access
>> fault is raised instead (and it will continue to be raised until the ACCESSED
>> flag is set for the appropriate PTE/PMD).
>>
>> For normal memory pages, handle_pte_fault will call pte_mkyoung (effectively
>> setting the ACCESSED flag). For transparent huge pages, pmd_mkyoung will only
>> be called for a write fault.
>>
>> This patch ensures that faults on transparent hugepages which do not result
>> in a CoW update the access flags for the faulting pmd.
>>
>> Cc: Chris Metcalf <cmetcalf@tilera.com>
>> Cc: Kirill A. Shutemov <kirill@shutemov.name>
>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>> Signed-off-by: Will Deacon <will.deacon@arm.com>
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
>
>> Ok chaps, I rebased this thing onto today's next (which basically
>> necessitated a rewrite) so I've reluctantly dropped my acks and kindly
>> ask if you could eyeball the new code, especially where the locking is
>> concerned. In the numa code (do_huge_pmd_prot_none), Peter checks again
>> that the page is not splitting, but I can't see why that is required.
> I don't either.  If the thing was splitting when the fault happened,
> that path is not taken.  And the locked pmd_same() check should rule
> out splitting setting in after testing pmd_trans_huge_splitting().

Why I can't find function pmd_trans_huge_splitting() you mentioned in 
latest mainline codes and linux-next?

>
> Peter?
>
> --
> 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] 9+ messages in thread

* Re: [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault.
  2012-10-25 16:44 [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault Will Deacon
  2012-10-25 19:51 ` Johannes Weiner
@ 2012-10-26  6:19 ` Ni zhan Chen
  2012-10-26  9:34   ` Will Deacon
  2012-10-26  7:44 ` Kirill A. Shutemov
  2 siblings, 1 reply; 9+ messages in thread
From: Ni zhan Chen @ 2012-10-26  6:19 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-mm, linux-kernel, linux-arch, mhocko, peterz, akpm,
	Chris Metcalf, Kirill A. Shutemov, Andrea Arcangeli

On 10/26/2012 12:44 AM, Will Deacon wrote:
> On x86 memory accesses to pages without the ACCESSED flag set result in the
> ACCESSED flag being set automatically. With the ARM architecture a page access
> fault is raised instead (and it will continue to be raised until the ACCESSED
> flag is set for the appropriate PTE/PMD).
>
> For normal memory pages, handle_pte_fault will call pte_mkyoung (effectively
> setting the ACCESSED flag). For transparent huge pages, pmd_mkyoung will only
> be called for a write fault.
>
> This patch ensures that faults on transparent hugepages which do not result
> in a CoW update the access flags for the faulting pmd.

Could you write changlog?

>
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>
> Ok chaps, I rebased this thing onto today's next (which basically
> necessitated a rewrite) so I've reluctantly dropped my acks and kindly
> ask if you could eyeball the new code, especially where the locking is
> concerned. In the numa code (do_huge_pmd_prot_none), Peter checks again
> that the page is not splitting, but I can't see why that is required.
>
> Cheers,
>
> Will

Could you explain why you not call pmd_trans_huge_lock to confirm the 
pmd is splitting or stable as Andrea point out?

>
>   include/linux/huge_mm.h |    4 ++++
>   mm/huge_memory.c        |   22 ++++++++++++++++++++++
>   mm/memory.c             |    7 ++++++-
>   3 files changed, 32 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/huge_mm.h b/include/linux/huge_mm.h
> index 4f0f948..766fb27 100644
> --- a/include/linux/huge_mm.h
> +++ b/include/linux/huge_mm.h
> @@ -8,6 +8,10 @@ extern int do_huge_pmd_anonymous_page(struct mm_struct *mm,
>   extern int copy_huge_pmd(struct mm_struct *dst_mm, struct mm_struct *src_mm,
>   			 pmd_t *dst_pmd, pmd_t *src_pmd, unsigned long addr,
>   			 struct vm_area_struct *vma);
> +extern void huge_pmd_set_accessed(struct mm_struct *mm,
> +				  struct vm_area_struct *vma,
> +				  unsigned long address, pmd_t *pmd,
> +				  pmd_t orig_pmd, int dirty);
>   extern int do_huge_pmd_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
>   			       unsigned long address, pmd_t *pmd,
>   			       pmd_t orig_pmd);
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 3c14a96..f024d98 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -932,6 +932,28 @@ out:
>   	return ret;
>   }
>   
> +void huge_pmd_set_accessed(struct mm_struct *mm,
> +			   struct vm_area_struct *vma,
> +			   unsigned long address,
> +			   pmd_t *pmd, pmd_t orig_pmd,
> +			   int dirty)
> +{
> +	pmd_t entry;
> +	unsigned long haddr;
> +
> +	spin_lock(&mm->page_table_lock);
> +	if (unlikely(!pmd_same(*pmd, orig_pmd)))
> +		goto unlock;
> +
> +	entry = pmd_mkyoung(orig_pmd);
> +	haddr = address & HPAGE_PMD_MASK;
> +	if (pmdp_set_access_flags(vma, haddr, pmd, entry, dirty))
> +		update_mmu_cache_pmd(vma, address, pmd);
> +
> +unlock:
> +	spin_unlock(&mm->page_table_lock);
> +}
> +
>   static int do_huge_pmd_wp_page_fallback(struct mm_struct *mm,
>   					struct vm_area_struct *vma,
>   					unsigned long address,
> diff --git a/mm/memory.c b/mm/memory.c
> index f21ac1c..bcbc084 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3650,12 +3650,14 @@ retry:
>   
>   		barrier();
>   		if (pmd_trans_huge(orig_pmd) && !pmd_trans_splitting(orig_pmd)) {
> +			unsigned int dirty = flags & FAULT_FLAG_WRITE;
> +
>   			if (pmd_numa(vma, orig_pmd)) {
>   				do_huge_pmd_numa_page(mm, vma, address, pmd,
>   						      flags, orig_pmd);
>   			}
>   
> -			if ((flags & FAULT_FLAG_WRITE) && !pmd_write(orig_pmd)) {
> +			if (dirty && !pmd_write(orig_pmd)) {
>   				ret = do_huge_pmd_wp_page(mm, vma, address, pmd,
>   							  orig_pmd);
>   				/*
> @@ -3665,6 +3667,9 @@ retry:
>   				 */
>   				if (unlikely(ret & VM_FAULT_OOM))
>   					goto retry;
> +			} else {
> +				huge_pmd_set_accessed(mm, vma, address, pmd,
> +						      orig_pmd, dirty);
>   			}
>   
>   			return ret;


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault.
  2012-10-25 16:44 [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault Will Deacon
  2012-10-25 19:51 ` Johannes Weiner
  2012-10-26  6:19 ` Ni zhan Chen
@ 2012-10-26  7:44 ` Kirill A. Shutemov
  2012-10-26  9:07   ` Will Deacon
  2 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2012-10-26  7:44 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-mm, linux-kernel, linux-arch, mhocko, peterz, akpm,
	Chris Metcalf, Andrea Arcangeli

On Thu, Oct 25, 2012 at 05:44:31PM +0100, Will Deacon wrote:
> On x86 memory accesses to pages without the ACCESSED flag set result in the
> ACCESSED flag being set automatically. With the ARM architecture a page access
> fault is raised instead (and it will continue to be raised until the ACCESSED
> flag is set for the appropriate PTE/PMD).
> 
> For normal memory pages, handle_pte_fault will call pte_mkyoung (effectively
> setting the ACCESSED flag). For transparent huge pages, pmd_mkyoung will only
> be called for a write fault.
> 
> This patch ensures that faults on transparent hugepages which do not result
> in a CoW update the access flags for the faulting pmd.
> 
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> Cc: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
> 
> Ok chaps, I rebased this thing onto today's next (which basically
> necessitated a rewrite) so I've reluctantly dropped my acks and kindly
> ask if you could eyeball the new code, especially where the locking is
> concerned. In the numa code (do_huge_pmd_prot_none), Peter checks again
> that the page is not splitting, but I can't see why that is required.

In handle_mm_fault() we check if the pmd is under splitting without
page_table_lock. It's kind of speculative cheap check. We need to re-check
if the PMD is really not under splitting after taking page_table_lock.

See section "Locking in hugepage aware code" in Documentation/vm/transhuge.txt

-- 
 Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault.
  2012-10-26  7:44 ` Kirill A. Shutemov
@ 2012-10-26  9:07   ` Will Deacon
  2012-10-26 10:15     ` Kirill A. Shutemov
  0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2012-10-26  9:07 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: linux-mm, linux-kernel, linux-arch, mhocko, peterz, akpm,
	Chris Metcalf, Andrea Arcangeli

On Fri, Oct 26, 2012 at 08:44:35AM +0100, Kirill A. Shutemov wrote:
> On Thu, Oct 25, 2012 at 05:44:31PM +0100, Will Deacon wrote:
> > On x86 memory accesses to pages without the ACCESSED flag set result in the
> > ACCESSED flag being set automatically. With the ARM architecture a page access
> > fault is raised instead (and it will continue to be raised until the ACCESSED
> > flag is set for the appropriate PTE/PMD).
> > 
> > For normal memory pages, handle_pte_fault will call pte_mkyoung (effectively
> > setting the ACCESSED flag). For transparent huge pages, pmd_mkyoung will only
> > be called for a write fault.
> > 
> > This patch ensures that faults on transparent hugepages which do not result
> > in a CoW update the access flags for the faulting pmd.
> > 
> > Cc: Chris Metcalf <cmetcalf@tilera.com>
> > Cc: Kirill A. Shutemov <kirill@shutemov.name>
> > Cc: Andrea Arcangeli <aarcange@redhat.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> > 
> > Ok chaps, I rebased this thing onto today's next (which basically
> > necessitated a rewrite) so I've reluctantly dropped my acks and kindly
> > ask if you could eyeball the new code, especially where the locking is
> > concerned. In the numa code (do_huge_pmd_prot_none), Peter checks again
> > that the page is not splitting, but I can't see why that is required.
> 
> In handle_mm_fault() we check if the pmd is under splitting without
> page_table_lock. It's kind of speculative cheap check. We need to re-check
> if the PMD is really not under splitting after taking page_table_lock.

I appreciate the need to check whether the thing is splitting, but I thought
that the pmd_same(*pmd, orig_pmd) check after taking the page_table_lock
would be sufficient, because we know that the entry hasn't changed and that
it wasn't splitting before we took the lock. This also mirrors the approach
taken by do_huge_pmd_wp_page.

Is there something I'm missing?

Cheers,

Will

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault.
  2012-10-26  6:19 ` Ni zhan Chen
@ 2012-10-26  9:34   ` Will Deacon
  2012-10-26  9:49     ` Ni zhan Chen
  0 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2012-10-26  9:34 UTC (permalink / raw)
  To: Ni zhan Chen
  Cc: linux-mm, linux-kernel, linux-arch, mhocko, peterz, akpm,
	Chris Metcalf, Kirill A. Shutemov, Andrea Arcangeli

On Fri, Oct 26, 2012 at 07:19:55AM +0100, Ni zhan Chen wrote:
> On 10/26/2012 12:44 AM, Will Deacon wrote:
> > On x86 memory accesses to pages without the ACCESSED flag set result in the
> > ACCESSED flag being set automatically. With the ARM architecture a page access
> > fault is raised instead (and it will continue to be raised until the ACCESSED
> > flag is set for the appropriate PTE/PMD).
> >
> > For normal memory pages, handle_pte_fault will call pte_mkyoung (effectively
> > setting the ACCESSED flag). For transparent huge pages, pmd_mkyoung will only
> > be called for a write fault.
> >
> > This patch ensures that faults on transparent hugepages which do not result
> > in a CoW update the access flags for the faulting pmd.
> 
> Could you write changlog?

>From v2? I included something below my SoB. The code should do exactly the
same as before, it's just rebased onto next so that I can play nicely with
Peter's patches.

> >
> > Cc: Chris Metcalf <cmetcalf@tilera.com>
> > Cc: Kirill A. Shutemov <kirill@shutemov.name>
> > Cc: Andrea Arcangeli <aarcange@redhat.com>
> > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > ---
> >
> > Ok chaps, I rebased this thing onto today's next (which basically
> > necessitated a rewrite) so I've reluctantly dropped my acks and kindly
> > ask if you could eyeball the new code, especially where the locking is
> > concerned. In the numa code (do_huge_pmd_prot_none), Peter checks again
> > that the page is not splitting, but I can't see why that is required.
> >
> > Cheers,
> >
> > Will
> 
> Could you explain why you not call pmd_trans_huge_lock to confirm the 
> pmd is splitting or stable as Andrea point out?

The way handle_mm_fault is now structured after the numa changes means that
we only enter the huge pmd page aging code if the entry wasn't splitting
before taking the lock, so it seemed a bit gratuitous to jump through those
hoops again in pmd_trans_huge_lock.

Will

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault.
  2012-10-26  9:34   ` Will Deacon
@ 2012-10-26  9:49     ` Ni zhan Chen
  0 siblings, 0 replies; 9+ messages in thread
From: Ni zhan Chen @ 2012-10-26  9:49 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-mm, linux-kernel, linux-arch, mhocko, peterz, akpm,
	Chris Metcalf, Kirill A. Shutemov, Andrea Arcangeli

On 10/26/2012 05:34 PM, Will Deacon wrote:
> On Fri, Oct 26, 2012 at 07:19:55AM +0100, Ni zhan Chen wrote:
>> On 10/26/2012 12:44 AM, Will Deacon wrote:
>>> On x86 memory accesses to pages without the ACCESSED flag set result in the
>>> ACCESSED flag being set automatically. With the ARM architecture a page access
>>> fault is raised instead (and it will continue to be raised until the ACCESSED
>>> flag is set for the appropriate PTE/PMD).
>>>
>>> For normal memory pages, handle_pte_fault will call pte_mkyoung (effectively
>>> setting the ACCESSED flag). For transparent huge pages, pmd_mkyoung will only
>>> be called for a write fault.
>>>
>>> This patch ensures that faults on transparent hugepages which do not result
>>> in a CoW update the access flags for the faulting pmd.
>> Could you write changlog?
> >From v2? I included something below my SoB. The code should do exactly the
> same as before, it's just rebased onto next so that I can play nicely with
> Peter's patches.
>
>>> Cc: Chris Metcalf <cmetcalf@tilera.com>
>>> Cc: Kirill A. Shutemov <kirill@shutemov.name>
>>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>>> Signed-off-by: Will Deacon <will.deacon@arm.com>
>>> ---
>>>
>>> Ok chaps, I rebased this thing onto today's next (which basically
>>> necessitated a rewrite) so I've reluctantly dropped my acks and kindly
>>> ask if you could eyeball the new code, especially where the locking is
>>> concerned. In the numa code (do_huge_pmd_prot_none), Peter checks again
>>> that the page is not splitting, but I can't see why that is required.
>>>
>>> Cheers,
>>>
>>> Will
>> Could you explain why you not call pmd_trans_huge_lock to confirm the
>> pmd is splitting or stable as Andrea point out?
> The way handle_mm_fault is now structured after the numa changes means that
> we only enter the huge pmd page aging code if the entry wasn't splitting

Why you call it huge pmd page *aging* code?

Regards,
Chen

> before taking the lock, so it seemed a bit gratuitous to jump through those
> hoops again in pmd_trans_huge_lock.
>
> Will
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault.
  2012-10-26  9:07   ` Will Deacon
@ 2012-10-26 10:15     ` Kirill A. Shutemov
  0 siblings, 0 replies; 9+ messages in thread
From: Kirill A. Shutemov @ 2012-10-26 10:15 UTC (permalink / raw)
  To: Will Deacon
  Cc: linux-mm, linux-kernel, linux-arch, mhocko, peterz, akpm,
	Chris Metcalf, Andrea Arcangeli, Peter Zijlstra

On Fri, Oct 26, 2012 at 10:07:15AM +0100, Will Deacon wrote:
> On Fri, Oct 26, 2012 at 08:44:35AM +0100, Kirill A. Shutemov wrote:
> > On Thu, Oct 25, 2012 at 05:44:31PM +0100, Will Deacon wrote:
> > > On x86 memory accesses to pages without the ACCESSED flag set result in the
> > > ACCESSED flag being set automatically. With the ARM architecture a page access
> > > fault is raised instead (and it will continue to be raised until the ACCESSED
> > > flag is set for the appropriate PTE/PMD).
> > > 
> > > For normal memory pages, handle_pte_fault will call pte_mkyoung (effectively
> > > setting the ACCESSED flag). For transparent huge pages, pmd_mkyoung will only
> > > be called for a write fault.
> > > 
> > > This patch ensures that faults on transparent hugepages which do not result
> > > in a CoW update the access flags for the faulting pmd.
> > > 
> > > Cc: Chris Metcalf <cmetcalf@tilera.com>
> > > Cc: Kirill A. Shutemov <kirill@shutemov.name>
> > > Cc: Andrea Arcangeli <aarcange@redhat.com>
> > > Signed-off-by: Will Deacon <will.deacon@arm.com>
> > > ---
> > > 
> > > Ok chaps, I rebased this thing onto today's next (which basically
> > > necessitated a rewrite) so I've reluctantly dropped my acks and kindly
> > > ask if you could eyeball the new code, especially where the locking is
> > > concerned. In the numa code (do_huge_pmd_prot_none), Peter checks again
> > > that the page is not splitting, but I can't see why that is required.
> > 
> > In handle_mm_fault() we check if the pmd is under splitting without
> > page_table_lock. It's kind of speculative cheap check. We need to re-check
> > if the PMD is really not under splitting after taking page_table_lock.
> 
> I appreciate the need to check whether the thing is splitting, but I thought
> that the pmd_same(*pmd, orig_pmd) check after taking the page_table_lock
> would be sufficient, because we know that the entry hasn't changed and that
> it wasn't splitting before we took the lock. This also mirrors the approach
> taken by do_huge_pmd_wp_page.
> 
> Is there something I'm missing?

Hm.. You're correct from my POV.

Acked-by: Kirill A. Shutemov <kirill@shutemov.name>

I think the check in do_huge_pmd_prot_none() is redundant. It only add
latency. I'll prepare patch to remove it.

-- 
 Kirill A. Shutemov

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-10-26 10:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-25 16:44 [PATCH v3] mm: thp: Set the accessed flag for old pages on access fault Will Deacon
2012-10-25 19:51 ` Johannes Weiner
2012-10-26  3:07   ` Ni zhan Chen
2012-10-26  6:19 ` Ni zhan Chen
2012-10-26  9:34   ` Will Deacon
2012-10-26  9:49     ` Ni zhan Chen
2012-10-26  7:44 ` Kirill A. Shutemov
2012-10-26  9:07   ` Will Deacon
2012-10-26 10:15     ` Kirill A. Shutemov

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).