2019-11-28 01:06:49

by Wei Yang

Subject: [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(-)

diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
index eff4b4520c8d..76e03650a3ab 100644
--- a/mm/page_vma_mapped.c
+++ b/mm/page_vma_mapped.c
@@ -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) {
--
2.17.1

2019-11-28 08:35:19

by Kirill A. Shutemov

Subject: Re: [PATCH 1/2] mm/page_vma_mapped: use PMD_SIZE instead of calculating it

On Thu, Nov 28, 2019 at 09:03:20AM +0800, Wei Yang wrote:
> 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(-)
>
> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> index eff4b4520c8d..76e03650a3ab 100644
> --- a/mm/page_vma_mapped.c
> +++ b/mm/page_vma_mapped.c
> @@ -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.

--
Kirill A. Shutemov

2019-11-28 21:24:05

by Wei Yang

Subject: Re: [PATCH 1/2] mm/page_vma_mapped: use PMD_SIZE instead of calculating it

On Thu, Nov 28, 2019 at 11:32:55AM +0300, Kirill A. Shutemov wrote:
>On Thu, Nov 28, 2019 at 09:03:20AM +0800, Wei Yang wrote:
>> 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(-)
>>
>> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>> index eff4b4520c8d..76e03650a3ab 100644
>> --- a/mm/page_vma_mapped.c
>> +++ b/mm/page_vma_mapped.c
>> @@ -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?

>--
> Kirill A. Shutemov

--
Wei Yang
Help you, Help me

2019-12-02 08:07:15

by Kirill A. Shutemov

Subject: Re: [PATCH 1/2] mm/page_vma_mapped: use PMD_SIZE instead of calculating it

On Thu, Nov 28, 2019 at 09:22:26PM +0000, Wei Yang wrote:
> On Thu, Nov 28, 2019 at 11:32:55AM +0300, Kirill A. Shutemov wrote:
> >On Thu, Nov 28, 2019 at 09:03:20AM +0800, Wei Yang wrote:
> >> 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(-)
> >>
> >> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> >> index eff4b4520c8d..76e03650a3ab 100644
> >> --- a/mm/page_vma_mapped.c
> >> +++ b/mm/page_vma_mapped.c
> >> @@ -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?

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

--
Kirill A. Shutemov

2019-12-02 08:56:03

by Wei Yang

Subject: Re: [PATCH 1/2] mm/page_vma_mapped: use PMD_SIZE instead of calculating it

On Mon, Dec 02, 2019 at 11:03:15AM +0300, Kirill A. Shutemov wrote:
>On Thu, Nov 28, 2019 at 09:22:26PM +0000, Wei Yang wrote:
>> On Thu, Nov 28, 2019 at 11:32:55AM +0300, Kirill A. Shutemov wrote:
>> >On Thu, Nov 28, 2019 at 09:03:20AM +0800, Wei Yang wrote:
>> >> 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(-)
>> >>
>> >> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>> >> index eff4b4520c8d..76e03650a3ab 100644
>> >> --- a/mm/page_vma_mapped.c
>> >> +++ b/mm/page_vma_mapped.c
>> >> @@ -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?
>
>We only support PUD THP for DAX. Means, we don't have struct page for it.
>

Ok, many background story behind it.

Thanks :-)

>--
> Kirill A. Shutemov

--
Wei Yang
Help you, Help me

2019-12-02 22:25:30

by Wei Yang

Subject: Re: [PATCH 1/2] mm/page_vma_mapped: use PMD_SIZE instead of calculating it

On Mon, Dec 02, 2019 at 11:03:15AM +0300, Kirill A. Shutemov wrote:
>On Thu, Nov 28, 2019 at 09:22:26PM +0000, Wei Yang wrote:
>> On Thu, Nov 28, 2019 at 11:32:55AM +0300, Kirill A. Shutemov wrote:
>> >On Thu, Nov 28, 2019 at 09:03:20AM +0800, Wei Yang wrote:
>> >> 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(-)
>> >>
>> >> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>> >> index eff4b4520c8d..76e03650a3ab 100644
>> >> --- a/mm/page_vma_mapped.c
>> >> +++ b/mm/page_vma_mapped.c
>> >> @@ -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?
>
>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?

>--
> Kirill A. Shutemov

--
Wei Yang
Help you, Help me

2019-12-03 09:48:22

by Kirill A. Shutemov

Subject: Re: [PATCH 1/2] mm/page_vma_mapped: use PMD_SIZE instead of calculating it

On Mon, Dec 02, 2019 at 10:21:51PM +0000, Wei Yang wrote:
> On Mon, Dec 02, 2019 at 11:03:15AM +0300, Kirill A. Shutemov wrote:
> >On Thu, Nov 28, 2019 at 09:22:26PM +0000, Wei Yang wrote:
> >> On Thu, Nov 28, 2019 at 11:32:55AM +0300, Kirill A. Shutemov wrote:
> >> >On Thu, Nov 28, 2019 at 09:03:20AM +0800, Wei Yang wrote:
> >> >> 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(-)
> >> >>
> >> >> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
> >> >> index eff4b4520c8d..76e03650a3ab 100644
> >> >> --- a/mm/page_vma_mapped.c
> >> >> +++ b/mm/page_vma_mapped.c
> >> >> @@ -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?
> >
> >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.

--
Kirill A. Shutemov

2019-12-03 15:16:14

by Wei Yang

Subject: Re: [PATCH 1/2] mm/page_vma_mapped: use PMD_SIZE instead of calculating it

On Tue, Dec 03, 2019 at 12:47:30PM +0300, Kirill A. Shutemov wrote:
>On Mon, Dec 02, 2019 at 10:21:51PM +0000, Wei Yang wrote:
>> On Mon, Dec 02, 2019 at 11:03:15AM +0300, Kirill A. Shutemov wrote:
>> >On Thu, Nov 28, 2019 at 09:22:26PM +0000, Wei Yang wrote:
>> >> On Thu, Nov 28, 2019 at 11:32:55AM +0300, Kirill A. Shutemov wrote:
>> >> >On Thu, Nov 28, 2019 at 09:03:20AM +0800, Wei Yang wrote:
>> >> >> 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(-)
>> >> >>
>> >> >> diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c
>> >> >> index eff4b4520c8d..76e03650a3ab 100644
>> >> >> --- a/mm/page_vma_mapped.c
>> >> >> +++ b/mm/page_vma_mapped.c
>> >> >> @@ -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?
>> >
>> >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.

>--
> Kirill A. Shutemov

--
Wei Yang
Help you, Help me