[v2 PATCH 1/2] mm: thp: register mm for khugepaged when merging vma for shmem

[v2 PATCH 1/2] mm: thp: register mm for khugepaged when merging vma for shmem

[Yang Shi]

When merging anonymous page vma, if the size of vma can fit in at least
one hugepage, the mm will be registered for khugepaged for collapsing
THP in the future.

But, it skips shmem vma. Doing so for shmem too, but not file-private
mapping, when merging vma in order to increase the odd to collapse
hugepage by khugepaged.

Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
---
v1 --> 2:
* Exclude file-private mapping per Kirill's comment

 mm/khugepaged.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index d7b2a4b..9b0ec30 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -440,8 +440,12 @@ int khugepaged_enter_vma_merge(struct vm_area_struct *vma,
 		 * page fault if needed.
 		 */
 		return 0;
-	if (vma->vm_ops || (vm_flags & VM_NO_KHUGEPAGED))
-		/* khugepaged not yet working on file or special mappings */
+	if ((vma->vm_ops && (!shmem_file(vma->vm_file) || vma->anon_vma)) ||
+	    (vm_flags & VM_NO_KHUGEPAGED))
+		/*
+		 * khugepaged not yet working on non-shmem file or special
+		 * mappings. And, file-private shmem THP is not supported.
+		 */
 		return 0;
 	hstart = (vma->vm_start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK;
 	hend = vma->vm_end & HPAGE_PMD_MASK;
-- 
1.8.3.1

[v2 PATCH 2/2] mm: thp: inc counter for collapsed shmem THP

[Yang Shi]

/sys/kernel/mm/transparent_hugepage/khugepaged/pages_collapsed is used
to record the counter of collapsed THP, but it just gets inc'ed in
anonymous THP collapse path, do this for shmem THP collapse too.

Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
Cc: Hugh Dickins <hughd@google.com>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
---
v1 --> v2:
* Seperate from the previous patch per Kirill's comment

 mm/khugepaged.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index 9b0ec30..4018826 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -1521,6 +1521,8 @@ static void collapse_shmem(struct mm_struct *mm,
 		unlock_page(new_page);
 
 		*hpage = NULL;
+
+		khugepaged_pages_collapsed++;
 	} else {
 		/* Something went wrong: rollback changes to the radix-tree */
 		shmem_uncharge(mapping->host, nr_none);
-- 
1.8.3.1

Re: [v2 PATCH 2/2] mm: thp: inc counter for collapsed shmem THP

[Kirill A. Shutemov]

On Thu, Jun 21, 2018 at 11:15:49PM +0000, yang.shi@linux.alibaba.com wrote:
> /sys/kernel/mm/transparent_hugepage/khugepaged/pages_collapsed is used
> to record the counter of collapsed THP, but it just gets inc'ed in
> anonymous THP collapse path, do this for shmem THP collapse too.
> 
> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

Re: [v2 PATCH 1/2] mm: thp: register mm for khugepaged when merging vma for shmem

[Kirill A. Shutemov]

On Thu, Jun 21, 2018 at 11:15:48PM +0000, yang.shi@linux.alibaba.com wrote:
> When merging anonymous page vma, if the size of vma can fit in at least
> one hugepage, the mm will be registered for khugepaged for collapsing
> THP in the future.
> 
> But, it skips shmem vma. Doing so for shmem too, but not file-private
> mapping, when merging vma in order to increase the odd to collapse
> hugepage by khugepaged.
> 
> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
> Cc: Hugh Dickins <hughd@google.com>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> ---
> v1 --> 2:
> * Exclude file-private mapping per Kirill's comment
> 
>  mm/khugepaged.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index d7b2a4b..9b0ec30 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -440,8 +440,12 @@ int khugepaged_enter_vma_merge(struct vm_area_struct *vma,
>  		 * page fault if needed.
>  		 */
>  		return 0;
> -	if (vma->vm_ops || (vm_flags & VM_NO_KHUGEPAGED))
> -		/* khugepaged not yet working on file or special mappings */
> +	if ((vma->vm_ops && (!shmem_file(vma->vm_file) || vma->anon_vma)) ||
> +	    (vm_flags & VM_NO_KHUGEPAGED))
> +		/*
> +		 * khugepaged not yet working on non-shmem file or special
> +		 * mappings. And, file-private shmem THP is not supported.
> +		 */
>  		return 0;

My point was that vma->anon_vma check above this one should not prevent
collapse for shmem.

Looking into this more, I think we should just replace all these checks
with hugepage_vma_check() call.

-- 
 Kirill A. Shutemov

Re: [v2 PATCH 1/2] mm: thp: register mm for khugepaged when merging vma for shmem

[Yang Shi]

On 6/22/18 12:59 AM, Kirill A. Shutemov wrote:
> On Thu, Jun 21, 2018 at 11:15:48PM +0000, yang.shi@linux.alibaba.com wrote:
>> When merging anonymous page vma, if the size of vma can fit in at least
>> one hugepage, the mm will be registered for khugepaged for collapsing
>> THP in the future.
>>
>> But, it skips shmem vma. Doing so for shmem too, but not file-private
>> mapping, when merging vma in order to increase the odd to collapse
>> hugepage by khugepaged.
>>
>> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
>> Cc: Hugh Dickins <hughd@google.com>
>> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
>> ---
>> v1 --> 2:
>> * Exclude file-private mapping per Kirill's comment
>>
>>   mm/khugepaged.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index d7b2a4b..9b0ec30 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -440,8 +440,12 @@ int khugepaged_enter_vma_merge(struct vm_area_struct *vma,
>>   		 * page fault if needed.
>>   		 */
>>   		return 0;
>> -	if (vma->vm_ops || (vm_flags & VM_NO_KHUGEPAGED))
>> -		/* khugepaged not yet working on file or special mappings */
>> +	if ((vma->vm_ops && (!shmem_file(vma->vm_file) || vma->anon_vma)) ||
>> +	    (vm_flags & VM_NO_KHUGEPAGED))
>> +		/*
>> +		 * khugepaged not yet working on non-shmem file or special
>> +		 * mappings. And, file-private shmem THP is not supported.
>> +		 */
>>   		return 0;
> My point was that vma->anon_vma check above this one should not prevent
> collapse for shmem.
>
> Looking into this more, I think we should just replace all these checks
> with hugepage_vma_check() call.

I got a little bit confused here. I thought the condition to *not* 
collapse file-private shmem mapping should be:

shmem_file(vma->vm_file) && vma->anon_vma

Is this right?

If it is right, it looks hugepage_vma_check() can't do this since it 
just returns true if shmem_file is true and it is page aligned without 
checking vma->anon_vma.

Thanks,
Yang

>

Re: [v2 PATCH 1/2] mm: thp: register mm for khugepaged when merging vma for shmem

[Kirill A. Shutemov]

On Fri, Jun 22, 2018 at 04:04:12PM +0000, yang.shi@linux.alibaba.com wrote:
> 
> 
> On 6/22/18 12:59 AM, Kirill A. Shutemov wrote:
> > On Thu, Jun 21, 2018 at 11:15:48PM +0000, yang.shi@linux.alibaba.com wrote:
> > > When merging anonymous page vma, if the size of vma can fit in at least
> > > one hugepage, the mm will be registered for khugepaged for collapsing
> > > THP in the future.
> > > 
> > > But, it skips shmem vma. Doing so for shmem too, but not file-private
> > > mapping, when merging vma in order to increase the odd to collapse
> > > hugepage by khugepaged.
> > > 
> > > Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
> > > Cc: Hugh Dickins <hughd@google.com>
> > > Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > > Cc: Vlastimil Babka <vbabka@suse.cz>
> > > ---
> > > v1 --> 2:
> > > * Exclude file-private mapping per Kirill's comment
> > > 
> > >   mm/khugepaged.c | 8 ++++++--
> > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > > index d7b2a4b..9b0ec30 100644
> > > --- a/mm/khugepaged.c
> > > +++ b/mm/khugepaged.c
> > > @@ -440,8 +440,12 @@ int khugepaged_enter_vma_merge(struct vm_area_struct *vma,
> > >   		 * page fault if needed.
> > >   		 */
> > >   		return 0;
> > > -	if (vma->vm_ops || (vm_flags & VM_NO_KHUGEPAGED))
> > > -		/* khugepaged not yet working on file or special mappings */
> > > +	if ((vma->vm_ops && (!shmem_file(vma->vm_file) || vma->anon_vma)) ||
> > > +	    (vm_flags & VM_NO_KHUGEPAGED))
> > > +		/*
> > > +		 * khugepaged not yet working on non-shmem file or special
> > > +		 * mappings. And, file-private shmem THP is not supported.
> > > +		 */
> > >   		return 0;
> > My point was that vma->anon_vma check above this one should not prevent
> > collapse for shmem.
> > 
> > Looking into this more, I think we should just replace all these checks
> > with hugepage_vma_check() call.
> 
> I got a little bit confused here. I thought the condition to *not* collapse
> file-private shmem mapping should be:
> 
> shmem_file(vma->vm_file) && vma->anon_vma
> 
> Is this right?

No, if shmem_file(vma->vm_file) is true, vma->anon_vma doesn't matter.
We don't care about anon_vma in such VMA as we don't touch file-private
pages.

-- 
 Kirill A. Shutemov

Re: [v2 PATCH 1/2] mm: thp: register mm for khugepaged when merging vma for shmem

[Yang Shi]

On 6/22/18 9:19 AM, Kirill A. Shutemov wrote:
> On Fri, Jun 22, 2018 at 04:04:12PM +0000, yang.shi@linux.alibaba.com wrote:
>>
>> On 6/22/18 12:59 AM, Kirill A. Shutemov wrote:
>>> On Thu, Jun 21, 2018 at 11:15:48PM +0000, yang.shi@linux.alibaba.com wrote:
>>>> When merging anonymous page vma, if the size of vma can fit in at least
>>>> one hugepage, the mm will be registered for khugepaged for collapsing
>>>> THP in the future.
>>>>
>>>> But, it skips shmem vma. Doing so for shmem too, but not file-private
>>>> mapping, when merging vma in order to increase the odd to collapse
>>>> hugepage by khugepaged.
>>>>
>>>> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
>>>> Cc: Hugh Dickins <hughd@google.com>
>>>> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
>>>> Cc: Vlastimil Babka <vbabka@suse.cz>
>>>> ---
>>>> v1 --> 2:
>>>> * Exclude file-private mapping per Kirill's comment
>>>>
>>>>    mm/khugepaged.c | 8 ++++++--
>>>>    1 file changed, 6 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>>>> index d7b2a4b..9b0ec30 100644
>>>> --- a/mm/khugepaged.c
>>>> +++ b/mm/khugepaged.c
>>>> @@ -440,8 +440,12 @@ int khugepaged_enter_vma_merge(struct vm_area_struct *vma,
>>>>    		 * page fault if needed.
>>>>    		 */
>>>>    		return 0;
>>>> -	if (vma->vm_ops || (vm_flags & VM_NO_KHUGEPAGED))
>>>> -		/* khugepaged not yet working on file or special mappings */
>>>> +	if ((vma->vm_ops && (!shmem_file(vma->vm_file) || vma->anon_vma)) ||
>>>> +	    (vm_flags & VM_NO_KHUGEPAGED))
>>>> +		/*
>>>> +		 * khugepaged not yet working on non-shmem file or special
>>>> +		 * mappings. And, file-private shmem THP is not supported.
>>>> +		 */
>>>>    		return 0;
>>> My point was that vma->anon_vma check above this one should not prevent
>>> collapse for shmem.
>>>
>>> Looking into this more, I think we should just replace all these checks
>>> with hugepage_vma_check() call.
>> I got a little bit confused here. I thought the condition to *not* collapse
>> file-private shmem mapping should be:
>>
>> shmem_file(vma->vm_file) && vma->anon_vma
>>
>> Is this right?
> No, if shmem_file(vma->vm_file) is true, vma->anon_vma doesn't matter.
> We don't care about anon_vma in such VMA as we don't touch file-private
> pages.

Thanks, I misunderstood that. hugepage_vma_check() sounds reasonable. 
Will fix in v3 soon.

Yang

>