Skip to contentSkip to conversation

mm: fix account pmd page to the process

PATCH In review zhongjiang 4 participants 12 messages Jun 16, 2016 – Jun 18, 2016

Subsystems (2)
lore
Share & tools
lore

zhongjiang

author
#0 lore raw

PATCH 1/1 mm: fix account pmd page to the process

From: zhong jiang <[email protected]>

when a process acquire a pmd table shared by other process, we increase the account to current process. otherwise, a race result in other tasks have set the pud entry. so it no need to increase it.

---
 mm/hugetlb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
mm/hugetlb.c +2 -3
index 19d0d08..3b025c5 100644
@@ -4189,10 +4189,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
-	} else {
+	} else 
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
-	}
+
 	spin_unlock(ptl);
 out:
 	pte = (pte_t *)pmd_alloc(mm, pud, addr);
Signature · 1 line
1.8.3.1
#1 lore raw
Quoted 16 lines · zhongjiang

From: zhong jiang <[email protected]>

when a process acquire a pmd table shared by other process, we increase the account to current process. otherwise, a race result in other tasks have set the pud entry. so it no need to increase it.

---
 mm/hugetlb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
mm/hugetlb.c +0 -0
index 19d0d08..3b025c5 100644

mm/hugetlb.c pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud) · line 4195

Quoted 8 lines
mm/hugetlb.c +1 -3
@@ -4189,10 +4189,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
-	} else {
+	} else 
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
-	}

The code is quite puzzling but is this correct? Shouldn't we rather do mm_dec_nr_pmds(mm) in that path to undo the previous inc?

Quoted 6 lines
mm/hugetlb.c +1 -0
+
 	spin_unlock(ptl);
 out:
 	pte = (pte_t *)pmd_alloc(mm, pud, addr);
Signature · 1 line
1.8.3.1
Signature · 8 lines
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#2 lore raw

[It seems that this patch has been sent several times and this particular copy didn't add Kirill who has added this code CC him now]

Quoted 39 lines · Michal Hocko
Quoted 16 lines · zhongjiang

From: zhong jiang <[email protected]>

when a process acquire a pmd table shared by other process, we increase the account to current process. otherwise, a race result in other tasks have set the pud entry. so it no need to increase it.

---
 mm/hugetlb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
mm/hugetlb.c +0 -0
index 19d0d08..3b025c5 100644

mm/hugetlb.c pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud) · line 4195

Quoted 8 lines
mm/hugetlb.c +1 -3
@@ -4189,10 +4189,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
-	} else {
+	} else 
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
-	}

The code is quite puzzling but is this correct? Shouldn't we rather do mm_dec_nr_pmds(mm) in that path to undo the previous inc?

Quoted 6 lines
mm/hugetlb.c +1 -0
+
 	spin_unlock(ptl);
 out:
 	pte = (pte_t *)pmd_alloc(mm, pud, addr);
Signature · 1 line
1.8.3.1
Signature · 2 lines
Michal Hocko
SUSE Labs
Signature · 8 lines
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#3 lore raw
Quoted 32 lines · Michal Hocko

[It seems that this patch has been sent several times and this particular copy didn't add Kirill who has added this code CC him now]

Quoted 28 lines · Michal Hocko
Quoted 16 lines · zhongjiang

From: zhong jiang <[email protected]>

when a process acquire a pmd table shared by other process, we increase the account to current process. otherwise, a race result in other tasks have set the pud entry. so it no need to increase it.

---
 mm/hugetlb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
mm/hugetlb.c +0 -0
index 19d0d08..3b025c5 100644

mm/hugetlb.c pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud) · line 4195

Quoted 8 lines
mm/hugetlb.c +1 -3
@@ -4189,10 +4189,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
-	} else {
+	} else 
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
-	}

The code is quite puzzling but is this correct? Shouldn't we rather do mm_dec_nr_pmds(mm) in that path to undo the previous inc?

I agree that the code is quite puzzling. :(

However, if this were an issue I would have expected to see some reports. Oracle DB makes use of this feature (shared page tables) and if the pmd count is wrong we would catch it in check_mm() at exit time.

Upon closer examination, I believe the code in question is never executed. Note the callers of huge_pmd_share. The calling code looks like:

                        if (want_pmd_share() && pud_none(*pud))
                                pte = huge_pmd_share(mm, addr, pud);
                        else
                                pte = (pte_t *)pmd_alloc(mm, pud, addr);

Therefore, we do not call huge_pmd_share unless pud_none(*pud). The code in question is only executed when !pud_none(*pud).

I think that entire if/else statement can be removed. We know pud_none(*pud), so just do pud_populate().

Signature · 1 line
Mike Kravetz
Quoted 8 lines
Quoted 8 lines
Quoted 7 lines
mm/hugetlb.c +1 -0
+
 	spin_unlock(ptl);
 out:
 	pte = (pte_t *)pmd_alloc(mm, pud, addr);
Signature · 1 line
1.8.3.1
Quoted 4 lines
Quoted 3 lines
Signature · 2 lines
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#4 lore raw
Quoted 43 lines · Mike Kravetz
Quoted 32 lines · Michal Hocko

[It seems that this patch has been sent several times and this particular copy didn't add Kirill who has added this code CC him now]

Quoted 28 lines · Michal Hocko
Quoted 16 lines · zhongjiang

From: zhong jiang <[email protected]>

when a process acquire a pmd table shared by other process, we increase the account to current process. otherwise, a race result in other tasks have set the pud entry. so it no need to increase it.

---
 mm/hugetlb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
mm/hugetlb.c +0 -0
index 19d0d08..3b025c5 100644

mm/hugetlb.c pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud) · line 4195

Quoted 8 lines
mm/hugetlb.c +1 -3
@@ -4189,10 +4189,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
-	} else {
+	} else 
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
-	}

The code is quite puzzling but is this correct? Shouldn't we rather do mm_dec_nr_pmds(mm) in that path to undo the previous inc?

I agree that the code is quite puzzling. :(

However, if this were an issue I would have expected to see some reports. Oracle DB makes use of this feature (shared page tables) and if the pmd count is wrong we would catch it in check_mm() at exit time.

Upon closer examination, I believe the code in question is never executed. Note the callers of huge_pmd_share. The calling code looks like:

Quoted 7 lines
                        if (want_pmd_share() && pud_none(*pud))
                                pte = huge_pmd_share(mm, addr, pud);
                        else
                                pte = (pte_t *)pmd_alloc(mm, pud, addr);

Therefore, we do not call huge_pmd_share unless pud_none(*pud). The code in question is only executed when !pud_none(*pud).

My understanding is that the check is needed after we retake page lock because we might have raced with other thread. But it's been quite some time since I've looked at hugetlb locking and page table sharing code.

Signature · 8 lines
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#5 lore raw
Quoted 49 lines · Michal Hocko
Quoted 48 lines · Mike Kravetz
Quoted 32 lines · Michal Hocko

[It seems that this patch has been sent several times and this particular copy didn't add Kirill who has added this code CC him now]

Quoted 28 lines · Michal Hocko
Quoted 16 lines · zhongjiang

From: zhong jiang <[email protected]>

when a process acquire a pmd table shared by other process, we increase the account to current process. otherwise, a race result in other tasks have set the pud entry. so it no need to increase it.

---
 mm/hugetlb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
mm/hugetlb.c +0 -0
index 19d0d08..3b025c5 100644

mm/hugetlb.c pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud) · line 4195

Quoted 8 lines
mm/hugetlb.c +1 -3
@@ -4189,10 +4189,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
-	} else {
+	} else 
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
-	}

The code is quite puzzling but is this correct? Shouldn't we rather do mm_dec_nr_pmds(mm) in that path to undo the previous inc?

I agree that the code is quite puzzling. :(

However, if this were an issue I would have expected to see some reports. Oracle DB makes use of this feature (shared page tables) and if the pmd count is wrong we would catch it in check_mm() at exit time.

Upon closer examination, I believe the code in question is never executed. Note the callers of huge_pmd_share. The calling code looks like:

                        if (want_pmd_share() && pud_none(*pud))
                                pte = huge_pmd_share(mm, addr, pud);
                        else
                                pte = (pte_t *)pmd_alloc(mm, pud, addr);
Quoted 6 lines
Quoted 2 lines

Therefore, we do not call huge_pmd_share unless pud_none(*pud). The code in question is only executed when !pud_none(*pud).

My understanding is that the check is needed after we retake page lock because we might have raced with other thread. But it's been quite some time since I've looked at hugetlb locking and page table sharing code.

That is correct, we could have raced. Duh!

In the case of a race, the other thread would have incremented the PMD count already. Your suggestion of decrementing pmd count in this case seems to be the correct approach. But, I need to think about this some more.

Signature · 7 lines
Mike Kravetz

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#6 lore raw
Quoted 57 lines · Mike Kravetz
Quoted 49 lines · Michal Hocko
Quoted 48 lines · Mike Kravetz
Quoted 32 lines · Michal Hocko

[It seems that this patch has been sent several times and this particular copy didn't add Kirill who has added this code CC him now]

Quoted 28 lines · Michal Hocko
Quoted 16 lines · zhongjiang

From: zhong jiang <[email protected]>

when a process acquire a pmd table shared by other process, we increase the account to current process. otherwise, a race result in other tasks have set the pud entry. so it no need to increase it.

---
 mm/hugetlb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
mm/hugetlb.c +0 -0
index 19d0d08..3b025c5 100644

mm/hugetlb.c pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud) · line 4195

Quoted 8 lines
mm/hugetlb.c +1 -3
@@ -4189,10 +4189,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
-	} else {
+	} else 
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
-	}

The code is quite puzzling but is this correct? Shouldn't we rather do mm_dec_nr_pmds(mm) in that path to undo the previous inc?

I agree that the code is quite puzzling. :(

However, if this were an issue I would have expected to see some reports. Oracle DB makes use of this feature (shared page tables) and if the pmd count is wrong we would catch it in check_mm() at exit time.

Upon closer examination, I believe the code in question is never executed. Note the callers of huge_pmd_share. The calling code looks like:

                        if (want_pmd_share() && pud_none(*pud))
                                pte = huge_pmd_share(mm, addr, pud);
                        else
                                pte = (pte_t *)pmd_alloc(mm, pud, addr);
Quoted 6 lines
Quoted 2 lines

Therefore, we do not call huge_pmd_share unless pud_none(*pud). The code in question is only executed when !pud_none(*pud).

My understanding is that the check is needed after we retake page lock because we might have raced with other thread. But it's been quite some time since I've looked at hugetlb locking and page table sharing code.

Quoted 6 lines

That is correct, we could have raced. Duh!

In the case of a race, the other thread would have incremented the PMD count already. Your suggestion of decrementing pmd count in this case seems to be the correct approach. But, I need to think about this some more.

Yes, I made mistake by increasing nr_pmds, not descreasing here.

Testcase:

#include <errno.h>
#include <stdio.h>
#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/mman.h>
#include <sys/syscall.h>
#include <sys/time.h>

#define HPGSZ 2097152UL

int main(int argc, char **argv) {
	char *addr;

	system("echo 1024 > /proc/sys/vm/nr_hugepages");
	addr = mmap(NULL, 1024*HPGSZ, PROT_WRITE | PROT_READ,
			MAP_SHARED | MAP_ANONYMOUS | MAP_HUGETLB | MAP_POPULATE, -1, 0);
	if (addr == MAP_FAILED) {
		fprintf(stderr, "Failed to alloc hugepage\n");
		return -1;
	}

	addr[0] = 1;
	fork();
	printf("addr[0]: %d\n", addr[0]);

	sleep(1);
	return 0;
}

You can simulate race by replacing 'if (pud_none(*pud))' with "if (0)". It would produce "BUG: non-zero nr_pmds on freeing mm: 2" on the test-case.

Fix:

Acked-by
#7 lore raw

On Fri 17-06-16 15:25:06, Kirill A. Shutemov wrote:
[...]

Quoted 15 lines
Quoted 1 line

From fd22922e7b4664e83653a84331f0a95b985bff0c Mon Sep 17 00:00:00 2001

From: "Kirill A. Shutemov" <[email protected]>
Date: Fri, 17 Jun 2016 15:07:03 +0300
Subject: [PATCH] hugetlb: fix nr_pmds accounting with shared page tables

We account HugeTLB's shared page table to all processes who share it. The accounting happens during huge_pmd_share().

If somebody populates pud entry under us, we should decrease pagetable's refcount and decrease nr_pmds of the process.

By mistake, I increase nr_pmds again in this case. :-/
It will lead to "BUG: non-zero nr_pmds on freeing mm: 2" on process' exit.

Quoted 7 lines

Let's fix this by increasing nr_pmds only when we're sure that the page table will be used.

Fixes dc6c9a35b66b ("mm: account pmd page tables to the process")
Cc <[email protected]> [4.0+]

Yes this patch is better. Is it worth backporting to stable though? BUG message sounds scary but it is not a real BUG().

Quoted 29 lines
---
 mm/hugetlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
mm/hugetlb.c +1 -2
index e197cd7080e6..ed6a537f0878 100644
@@ -4216,7 +4216,6 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 		if (saddr) {
 			spte = huge_pte_offset(svma->vm_mm, saddr);
 			if (spte) {
-				mm_inc_nr_pmds(mm);
 				get_page(virt_to_page(spte));
 				break;
 			}
@@ -4231,9 +4230,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
+		mm_inc_nr_pmds(mm);
 	} else {
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
 	}
 	spin_unlock(ptl);
 out:
Signature · 1 line
 Kirill A. Shutemov
Signature · 8 lines
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
#8 lore raw
Quoted 20 lines · Michal Hocko

On Fri 17-06-16 15:25:06, Kirill A. Shutemov wrote:
[...]

Quoted 18 lines
Quoted 1 line

From fd22922e7b4664e83653a84331f0a95b985bff0c Mon Sep 17 00:00:00 2001

From: "Kirill A. Shutemov" <[email protected]>
Date: Fri, 17 Jun 2016 15:07:03 +0300
Subject: [PATCH] hugetlb: fix nr_pmds accounting with shared page tables

We account HugeTLB's shared page table to all processes who share it. The accounting happens during huge_pmd_share().

If somebody populates pud entry under us, we should decrease pagetable's refcount and decrease nr_pmds of the process.

By mistake, I increase nr_pmds again in this case. :-/
It will lead to "BUG: non-zero nr_pmds on freeing mm: 2" on process' exit.

Let's fix this by increasing nr_pmds only when we're sure that the page table will be used.

Quoted 7 lines
Quoted 4 lines
Fixes dc6c9a35b66b ("mm: account pmd page tables to the process")
Cc <[email protected]> [4.0+]

Yes this patch is better. Is it worth backporting to stable though? BUG message sounds scary but it is not a real BUG().

I guess, we can live without stable backport.

Quoted 2 lines

Thanks.

Signature · 7 lines
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Reviewed-by
#9 lore raw
In reply to Kirill A. Shutemov
Quoted 12 lines · Kirill A. Shutemov

From fd22922e7b4664e83653a84331f0a95b985bff0c Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <[email protected]>
Date: Fri, 17 Jun 2016 15:07:03 +0300
Subject: [PATCH] hugetlb: fix nr_pmds accounting with shared page tables

We account HugeTLB's shared page table to all processes who share it. The accounting happens during huge_pmd_share().

If somebody populates pud entry under us, we should decrease pagetable's refcount and decrease nr_pmds of the process.

Quoted 8 lines

By mistake, I increase nr_pmds again in this case. :-/
It will lead to "BUG: non-zero nr_pmds on freeing mm: 2" on process' exit.

Let's fix this by increasing nr_pmds only when we're sure that the page table will be used.

Nice,

I agree that we do not necessarily need a back port. I have not seen reports of people experiencing this race and seeing the BUG (on mm tear-down).

zhongjiang, did someone actually hit the BUG? Or, did you find it by code examination?

Signature · 1 line
Mike Kravetz
Quoted 21 lines
Fixes dc6c9a35b66b ("mm: account pmd page tables to the process")
Cc <[email protected]> [4.0+]
---
 mm/hugetlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
mm/hugetlb.c +0 -1
index e197cd7080e6..ed6a537f0878 100644
@@ -4216,7 +4216,6 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 		if (saddr) {
 			spte = huge_pte_offset(svma->vm_mm, saddr);
 			if (spte) {
-				mm_inc_nr_pmds(mm);
 				get_page(virt_to_page(spte));
 				break;
 			}

mm/hugetlb.c pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud) · line 4238

Quoted 10 lines
mm/hugetlb.c +1 -1
@@ -4231,9 +4230,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
+		mm_inc_nr_pmds(mm);
 	} else {
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
 	}
 	spin_unlock(ptl);
 out:

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

#10 lore raw
Quoted 21 lines · Mike Kravetz
Quoted 11 lines · Kirill A. Shutemov

From fd22922e7b4664e83653a84331f0a95b985bff0c Mon Sep 17 00:00:00 2001
From: "Kirill A. Shutemov" <[email protected]>
Date: Fri, 17 Jun 2016 15:07:03 +0300
Subject: [PATCH] hugetlb: fix nr_pmds accounting with shared page tables

We account HugeTLB's shared page table to all processes who share it. The accounting happens during huge_pmd_share().

If somebody populates pud entry under us, we should decrease pagetable's refcount and decrease nr_pmds of the process.

Quoted 8 lines

By mistake, I increase nr_pmds again in this case. :-/
It will lead to "BUG: non-zero nr_pmds on freeing mm: 2" on process' exit.

Let's fix this by increasing nr_pmds only when we're sure that the page table will be used.

Nice,

Quoted 9 lines

I agree that we do not necessarily need a back port. I have not seen reports of people experiencing this race and seeing the BUG (on mm tear-down).

zhongjiang, did someone actually hit the BUG? Or, did you find it by code examination?

just code examination.

#11 lore raw
In reply to Michal Hocko
Quoted 12 lines · Michal Hocko
Quoted 11 lines · zhongjiang

From: zhong jiang <[email protected]>

when a process acquire a pmd table shared by other process, we increase the account to current process. otherwise, a race result in other tasks have set the pud entry. so it no need to increase it.

---
 mm/hugetlb.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
Quoted 15 lines

mm/hugetlb.c pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud) · line 4195

Quoted 13 lines
mm/hugetlb.c +1 -3
index 19d0d08..3b025c5 100644
@@ -4189,10 +4189,9 @@ pte_t *huge_pmd_share(struct mm_struct *mm, unsigned long addr, pud_t *pud)
 	if (pud_none(*pud)) {
 		pud_populate(mm, pud,
 				(pmd_t *)((unsigned long)spte & PAGE_MASK));
-	} else {
+	} else 
 		put_page(virt_to_page(spte));
-		mm_inc_nr_pmds(mm);
-	}

The code is quite puzzling but is this correct? Shouldn't we rather do mm_dec_nr_pmds(mm) in that path to undo the previous inc?

Yes, you are right. I will modify it in V2.

Thanks
zhongjiang

Quoted 7 lines
Quoted 7 lines
mm/hugetlb.c +1 -0
+
 	spin_unlock(ptl);
 out:
 	pte = (pte_t *)pmd_alloc(mm, pud, addr);
Signature · 1 line
1.8.3.1