mm/page_vma_mapped: use PMD_SIZE instead of calculating it

PATCH NAK Wei Yang 4 participants 15 messages Nov 28, 2019 – Dec 3, 2019

Subsystems (2)

Memory management MM rmap

lore

Share & tools

lore

Wei Yang

author

2019-11-28 01:03 UTC #0 lore raw

PATCH 1/2 mm/page_vma_mapped: use PMD_SIZE instead of calculating it

At this point, we are sure page is PageTransHuge, which means hpage_nr_pages is HPAGE_PMD_NR.

This is safe to use PMD_SIZE instead of calculating it.

Signed-off-by Wei Yang <[email protected]>

---
 mm/page_vma_mapped.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

mm/page_vma_mapped.c +1 -1

index eff4b4520c8d..76e03650a3ab 100644
@@ -223,7 +223,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			if (pvmw->address >= pvmw->vma->vm_end ||
 			    pvmw->address >=
 					__vma_address(pvmw->page, pvmw->vma) +
-					hpage_nr_pages(pvmw->page) * PAGE_SIZE)
+					PMD_SIZE)
 				return not_found(pvmw);
 			/* Did we cross page table boundary? */
 			if (pvmw->address % PMD_SIZE == 0) {

Signature · 1 line

2.17.1

Wei Yang

author

2019-11-28 01:03 UTC #1 lore raw

PATCH 2/2 mm/page_vma_mapped: page table boundary is already guaranteed

The check here is to guarantee pvmw->address iteration is limited in one page table boundary. To be specific, here the address range should be in one PMD_SIZE.

If my understanding is correct, this check is already done in the above check:

address >= __vma_address(page, vma) + PMD_SIZE

The boundary check here seems not necessary.

Signed-off-by Wei Yang <[email protected]>

---
Test:
more than 48 hours kernel build test shows this code is not touched.

---
 mm/page_vma_mapped.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

mm/page_vma_mapped.c +1 ~~-12~~

index 76e03650a3ab..25aada8a1271 100644
@@ -163,7 +163,6 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			return not_found(pvmw);
 		return true;
 	}
-restart:
 	pgd = pgd_offset(mm, pvmw->address);
 	if (!pgd_present(*pgd))
 		return false;

@@ -225,17 +224,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 					__vma_address(pvmw->page, pvmw->vma) +
 					PMD_SIZE)
 				return not_found(pvmw);
-			/* Did we cross page table boundary? */
-			if (pvmw->address % PMD_SIZE == 0) {
-				pte_unmap(pvmw->pte);
-				if (pvmw->ptl) {
-					spin_unlock(pvmw->ptl);
-					pvmw->ptl = NULL;
-				}
-				goto restart;
-			} else {
-				pvmw->pte++;
-			}
+			pvmw->pte++;
 		} while (pte_none(*pvmw->pte));
 
 		if (!pvmw->ptl) {

Signature · 1 line

2.17.1

Kirill A. Shutemov

NAK

2019-11-28 08:31 UTC #2 lore raw

Re: [PATCH 2/2] mm/page_vma_mapped: page table boundary is already guaranteed

Quoted 4 lines · Wei Yang

The check here is to guarantee pvmw->address iteration is limited in one page table boundary. To be specific, here the address range should be in one PMD_SIZE.

Quoted 8 lines

If my understanding is correct, this check is already done in the above check:
address >= __vma_address(page, vma) + PMD_SIZE
The boundary check here seems not necessary.
Signed-off-by Wei Yang <[email protected]>

NAK.

THP can be mapped with PTE not aligned to PMD_SIZE. Consider mremap().

Quoted 2 lines

Test:
more than 48 hours kernel build test shows this code is not touched.

Not an argument. I doubt mremap(2) is ever called in kernel build workload.

Signature · 1 line

 Kirill A. Shutemov

Wei Yang

2019-11-28 21:09 UTC #3 lore raw

Re: [PATCH 2/2] mm/page_vma_mapped: page table boundary is already guaranteed

Quoted 10 lines · Kirill A. Shutemov

Quoted 9 lines · Wei Yang
The check here is to guarantee pvmw->address iteration is limited in one page table boundary. To be specific, here the address range should be in one PMD_SIZE.
If my understanding is correct, this check is already done in the above check:
address >= __vma_address(page, vma) + PMD_SIZE

Quoted 8 lines

Quoted 3 lines
The boundary check here seems not necessary.
Signed-off-by Wei Yang <[email protected]>
NAK.
THP can be mapped with PTE not aligned to PMD_SIZE. Consider mremap().

Hi, Kirill

Thanks for your comment during Thanks Giving Day. Happy holiday:-)

I didn't think about this case before, thanks for reminding. Then I tried to understand your concern.

mremap() would expand/shrink a memory mapping. In this case, probably shrink is in concern. Since pvmw->page and pvmw->vma are not changed in the loop, the case you mentioned maybe pvmw->page is the head of a THP but part of it is unmapped.

This means the following condition stands:

    vma->vm_start <= vma_address(page) 
    vma->vm_end <=   vma_address(page) + page_size(page)

Since we have checked address with vm_end, do you think this case is also guarded?

Not sure my understanding is correct, look forward your comments.

Quoted 8 lines

Quoted 2 lines
Test:
more than 48 hours kernel build test shows this code is not touched.
Not an argument. I doubt mremap(2) is ever called in kernel build workload.
Signature · 1 line
Kirill A. Shutemov

Signature · 2 lines

Wei Yang
Help you, Help me

Matthew Wilcox

2019-11-28 22:39 UTC #4 lore raw

Re: [PATCH 2/2] mm/page_vma_mapped: page table boundary is already guaranteed

Quoted 24 lines · Wei Yang

Quoted 10 lines · Kirill A. Shutemov
Quoted 9 lines · Wei Yang
The check here is to guarantee pvmw->address iteration is limited in one page table boundary. To be specific, here the address range should be in one PMD_SIZE.
If my understanding is correct, this check is already done in the above check:
address >= __vma_address(page, vma) + PMD_SIZE
Quoted 8 lines
Quoted 3 lines
The boundary check here seems not necessary.
Signed-off-by Wei Yang <[email protected]>
NAK.
THP can be mapped with PTE not aligned to PMD_SIZE. Consider mremap().
Hi, Kirill
Thanks for your comment during Thanks Giving Day. Happy holiday:-)

Quoted 7 lines

I didn't think about this case before, thanks for reminding. Then I tried to understand your concern.
mremap() would expand/shrink a memory mapping. In this case, probably shrink is in concern. Since pvmw->page and pvmw->vma are not changed in the loop, the case you mentioned maybe pvmw->page is the head of a THP but part of it is unmapped.

mremap() can also move a mapping, see MREMAP_FIXED.

Quoted 23 lines

This means the following condition stands:
    vma->vm_start <= vma_address(page) 
    vma->vm_end <=   vma_address(page) + page_size(page)
Since we have checked address with vm_end, do you think this case is also guarded?
Not sure my understanding is correct, look forward your comments.
Quoted 8 lines
Quoted 2 lines
Test:
more than 48 hours kernel build test shows this code is not touched.
Not an argument. I doubt mremap(2) is ever called in kernel build workload.
Signature · 1 line
Kirill A. Shutemov
Signature · 2 lines
Wei Yang
Help you, Help me

Wei Yang

author

2019-11-29 08:30 UTC #5 lore raw

Re: [PATCH 2/2] mm/page_vma_mapped: page table boundary is already guaranteed

Quoted 28 lines · Matthew Wilcox

Quoted 27 lines · Wei Yang
Quoted 10 lines · Kirill A. Shutemov
Quoted 9 lines · Wei Yang
The check here is to guarantee pvmw->address iteration is limited in one page table boundary. To be specific, here the address range should be in one PMD_SIZE.
If my understanding is correct, this check is already done in the above check:
address >= __vma_address(page, vma) + PMD_SIZE
Quoted 8 lines
Quoted 3 lines
The boundary check here seems not necessary.
Signed-off-by Wei Yang <[email protected]>
NAK.
THP can be mapped with PTE not aligned to PMD_SIZE. Consider mremap().
Hi, Kirill
Thanks for your comment during Thanks Giving Day. Happy holiday:-)
I didn't think about this case before, thanks for reminding. Then I tried to understand your concern.

Quoted 6 lines

Quoted 4 lines
mremap() would expand/shrink a memory mapping. In this case, probably shrink is in concern. Since pvmw->page and pvmw->vma are not changed in the loop, the case you mentioned maybe pvmw->page is the head of a THP but part of it is unmapped.
mremap() can also move a mapping, see MREMAP_FIXED.

Hi, Matthew

Thanks for your comment.

I took a look into the MREMAP_FIXED case, but still not clear in which case it fall into the situation Kirill mentioned.

Per my understanding, move mapping is achieved in two steps:

* unmap some range in old vma if old_len >= new_len
* move vma

If the length doesn't change, we are expecting to have the "copy" of old vma. This doesn't change the THP PMD mapping.

So the change still happens in the unmap step, if I am correct.

Would you mind giving me more hint on the case when we would have the situation as Kirill mentioned?

Quoted 24 lines

Quoted 24 lines
This means the following condition stands:
    vma->vm_start <= vma_address(page) 
    vma->vm_end <=   vma_address(page) + page_size(page)
Since we have checked address with vm_end, do you think this case is also guarded?
Not sure my understanding is correct, look forward your comments.
Quoted 8 lines
Quoted 2 lines
Test:
more than 48 hours kernel build test shows this code is not touched.
Not an argument. I doubt mremap(2) is ever called in kernel build workload.
Signature · 1 line
Kirill A. Shutemov
Signature · 2 lines
Wei Yang
Help you, Help me

Signature · 2 lines

Wei Yang
Help you, Help me

Matthew Wilcox

2019-11-29 11:18 UTC #6 lore raw

Re: [PATCH 2/2] mm/page_vma_mapped: page table boundary is already guaranteed

Quoted 48 lines · Wei Yang

Quoted 28 lines · Matthew Wilcox
Quoted 27 lines · Wei Yang
Quoted 10 lines · Kirill A. Shutemov
Quoted 9 lines · Wei Yang
The check here is to guarantee pvmw->address iteration is limited in one page table boundary. To be specific, here the address range should be in one PMD_SIZE.
If my understanding is correct, this check is already done in the above check:
address >= __vma_address(page, vma) + PMD_SIZE
Quoted 8 lines
Quoted 3 lines
The boundary check here seems not necessary.
Signed-off-by Wei Yang <[email protected]>
NAK.
THP can be mapped with PTE not aligned to PMD_SIZE. Consider mremap().
Hi, Kirill
Thanks for your comment during Thanks Giving Day. Happy holiday:-)
I didn't think about this case before, thanks for reminding. Then I tried to understand your concern.
Quoted 6 lines
Quoted 4 lines
mremap() would expand/shrink a memory mapping. In this case, probably shrink is in concern. Since pvmw->page and pvmw->vma are not changed in the loop, the case you mentioned maybe pvmw->page is the head of a THP but part of it is unmapped.
mremap() can also move a mapping, see MREMAP_FIXED.
Hi, Matthew
Thanks for your comment.
I took a look into the MREMAP_FIXED case, but still not clear in which case it fall into the situation Kirill mentioned.
Per my understanding, move mapping is achieved in two steps:
* unmap some range in old vma if old_len >= new_len
* move vma

Quoted 7 lines

If the length doesn't change, we are expecting to have the "copy" of old vma. This doesn't change the THP PMD mapping.
So the change still happens in the unmap step, if I am correct.
Would you mind giving me more hint on the case when we would have the situation as Kirill mentioned?

Set up a THP mapping.
Move it to an address which is no longer 2MB aligned. Unmap it.

Wei Yang

author

2019-12-02 06:53 UTC #7 lore raw

Re: [PATCH 2/2] mm/page_vma_mapped: page table boundary is already guaranteed

Quoted 52 lines · Matthew Wilcox

Quoted 51 lines · Wei Yang
Quoted 28 lines · Matthew Wilcox
Quoted 27 lines · Wei Yang
Quoted 10 lines · Kirill A. Shutemov
Quoted 9 lines · Wei Yang
The check here is to guarantee pvmw->address iteration is limited in one page table boundary. To be specific, here the address range should be in one PMD_SIZE.
If my understanding is correct, this check is already done in the above check:
address >= __vma_address(page, vma) + PMD_SIZE
Quoted 8 lines
Quoted 3 lines
The boundary check here seems not necessary.
Signed-off-by Wei Yang <[email protected]>
NAK.
THP can be mapped with PTE not aligned to PMD_SIZE. Consider mremap().
Hi, Kirill
Thanks for your comment during Thanks Giving Day. Happy holiday:-)
I didn't think about this case before, thanks for reminding. Then I tried to understand your concern.
Quoted 6 lines
Quoted 4 lines
mremap() would expand/shrink a memory mapping. In this case, probably shrink is in concern. Since pvmw->page and pvmw->vma are not changed in the loop, the case you mentioned maybe pvmw->page is the head of a THP but part of it is unmapped.
mremap() can also move a mapping, see MREMAP_FIXED.
Hi, Matthew
Thanks for your comment.
I took a look into the MREMAP_FIXED case, but still not clear in which case it fall into the situation Kirill mentioned.
Per my understanding, move mapping is achieved in two steps:
* unmap some range in old vma if old_len >= new_len
* move vma
If the length doesn't change, we are expecting to have the "copy" of old vma. This doesn't change the THP PMD mapping.

Quoted 8 lines

Quoted 4 lines
So the change still happens in the unmap step, if I am correct.
Would you mind giving me more hint on the case when we would have the situation as Kirill mentioned?
Set up a THP mapping.
Move it to an address which is no longer 2MB aligned. Unmap it.

Thanks Matthew

I got the point, thanks a lot :-)

Signature · 2 lines

Wei Yang
Help you, Help me

Kirill A. Shutemov

2019-11-28 08:33 UTC #8 lore raw

In reply to Wei Yang

Quoted 15 lines · Wei Yang

At this point, we are sure page is PageTransHuge, which means hpage_nr_pages is HPAGE_PMD_NR.
This is safe to use PMD_SIZE instead of calculating it.
Signed-off-by Wei Yang <[email protected]>
---
 mm/page_vma_mapped.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/page_vma_mapped.c +0 -0
index eff4b4520c8d..76e03650a3ab 100644

mm/page_vma_mapped.c bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) · line 229

Quoted 8 lines

mm/page_vma_mapped.c +1 -1

@@ -223,7 +223,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			if (pvmw->address >= pvmw->vma->vm_end ||
 			    pvmw->address >=
 					__vma_address(pvmw->page, pvmw->vma) +
-					hpage_nr_pages(pvmw->page) * PAGE_SIZE)
+					PMD_SIZE)
 				return not_found(pvmw);
 			/* Did we cross page table boundary? */
 			if (pvmw->address % PMD_SIZE == 0) {

It is dubious cleanup. Maybe page_size(pvmw->page) instead? It will not break if we ever get PUD THP pages.

Signature · 1 line

 Kirill A. Shutemov

Wei Yang

2019-11-28 21:22 UTC #9 lore raw

Quoted 11 lines · Kirill A. Shutemov

Quoted 10 lines · Wei Yang
At this point, we are sure page is PageTransHuge, which means hpage_nr_pages is HPAGE_PMD_NR.
This is safe to use PMD_SIZE instead of calculating it.
Signed-off-by Wei Yang <[email protected]>
---
 mm/page_vma_mapped.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Quoted 17 lines

mm/page_vma_mapped.c bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) · line 229
Quoted 13 lines
mm/page_vma_mapped.c +1 -1
index eff4b4520c8d..76e03650a3ab 100644
@@ -223,7 +223,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			if (pvmw->address >= pvmw->vma->vm_end ||
 			    pvmw->address >=
 					__vma_address(pvmw->page, pvmw->vma) +
-					hpage_nr_pages(pvmw->page) * PAGE_SIZE)
+					PMD_SIZE)
 				return not_found(pvmw);
 			/* Did we cross page table boundary? */
 			if (pvmw->address % PMD_SIZE == 0) {
It is dubious cleanup. Maybe page_size(pvmw->page) instead? It will not break if we ever get PUD THP pages.

Thanks for your comment.

I took a look into the code again and found I may miss something.

I found we support PUD THP pages, whilc hpage_nr_pages() just return HPAGE_PMD_NR on PageTransHuge. Why this is not possible to return PUD number?

Search in the kernel tree, one implementation of PUD_SIZE fault is dev_dax_huge_fault. If page fault happens here, would this if break the loop too early?

Quoted 2 lines

Signature · 1 line
Kirill A. Shutemov

Signature · 2 lines

Wei Yang
Help you, Help me

Kirill A. Shutemov

2019-12-02 08:03 UTC #10 lore raw

Quoted 30 lines · Wei Yang

Quoted 11 lines · Kirill A. Shutemov
Quoted 10 lines · Wei Yang
At this point, we are sure page is PageTransHuge, which means hpage_nr_pages is HPAGE_PMD_NR.
This is safe to use PMD_SIZE instead of calculating it.
Signed-off-by Wei Yang <[email protected]>
---
 mm/page_vma_mapped.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 17 lines
mm/page_vma_mapped.c bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) · line 229
Quoted 13 lines
mm/page_vma_mapped.c +1 -1
index eff4b4520c8d..76e03650a3ab 100644
@@ -223,7 +223,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			if (pvmw->address >= pvmw->vma->vm_end ||
 			    pvmw->address >=
 					__vma_address(pvmw->page, pvmw->vma) +
-					hpage_nr_pages(pvmw->page) * PAGE_SIZE)
+					PMD_SIZE)
 				return not_found(pvmw);
 			/* Did we cross page table boundary? */
 			if (pvmw->address % PMD_SIZE == 0) {
It is dubious cleanup. Maybe page_size(pvmw->page) instead? It will not break if we ever get PUD THP pages.

Quoted 6 lines

Thanks for your comment.
I took a look into the code again and found I may miss something.
I found we support PUD THP pages, whilc hpage_nr_pages() just return HPAGE_PMD_NR on PageTransHuge. Why this is not possible to return PUD number?

We only support PUD THP for DAX. Means, we don't have struct page for it.

Signature · 1 line

 Kirill A. Shutemov

Wei Yang

author

2019-12-02 08:54 UTC #11 lore raw

Quoted 31 lines · Kirill A. Shutemov

Quoted 30 lines · Wei Yang
Quoted 28 lines · Kirill A. Shutemov
Quoted 15 lines · Wei Yang
At this point, we are sure page is PageTransHuge, which means hpage_nr_pages is HPAGE_PMD_NR.
This is safe to use PMD_SIZE instead of calculating it.
Signed-off-by Wei Yang <[email protected]>
---
 mm/page_vma_mapped.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/page_vma_mapped.c +0 -0
index eff4b4520c8d..76e03650a3ab 100644
mm/page_vma_mapped.c bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) · line 229
Quoted 8 lines
mm/page_vma_mapped.c +1 -1
@@ -223,7 +223,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			if (pvmw->address >= pvmw->vma->vm_end ||
 			    pvmw->address >=
 					__vma_address(pvmw->page, pvmw->vma) +
-					hpage_nr_pages(pvmw->page) * PAGE_SIZE)
+					PMD_SIZE)
 				return not_found(pvmw);
 			/* Did we cross page table boundary? */
 			if (pvmw->address % PMD_SIZE == 0) {
It is dubious cleanup. Maybe page_size(pvmw->page) instead? It will not break if we ever get PUD THP pages.

Quoted 9 lines

Quoted 6 lines
Thanks for your comment.
I took a look into the code again and found I may miss something.
I found we support PUD THP pages, whilc hpage_nr_pages() just return HPAGE_PMD_NR on PageTransHuge. Why this is not possible to return PUD number?
We only support PUD THP for DAX. Means, we don't have struct page for it.

Ok, many background story behind it.

Thanks :-)

Quoted 2 lines

Signature · 1 line
Kirill A. Shutemov

Signature · 2 lines

Wei Yang
Help you, Help me

Wei Yang

2019-12-02 22:21 UTC #12 lore raw

In reply to Kirill A. Shutemov

Quoted 31 lines · Kirill A. Shutemov

Quoted 30 lines · Wei Yang
Quoted 28 lines · Kirill A. Shutemov
Quoted 15 lines · Wei Yang
At this point, we are sure page is PageTransHuge, which means hpage_nr_pages is HPAGE_PMD_NR.
This is safe to use PMD_SIZE instead of calculating it.
Signed-off-by Wei Yang <[email protected]>
---
 mm/page_vma_mapped.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/page_vma_mapped.c +0 -0
index eff4b4520c8d..76e03650a3ab 100644
mm/page_vma_mapped.c bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) · line 229
Quoted 8 lines
mm/page_vma_mapped.c +1 -1
@@ -223,7 +223,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			if (pvmw->address >= pvmw->vma->vm_end ||
 			    pvmw->address >=
 					__vma_address(pvmw->page, pvmw->vma) +
-					hpage_nr_pages(pvmw->page) * PAGE_SIZE)
+					PMD_SIZE)
 				return not_found(pvmw);
 			/* Did we cross page table boundary? */
 			if (pvmw->address % PMD_SIZE == 0) {
It is dubious cleanup. Maybe page_size(pvmw->page) instead? It will not break if we ever get PUD THP pages.

Quoted 9 lines

Quoted 6 lines
Thanks for your comment.
I took a look into the code again and found I may miss something.
I found we support PUD THP pages, whilc hpage_nr_pages() just return HPAGE_PMD_NR on PageTransHuge. Why this is not possible to return PUD number?
We only support PUD THP for DAX. Means, we don't have struct page for it.

BTW, may I ask why you suggest to use page_size() if we are sure only PMD THP page is used here? To be more generic?

Quoted 2 lines

Signature · 1 line
Kirill A. Shutemov

Signature · 2 lines

Wei Yang
Help you, Help me

Kirill A. Shutemov

2019-12-03 09:47 UTC #13 lore raw

Quoted 36 lines · Wei Yang

Quoted 35 lines · Kirill A. Shutemov
Quoted 34 lines · Wei Yang
Quoted 11 lines · Kirill A. Shutemov
Quoted 10 lines · Wei Yang
At this point, we are sure page is PageTransHuge, which means hpage_nr_pages is HPAGE_PMD_NR.
This is safe to use PMD_SIZE instead of calculating it.
Signed-off-by Wei Yang <[email protected]>
---
 mm/page_vma_mapped.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 17 lines
mm/page_vma_mapped.c bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) · line 229
Quoted 13 lines
mm/page_vma_mapped.c +1 -1
index eff4b4520c8d..76e03650a3ab 100644
@@ -223,7 +223,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			if (pvmw->address >= pvmw->vma->vm_end ||
 			    pvmw->address >=
 					__vma_address(pvmw->page, pvmw->vma) +
-					hpage_nr_pages(pvmw->page) * PAGE_SIZE)
+					PMD_SIZE)
 				return not_found(pvmw);
 			/* Did we cross page table boundary? */
 			if (pvmw->address % PMD_SIZE == 0) {
It is dubious cleanup. Maybe page_size(pvmw->page) instead? It will not break if we ever get PUD THP pages.
Thanks for your comment.
I took a look into the code again and found I may miss something.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
I found we support PUD THP pages, whilc hpage_nr_pages() just return HPAGE_PMD_NR on PageTransHuge. Why this is not possible to return PUD number?
We only support PUD THP for DAX. Means, we don't have struct page for it.
BTW, may I ask why you suggest to use page_size() if we are sure only PMD THP page is used here? To be more generic?

Yeah. I would rather not touch it at all.

Signature · 1 line

 Kirill A. Shutemov

Wei Yang

2019-12-03 15:14 UTC #14 lore raw

Quoted 40 lines · Kirill A. Shutemov

Quoted 39 lines · Wei Yang
Quoted 38 lines · Kirill A. Shutemov
Quoted 36 lines · Wei Yang
Quoted 11 lines · Kirill A. Shutemov
Quoted 10 lines · Wei Yang
At this point, we are sure page is PageTransHuge, which means hpage_nr_pages is HPAGE_PMD_NR.
This is safe to use PMD_SIZE instead of calculating it.
Signed-off-by Wei Yang <[email protected]>
---
 mm/page_vma_mapped.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 17 lines
mm/page_vma_mapped.c bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw) · line 229
Quoted 13 lines
mm/page_vma_mapped.c +1 -1
index eff4b4520c8d..76e03650a3ab 100644
@@ -223,7 +223,7 @@ bool page_vma_mapped_walk(struct page_vma_mapped_walk *pvmw)
 			if (pvmw->address >= pvmw->vma->vm_end ||
 			    pvmw->address >=
 					__vma_address(pvmw->page, pvmw->vma) +
-					hpage_nr_pages(pvmw->page) * PAGE_SIZE)
+					PMD_SIZE)
 				return not_found(pvmw);
 			/* Did we cross page table boundary? */
 			if (pvmw->address % PMD_SIZE == 0) {
It is dubious cleanup. Maybe page_size(pvmw->page) instead? It will not break if we ever get PUD THP pages.
Thanks for your comment.
I took a look into the code again and found I may miss something.
I found we support PUD THP pages, whilc hpage_nr_pages() just return HPAGE_PMD_NR on PageTransHuge. Why this is not possible to return PUD number?

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
We only support PUD THP for DAX. Means, we don't have struct page for it.
BTW, may I ask why you suggest to use page_size() if we are sure only PMD THP page is used here? To be more generic?
Yeah. I would rather not touch it at all.

Got it, thanks.

Quoted 2 lines

Signature · 1 line
Kirill A. Shutemov

Signature · 2 lines

Wei Yang
Help you, Help me