Although the zero huge page is being shared across various processes, each
mapping needs to update its mm's MM_ANONPAGES stat by HPAGE_PMD_NR in order
to be consistent. This just updates the stats in set_huge_zero_page() after
the mapping gets created.
Cc: Andrew Morton <[email protected]>
Cc: Zi Yan <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Anshuman Khandual <[email protected]>
---
Should it update MM_SHMEM_PAGES instead ? Applies on latest mainline.
mm/huge_memory.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 63ed6b25deaa..262703304807 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -706,6 +706,7 @@ static void set_huge_zero_page(pgtable_t pgtable, struct mm_struct *mm,
if (pgtable)
pgtable_trans_huge_deposit(mm, pmd, pgtable);
set_pmd_at(mm, haddr, pmd, entry);
+ add_mm_counter(mm, MM_ANONPAGES, HPAGE_PMD_NR);
mm_inc_nr_ptes(mm);
}
--
2.20.1
On 13 May 2021, at 3:20, Anshuman Khandual wrote:
> Although the zero huge page is being shared across various processes, each
> mapping needs to update its mm's MM_ANONPAGES stat by HPAGE_PMD_NR in order
> to be consistent. This just updates the stats in set_huge_zero_page() after
> the mapping gets created.
In addition, MM_ANONPAGES stats should be decreased at zap_huge_pmd() and
__split_huge_pmd_locked() when the zero huge page mapping is removed from
a process, right?
>
> Cc: Andrew Morton <[email protected]>
> Cc: Zi Yan <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> Should it update MM_SHMEM_PAGES instead ? Applies on latest mainline.
zero huge page is added via do_huge_pmd_anonymous_page(), I think MM_ANONPAGES
is appropriate.
>
> mm/huge_memory.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 63ed6b25deaa..262703304807 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -706,6 +706,7 @@ static void set_huge_zero_page(pgtable_t pgtable, struct mm_struct *mm,
> if (pgtable)
> pgtable_trans_huge_deposit(mm, pmd, pgtable);
> set_pmd_at(mm, haddr, pmd, entry);
> + add_mm_counter(mm, MM_ANONPAGES, HPAGE_PMD_NR);
> mm_inc_nr_ptes(mm);
> }
>
> --
> 2.20.1
—
Best Regards,
Yan Zi
On Thu, May 13, 2021 at 12:20 AM Anshuman Khandual
<[email protected]> wrote:
>
> Although the zero huge page is being shared across various processes, each
> mapping needs to update its mm's MM_ANONPAGES stat by HPAGE_PMD_NR in order
> to be consistent. This just updates the stats in set_huge_zero_page() after
> the mapping gets created.
I don't get why MM_ANONPAGES needs to be inc'ed when huge zero page is
installed. This may cause inconsistency between some counters, for
example, MM_ANONPAGES may be much bigger than anon LRU.
MM_ANONPAGES should not be inc'ed unless a new page is allocated and
installed, right?
>
> Cc: Andrew Morton <[email protected]>
> Cc: Zi Yan <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Anshuman Khandual <[email protected]>
> ---
> Should it update MM_SHMEM_PAGES instead ? Applies on latest mainline.
>
> mm/huge_memory.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 63ed6b25deaa..262703304807 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -706,6 +706,7 @@ static void set_huge_zero_page(pgtable_t pgtable, struct mm_struct *mm,
> if (pgtable)
> pgtable_trans_huge_deposit(mm, pmd, pgtable);
> set_pmd_at(mm, haddr, pmd, entry);
> + add_mm_counter(mm, MM_ANONPAGES, HPAGE_PMD_NR);
> mm_inc_nr_ptes(mm);
> }
>
> --
> 2.20.1
>
>
On Thu, May 13, 2021 at 9:50 AM Yang Shi <[email protected]> wrote:
>
> On Thu, May 13, 2021 at 12:20 AM Anshuman Khandual
> <[email protected]> wrote:
> >
> > Although the zero huge page is being shared across various processes, each
> > mapping needs to update its mm's MM_ANONPAGES stat by HPAGE_PMD_NR in order
> > to be consistent. This just updates the stats in set_huge_zero_page() after
> > the mapping gets created.
>
> I don't get why MM_ANONPAGES needs to be inc'ed when huge zero page is
> installed. This may cause inconsistency between some counters, for
> example, MM_ANONPAGES may be much bigger than anon LRU.
>
> MM_ANONPAGES should not be inc'ed unless a new page is allocated and
> installed, right?
I just realized I mixed MM_ANONPAGES up with the global anon pages
counter. Take back my comment. Sorry for the confusion.
>
> >
> > Cc: Andrew Morton <[email protected]>
> > Cc: Zi Yan <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Anshuman Khandual <[email protected]>
> > ---
> > Should it update MM_SHMEM_PAGES instead ? Applies on latest mainline.
> >
> > mm/huge_memory.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index 63ed6b25deaa..262703304807 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -706,6 +706,7 @@ static void set_huge_zero_page(pgtable_t pgtable, struct mm_struct *mm,
> > if (pgtable)
> > pgtable_trans_huge_deposit(mm, pmd, pgtable);
> > set_pmd_at(mm, haddr, pmd, entry);
> > + add_mm_counter(mm, MM_ANONPAGES, HPAGE_PMD_NR);
> > mm_inc_nr_ptes(mm);
> > }
> >
> > --
> > 2.20.1
> >
> >
On 5/13/21 7:42 PM, Zi Yan wrote:
> On 13 May 2021, at 3:20, Anshuman Khandual wrote:
>
>> Although the zero huge page is being shared across various processes, each
>> mapping needs to update its mm's MM_ANONPAGES stat by HPAGE_PMD_NR in order
>> to be consistent. This just updates the stats in set_huge_zero_page() after
>> the mapping gets created.
>
> In addition, MM_ANONPAGES stats should be decreased at zap_huge_pmd() and
Right, would something like this work ?
diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 63ed6b2..776984d 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -1678,6 +1678,7 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
tlb_remove_page_size(tlb, pmd_page(orig_pmd), HPAGE_PMD_SIZE);
} else if (is_huge_zero_pmd(orig_pmd)) {
zap_deposited_table(tlb->mm, pmd);
+ add_mm_counter(tlb->mm, MM_ANONPAGES, -HPAGE_PMD_NR);
spin_unlock(ptl);
tlb_remove_page_size(tlb, pmd_page(orig_pmd), HPAGE_PMD_SIZE);
} else {
> __split_huge_pmd_locked() when the zero huge page mapping is removed from
> a process, right?
__split_huge_pmd_locked() calls __split_huge_zero_page_pmd() which will
replace a zero huge page with multiple (HPAGE_PMD_NR) zero small pages.
Why should MM_ANONPAGES stats change for the MM when the mapping is still
out there but in normal pages now.
>
>>
>> Cc: Andrew Morton <[email protected]>
>> Cc: Zi Yan <[email protected]>
>> Cc: [email protected]
>> Cc: [email protected]
>> Signed-off-by: Anshuman Khandual <[email protected]>
>> ---
>> Should it update MM_SHMEM_PAGES instead ? Applies on latest mainline.
>
> zero huge page is added via do_huge_pmd_anonymous_page(), I think MM_ANONPAGES
> is appropriate.
Okay, sure.
>
>>
>> mm/huge_memory.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 63ed6b25deaa..262703304807 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
>> @@ -706,6 +706,7 @@ static void set_huge_zero_page(pgtable_t pgtable, struct mm_struct *mm,
>> if (pgtable)
>> pgtable_trans_huge_deposit(mm, pmd, pgtable);
>> set_pmd_at(mm, haddr, pmd, entry);
>> + add_mm_counter(mm, MM_ANONPAGES, HPAGE_PMD_NR);
>> mm_inc_nr_ptes(mm);
>> }
>>
>> --
>> 2.20.1
>
>
> —
> Best Regards,
> Yan Zi
>
On 16 May 2021, at 23:51, Anshuman Khandual wrote:
> On 5/13/21 7:42 PM, Zi Yan wrote:
>> On 13 May 2021, at 3:20, Anshuman Khandual wrote:
>>
>>> Although the zero huge page is being shared across various processes, each
>>> mapping needs to update its mm's MM_ANONPAGES stat by HPAGE_PMD_NR in order
>>> to be consistent. This just updates the stats in set_huge_zero_page() after
>>> the mapping gets created.
>>
>> In addition, MM_ANONPAGES stats should be decreased at zap_huge_pmd() and
>
> Right, would something like this work ?
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 63ed6b2..776984d 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -1678,6 +1678,7 @@ int zap_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
> tlb_remove_page_size(tlb, pmd_page(orig_pmd), HPAGE_PMD_SIZE);
> } else if (is_huge_zero_pmd(orig_pmd)) {
> zap_deposited_table(tlb->mm, pmd);
> + add_mm_counter(tlb->mm, MM_ANONPAGES, -HPAGE_PMD_NR);
> spin_unlock(ptl);
> tlb_remove_page_size(tlb, pmd_page(orig_pmd), HPAGE_PMD_SIZE);
> } else {
>
LGTM.
>> __split_huge_pmd_locked() when the zero huge page mapping is removed from
>> a process, right?
>
> __split_huge_pmd_locked() calls __split_huge_zero_page_pmd() which will
> replace a zero huge page with multiple (HPAGE_PMD_NR) zero small pages.
> Why should MM_ANONPAGES stats change for the MM when the mapping is still
> out there but in normal pages now.
Ah, you are right. I missed this part. No need to change __split_huge_pmd_locked().
>>
>>>
>>> Cc: Andrew Morton <[email protected]>
>>> Cc: Zi Yan <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> Signed-off-by: Anshuman Khandual <[email protected]>
>>> ---
>>> Should it update MM_SHMEM_PAGES instead ? Applies on latest mainline.
>>
>> zero huge page is added via do_huge_pmd_anonymous_page(), I think MM_ANONPAGES
>> is appropriate.
>
> Okay, sure.
>
>>
>>>
>>> mm/huge_memory.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>>> index 63ed6b25deaa..262703304807 100644
>>> --- a/mm/huge_memory.c
>>> +++ b/mm/huge_memory.c
>>> @@ -706,6 +706,7 @@ static void set_huge_zero_page(pgtable_t pgtable, struct mm_struct *mm,
>>> if (pgtable)
>>> pgtable_trans_huge_deposit(mm, pmd, pgtable);
>>> set_pmd_at(mm, haddr, pmd, entry);
>>> + add_mm_counter(mm, MM_ANONPAGES, HPAGE_PMD_NR);
>>> mm_inc_nr_ptes(mm);
>>> }
>>>
>>> --
>>> 2.20.1
>>
>>
>> —
>> Best Regards,
>> Yan Zi
>>
—
Best Regards,
Yan, Zi