Patchwork [-V6,18/27] mm/THP: withdraw the pgtable after pmdp related operations

login
register
mail settings
Submitter Aneesh Kumar K.V
Date April 24, 2013, 9:08 a.m.
Message ID <87a9oo2t5a.fsf@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/239089/
State Accepted
Commit a6bf2bb03e5bad7e9289d80ecb5faac11630c7ab
Headers show

Comments

Aneesh Kumar K.V - April 24, 2013, 9:08 a.m.
Andrea Arcangeli <aarcange@redhat.com> writes:

> Hi,
>
> On Mon, Apr 22, 2013 at 03:30:52PM +0530, Aneesh Kumar K.V wrote:
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> 
>> For architectures like ppc64 we look at deposited pgtable when
>> calling pmdp_get_and_clear. So do the pgtable_trans_huge_withdraw
>> after finishing pmdp related operations.
>> 
>> Cc: Andrea Arcangeli <aarcange@redhat.com>
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>>  mm/huge_memory.c |    3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 84f3180..2a43782 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -1363,9 +1363,10 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>>  		struct page *page;
>>  		pgtable_t pgtable;
>>  		pmd_t orig_pmd;
>> -		pgtable = pgtable_trans_huge_withdraw(tlb->mm, pmd);
>> +
>>  		orig_pmd = pmdp_get_and_clear(tlb->mm, addr, pmd);
>>  		tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
>> +		pgtable = pgtable_trans_huge_withdraw(tlb->mm, pmd);
>>  		if (is_huge_zero_pmd(orig_pmd)) {
>>  			tlb->mm->nr_ptes--;
>>  			spin_unlock(&tlb->mm->page_table_lock);
>
> I think here a comment inline (not only in the commit msg) is in
> order. Otherwise it's hard to imagine others to be aware of this arch
> detail when they will read the code later. So it would be prone to
> break later without a comment.

How about ?

From 7444a5eda33c00eea465b51c405cb830c57513b7 Mon Sep 17 00:00:00 2001
From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
Date: Wed, 6 Mar 2013 12:50:37 +0530
Subject: [PATCH] mm/THP: withdraw the pgtable after pmdp related operations

For architectures like ppc64 we look at deposited pgtable when
calling pmdp_get_and_clear. So do the pgtable_trans_huge_withdraw
after finishing pmdp related operations.

Cc: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 mm/huge_memory.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
Andrea Arcangeli - April 24, 2013, 3:14 p.m.
Hi,

On Wed, Apr 24, 2013 at 02:38:01PM +0530, Aneesh Kumar K.V wrote:
> From 7444a5eda33c00eea465b51c405cb830c57513b7 Mon Sep 17 00:00:00 2001
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> Date: Wed, 6 Mar 2013 12:50:37 +0530
> Subject: [PATCH] mm/THP: withdraw the pgtable after pmdp related operations
> 
> For architectures like ppc64 we look at deposited pgtable when
> calling pmdp_get_and_clear. So do the pgtable_trans_huge_withdraw
> after finishing pmdp related operations.
> 
> Cc: Andrea Arcangeli <aarcange@redhat.com>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  mm/huge_memory.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

Reviewed-by: Andrea Arcangeli <aarcange@redhat.com>

> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 84f3180..21c5ebd 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1363,9 +1363,15 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>  		struct page *page;
>  		pgtable_t pgtable;
>  		pmd_t orig_pmd;
> -		pgtable = pgtable_trans_huge_withdraw(tlb->mm, pmd);
> +		/*
> +		 * For architectures like ppc64 we look at deposited pgtable
> +		 * when calling pmdp_get_and_clear. So do the
> +		 * pgtable_trans_huge_withdraw after finishing pmdp related
> +		 * operations.
> +		 */
>  		orig_pmd = pmdp_get_and_clear(tlb->mm, addr, pmd);
>  		tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
> +		pgtable = pgtable_trans_huge_withdraw(tlb->mm, pmd);

So I assume you're going to check the pmdp pointer address in
_withdraw, as the *pmd content is already clear. And that you're
checking the deposited pmd earlier in pmdp_get_and_clear. A bit
strange overall not seeing how exactly you're using the new parameter
and the deposited pmds, but safe.

Thanks,
Andrea

Patch

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 84f3180..21c5ebd 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1363,9 +1363,15 @@  int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
 		struct page *page;
 		pgtable_t pgtable;
 		pmd_t orig_pmd;
-		pgtable = pgtable_trans_huge_withdraw(tlb->mm, pmd);
+		/*
+		 * For architectures like ppc64 we look at deposited pgtable
+		 * when calling pmdp_get_and_clear. So do the
+		 * pgtable_trans_huge_withdraw after finishing pmdp related
+		 * operations.
+		 */
 		orig_pmd = pmdp_get_and_clear(tlb->mm, addr, pmd);
 		tlb_remove_pmd_tlb_entry(tlb, pmd, addr);
+		pgtable = pgtable_trans_huge_withdraw(tlb->mm, pmd);
 		if (is_huge_zero_pmd(orig_pmd)) {
 			tlb->mm->nr_ptes--;
 			spin_unlock(&tlb->mm->page_table_lock);