| Message ID | 20191128010321.21730-1-richardw.yang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] mm/page_vma_mapped: use PMD_SIZE instead of calculating it | expand |
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 <richardw.yang@linux.intel.com> > --- > 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.
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 <richardw.yang@linux.intel.com> >> --- >> 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
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 <richardw.yang@linux.intel.com> > >> --- > >> 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.
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 <richardw.yang@linux.intel.com> >> >> --- >> >> 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
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 <richardw.yang@linux.intel.com> >> >> --- >> >> 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
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 <richardw.yang@linux.intel.com> > >> >> --- > >> >> 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.
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 <richardw.yang@linux.intel.com> >> >> >> --- >> >> >> 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
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) {
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 <richardw.yang@linux.intel.com> --- mm/page_vma_mapped.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)