There are duplicated place using release_pte_pages().
And release_all_pte_pages() can also be removed.
Signed-off-by: Bob Liu <[email protected]>
---
mm/huge_memory.c | 37 +++++++++++--------------------------
1 file changed, 11 insertions(+), 26 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index a863af2..462d6ea 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1700,64 +1700,49 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte)
}
}
-static void release_all_pte_pages(pte_t *pte)
-{
- release_pte_pages(pte, pte + HPAGE_PMD_NR);
-}
-
static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
unsigned long address,
pte_t *pte)
{
struct page *page;
pte_t *_pte;
- int referenced = 0, isolated = 0, none = 0;
+ int referenced = 0, isolated = 1, none = 0;
for (_pte = pte; _pte < pte+HPAGE_PMD_NR;
_pte++, address += PAGE_SIZE) {
pte_t pteval = *_pte;
if (pte_none(pteval)) {
if (++none <= khugepaged_max_ptes_none)
continue;
- else {
- release_pte_pages(pte, _pte);
+ else
goto out;
- }
}
- if (!pte_present(pteval) || !pte_write(pteval)) {
- release_pte_pages(pte, _pte);
+ if (!pte_present(pteval) || !pte_write(pteval))
goto out;
- }
page = vm_normal_page(vma, address, pteval);
- if (unlikely(!page)) {
- release_pte_pages(pte, _pte);
+ if (unlikely(!page))
goto out;
- }
+
VM_BUG_ON(PageCompound(page));
BUG_ON(!PageAnon(page));
VM_BUG_ON(!PageSwapBacked(page));
/* cannot use mapcount: can't collapse if there's a gup pin */
- if (page_count(page) != 1) {
- release_pte_pages(pte, _pte);
+ if (page_count(page) != 1)
goto out;
- }
/*
* We can do it before isolate_lru_page because the
* page can't be freed from under us. NOTE: PG_lock
* is needed to serialize against split_huge_page
* when invoked from the VM.
*/
- if (!trylock_page(page)) {
- release_pte_pages(pte, _pte);
+ if (!trylock_page(page))
goto out;
- }
/*
* Isolate the page to avoid collapsing an hugepage
* currently in use by the VM.
*/
if (isolate_lru_page(page)) {
unlock_page(page);
- release_pte_pages(pte, _pte);
goto out;
}
/* 0 stands for page_is_file_cache(page) == false */
@@ -1770,11 +1755,11 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
mmu_notifier_test_young(vma->vm_mm, address))
referenced = 1;
}
- if (unlikely(!referenced))
- release_all_pte_pages(pte);
- else
- isolated = 1;
+ if (unlikely(!referenced)) {
out:
+ release_pte_pages(pte, _pte);
+ isolated = 0;
+ }
return isolated;
}
--
1.7.9.5
Introduce hugepage_get_pmd() to simple code.
Signed-off-by: Bob Liu <[email protected]>
---
mm/huge_memory.c | 68 ++++++++++++++++++++++--------------------------------
1 file changed, 27 insertions(+), 41 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 462d6ea..e575b29 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1115,6 +1115,25 @@ int change_huge_pmd(struct vm_area_struct *vma, pmd_t *pmd,
return ret;
}
+static pmd_t *hugepage_get_pmd(struct mm_struct *mm, unsigned long address)
+{
+ pgd_t *pgd;
+ pud_t *pud;
+ pmd_t *pmd = NULL;
+
+ pgd = pgd_offset(mm, address);
+ if (!pgd_present(*pgd))
+ goto out;
+
+ pud = pud_offset(pgd, address);
+ if (!pud_present(*pud))
+ goto out;
+
+ pmd = pmd_offset(pud, address);
+out:
+ return pmd;
+}
+
/*
* Returns 1 if a given pmd maps a stable (not under splitting) thp.
* Returns -1 if it maps a thp under splitting. Returns 0 otherwise.
@@ -1145,22 +1164,14 @@ pmd_t *page_check_address_pmd(struct page *page,
unsigned long address,
enum page_check_address_pmd_flag flag)
{
- pgd_t *pgd;
- pud_t *pud;
pmd_t *pmd, *ret = NULL;
if (address & ~HPAGE_PMD_MASK)
goto out;
- pgd = pgd_offset(mm, address);
- if (!pgd_present(*pgd))
- goto out;
-
- pud = pud_offset(pgd, address);
- if (!pud_present(*pud))
+ pmd = hugepage_get_pmd(mm, address);
+ if (!pmd)
goto out;
-
- pmd = pmd_offset(pud, address);
if (pmd_none(*pmd))
goto out;
if (pmd_page(*pmd) != page)
@@ -1908,8 +1919,6 @@ static void collapse_huge_page(struct mm_struct *mm,
struct vm_area_struct *vma,
int node)
{
- pgd_t *pgd;
- pud_t *pud;
pmd_t *pmd, _pmd;
pte_t *pte;
pgtable_t pgtable;
@@ -1955,16 +1964,9 @@ static void collapse_huge_page(struct mm_struct *mm,
goto out;
VM_BUG_ON(vma->vm_flags & VM_NO_THP);
- pgd = pgd_offset(mm, address);
- if (!pgd_present(*pgd))
+ pmd = hugepage_get_pmd(mm, address);
+ if (!pmd)
goto out;
-
- pud = pud_offset(pgd, address);
- if (!pud_present(*pud))
- goto out;
-
- pmd = pmd_offset(pud, address);
- /* pmd can't go away or become huge under us */
if (!pmd_present(*pmd) || pmd_trans_huge(*pmd))
goto out;
@@ -2048,8 +2050,6 @@ static int khugepaged_scan_pmd(struct mm_struct *mm,
unsigned long address,
struct page **hpage)
{
- pgd_t *pgd;
- pud_t *pud;
pmd_t *pmd;
pte_t *pte, *_pte;
int ret = 0, referenced = 0, none = 0;
@@ -2060,15 +2060,9 @@ static int khugepaged_scan_pmd(struct mm_struct *mm,
VM_BUG_ON(address & ~HPAGE_PMD_MASK);
- pgd = pgd_offset(mm, address);
- if (!pgd_present(*pgd))
- goto out;
-
- pud = pud_offset(pgd, address);
- if (!pud_present(*pud))
+ pmd = hugepage_get_pmd(mm, address);
+ if (!pmd)
goto out;
-
- pmd = pmd_offset(pud, address);
if (!pmd_present(*pmd) || pmd_trans_huge(*pmd))
goto out;
@@ -2363,21 +2357,13 @@ void __split_huge_page_pmd(struct mm_struct *mm, pmd_t *pmd)
static void split_huge_page_address(struct mm_struct *mm,
unsigned long address)
{
- pgd_t *pgd;
- pud_t *pud;
pmd_t *pmd;
VM_BUG_ON(!(address & ~HPAGE_PMD_MASK));
- pgd = pgd_offset(mm, address);
- if (!pgd_present(*pgd))
- return;
-
- pud = pud_offset(pgd, address);
- if (!pud_present(*pud))
+ pmd = hugepage_get_pmd(mm, address);
+ if (!pmd)
return;
-
- pmd = pmd_offset(pud, address);
if (!pmd_present(*pmd))
return;
/*
--
1.7.9.5
Multi place do the same check.
Signed-off-by: Bob Liu <[email protected]>
---
mm/huge_memory.c | 38 +++++++++++++++++---------------------
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index e575b29..3588fec 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1913,6 +1913,20 @@ static struct page
}
#endif
+static bool hugepage_vma_check(struct vm_area_struct *vma)
+{
+ if ((!(vma->vm_flags & VM_HUGEPAGE) && !khugepaged_always()) ||
+ (vma->vm_flags & VM_NOHUGEPAGE))
+ return false;
+
+ if (!vma->anon_vma || vma->vm_ops)
+ return false;
+ if (is_vma_temporary_stack(vma))
+ return false;
+ VM_BUG_ON(vma->vm_flags & VM_NO_THP);
+ return true;
+}
+
static void collapse_huge_page(struct mm_struct *mm,
unsigned long address,
struct page **hpage,
@@ -1953,17 +1967,8 @@ static void collapse_huge_page(struct mm_struct *mm,
hend = vma->vm_end & HPAGE_PMD_MASK;
if (address < hstart || address + HPAGE_PMD_SIZE > hend)
goto out;
-
- if ((!(vma->vm_flags & VM_HUGEPAGE) && !khugepaged_always()) ||
- (vma->vm_flags & VM_NOHUGEPAGE))
- goto out;
-
- if (!vma->anon_vma || vma->vm_ops)
- goto out;
- if (is_vma_temporary_stack(vma))
+ if (!hugepage_vma_check(vma))
goto out;
- VM_BUG_ON(vma->vm_flags & VM_NO_THP);
-
pmd = hugepage_get_pmd(mm, address);
if (!pmd)
goto out;
@@ -2171,20 +2176,11 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages,
progress++;
break;
}
-
- if ((!(vma->vm_flags & VM_HUGEPAGE) &&
- !khugepaged_always()) ||
- (vma->vm_flags & VM_NOHUGEPAGE)) {
- skip:
+ if (!hugepage_vma_check(vma)) {
+skip:
progress++;
continue;
}
- if (!vma->anon_vma || vma->vm_ops)
- goto skip;
- if (is_vma_temporary_stack(vma))
- goto skip;
- VM_BUG_ON(vma->vm_flags & VM_NO_THP);
-
hstart = (vma->vm_start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK;
hend = vma->vm_end & HPAGE_PMD_MASK;
if (hstart >= hend)
--
1.7.9.5
Introduce mk_huge_pmd() to simple code
Signed-off-by: Bob Liu <[email protected]>
---
mm/huge_memory.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 3588fec..9fd1312 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -605,6 +605,15 @@ static inline pmd_t maybe_pmd_mkwrite(pmd_t pmd, struct vm_area_struct *vma)
return pmd;
}
+static inline pmd_t mk_huge_pmd(struct page *page, struct vm_area_struct *vma)
+{
+ pmd_t entry;
+ entry = mk_pmd(page, vma->vm_page_prot);
+ entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
+ entry = pmd_mkhuge(entry);
+ return entry;
+}
+
static int __do_huge_pmd_anonymous_page(struct mm_struct *mm,
struct vm_area_struct *vma,
unsigned long haddr, pmd_t *pmd,
@@ -628,9 +637,7 @@ static int __do_huge_pmd_anonymous_page(struct mm_struct *mm,
pte_free(mm, pgtable);
} else {
pmd_t entry;
- entry = mk_pmd(page, vma->vm_page_prot);
- entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
- entry = pmd_mkhuge(entry);
+ entry = mk_huge_pmd(page, vma);
/*
* The spinlocking to take the lru_lock inside
* page_add_new_anon_rmap() acts as a full memory
@@ -950,9 +957,7 @@ int do_huge_pmd_wp_page(struct mm_struct *mm, struct vm_area_struct *vma,
} else {
pmd_t entry;
VM_BUG_ON(!PageHead(page));
- entry = mk_pmd(new_page, vma->vm_page_prot);
- entry = maybe_pmd_mkwrite(pmd_mkdirty(entry), vma);
- entry = pmd_mkhuge(entry);
+ entry = mk_huge_pmd(new_page, vma);
pmdp_clear_flush(vma, haddr, pmd);
page_add_new_anon_rmap(new_page, vma, haddr);
set_pmd_at(mm, haddr, pmd, entry);
@@ -2019,9 +2024,7 @@ static void collapse_huge_page(struct mm_struct *mm,
__SetPageUptodate(new_page);
pgtable = pmd_pgtable(_pmd);
- _pmd = mk_pmd(new_page, vma->vm_page_prot);
- _pmd = maybe_pmd_mkwrite(pmd_mkdirty(_pmd), vma);
- _pmd = pmd_mkhuge(_pmd);
+ _pmd = mk_huge_pmd(new_page, vma);
/*
* spin_lock() below is not the equivalent of smp_wmb(), so
--
1.7.9.5
On Thu, 18 Oct 2012, Bob Liu wrote:
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index a863af2..462d6ea 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1700,64 +1700,49 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte)
> }
> }
>
> -static void release_all_pte_pages(pte_t *pte)
> -{
> - release_pte_pages(pte, pte + HPAGE_PMD_NR);
> -}
> -
> static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> unsigned long address,
> pte_t *pte)
> {
> struct page *page;
> pte_t *_pte;
> - int referenced = 0, isolated = 0, none = 0;
> + int referenced = 0, isolated = 1, none = 0;
> for (_pte = pte; _pte < pte+HPAGE_PMD_NR;
> _pte++, address += PAGE_SIZE) {
> pte_t pteval = *_pte;
> if (pte_none(pteval)) {
> if (++none <= khugepaged_max_ptes_none)
> continue;
> - else {
> - release_pte_pages(pte, _pte);
> + else
> goto out;
> - }
> }
> - if (!pte_present(pteval) || !pte_write(pteval)) {
> - release_pte_pages(pte, _pte);
> + if (!pte_present(pteval) || !pte_write(pteval))
> goto out;
> - }
> page = vm_normal_page(vma, address, pteval);
> - if (unlikely(!page)) {
> - release_pte_pages(pte, _pte);
> + if (unlikely(!page))
> goto out;
> - }
> +
> VM_BUG_ON(PageCompound(page));
> BUG_ON(!PageAnon(page));
> VM_BUG_ON(!PageSwapBacked(page));
>
> /* cannot use mapcount: can't collapse if there's a gup pin */
> - if (page_count(page) != 1) {
> - release_pte_pages(pte, _pte);
> + if (page_count(page) != 1)
> goto out;
> - }
> /*
> * We can do it before isolate_lru_page because the
> * page can't be freed from under us. NOTE: PG_lock
> * is needed to serialize against split_huge_page
> * when invoked from the VM.
> */
> - if (!trylock_page(page)) {
> - release_pte_pages(pte, _pte);
> + if (!trylock_page(page))
> goto out;
> - }
> /*
> * Isolate the page to avoid collapsing an hugepage
> * currently in use by the VM.
> */
> if (isolate_lru_page(page)) {
> unlock_page(page);
> - release_pte_pages(pte, _pte);
> goto out;
> }
> /* 0 stands for page_is_file_cache(page) == false */
> @@ -1770,11 +1755,11 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> mmu_notifier_test_young(vma->vm_mm, address))
> referenced = 1;
> }
> - if (unlikely(!referenced))
> - release_all_pte_pages(pte);
> - else
> - isolated = 1;
> + if (unlikely(!referenced)) {
> out:
Labels inside of conditionals are never good if they can be avoided and in
this case you can avoid it by doing
if (likely(referenced))
return 1;
out:
...
> + release_pte_pages(pte, _pte);
> + isolated = 0;
> + }
> return isolated;
> }
>
On Thu, 18 Oct 2012, Bob Liu wrote:
> Introduce hugepage_get_pmd() to simple code.
>
I don't see this as simple just because you're removing more lines, I find
pagetable walks to be harder to read when split up like this and the "get"
part implies you're grabbing a reference on the pmd, which you're not.
On Thu, 18 Oct 2012, Bob Liu wrote:
> Multi place do the same check.
>
This only succeeds in removing four lines and makes the code more
difficult to follow, nack.
On Thu, 18 Oct 2012 13:16:04 -0700 (PDT)
David Rientjes <[email protected]> wrote:
> On Thu, 18 Oct 2012, Bob Liu wrote:
>
> > Multi place do the same check.
> >
>
> This only succeeds in removing four lines and makes the code more
> difficult to follow
I like it. It takes a bunch of code sequences which may or may not be the
same and removes all doubt, while making the call sites clearer.