[PATCH v2] mm: fix mlock incorrent event account

[PATCH v2] mm: fix mlock incorrent event account

[zhongjiang]

From: zhong jiang <zhongjiang@huawei.com>

Recently, when I address in the issue, Subject "mlock: fix mlock count
can not decrease in race condition" had been take over, I review
the code and find the potential issue. it will result in the incorrect
account, it will make us misunderstand straightforward.

The following testcase can prove the issue.

int main(void)
{
    char *map;
    int fd;

    fd = open("test", O_CREAT|O_RDWR);
    unlink("test");
    ftruncate(fd, 4096);
    map = mmap(NULL, 4096, PROT_WRITE, MAP_PRIVATE, fd, 0);
    map[0] = 11;
    mlock(map, 4096);
    ftruncate(fd, 0);
    close(fd);
    munlock(map, 4096);
    munmap(map, 4096);

    return 0;
}

before:
unevictable_pgs_mlocked 10589
unevictable_pgs_munlocked 10588
unevictable_pgs_cleared 1

apply the patch;
after:
unevictable_pgs_mlocked 9497
unevictable_pgs_munlocked 9497
unevictable_pgs_cleared 1

unmap_mapping_range unmap them,  page_remove_rmap will deal with
clear_page_mlock situation.  we clear page Mlock flag and successful
isolate the page,  the page will putback the evictable list. but it is not
record the munlock event.

The patch add the event account when successful page isolation.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 mm/mlock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/mlock.c b/mm/mlock.c
index c483c5c..941930b 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -64,6 +64,7 @@ void clear_page_mlock(struct page *page)
 			    -hpage_nr_pages(page));
 	count_vm_event(UNEVICTABLE_PGCLEARED);
 	if (!isolate_lru_page(page)) {
+		count_vm_event(UNEVICTABLE_PGMUNLOCKED);
 		putback_lru_page(page);
 	} else {
 		/*
-- 
1.8.3.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm: fix mlock incorrent event account

[Vlastimil Babka]

On 05/26/2017 05:54 AM, zhongjiang wrote:
> From: zhong jiang <zhongjiang@huawei.com>
> 
> Recently, when I address in the issue, Subject "mlock: fix mlock count
> can not decrease in race condition" had been take over, I review
> the code and find the potential issue. it will result in the incorrect
> account, it will make us misunderstand straightforward.
> 
> The following testcase can prove the issue.
> 
> int main(void)
> {
>     char *map;
>     int fd;
> 
>     fd = open("test", O_CREAT|O_RDWR);
>     unlink("test");
>     ftruncate(fd, 4096);
>     map = mmap(NULL, 4096, PROT_WRITE, MAP_PRIVATE, fd, 0);
>     map[0] = 11;
>     mlock(map, 4096);
>     ftruncate(fd, 0);
>     close(fd);
>     munlock(map, 4096);
>     munmap(map, 4096);
> 
>     return 0;
> }
> 
> before:
> unevictable_pgs_mlocked 10589
> unevictable_pgs_munlocked 10588
> unevictable_pgs_cleared 1
> 
> apply the patch;
> after:
> unevictable_pgs_mlocked 9497
> unevictable_pgs_munlocked 9497
> unevictable_pgs_cleared 1
> 
> unmap_mapping_range unmap them,  page_remove_rmap will deal with
> clear_page_mlock situation.  we clear page Mlock flag and successful
> isolate the page,  the page will putback the evictable list. but it is not
> record the munlock event.
> 
> The patch add the event account when successful page isolation.
> 
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>

Hi,

I think this is by design. UNEVICTABLE_PGMUNLOCKED is supposed for explicit
munlock() actions from userspace. Truncation etc is counted by
UNEVICTABLE_PGCLEARED.

Vlastimil

> ---
>  mm/mlock.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/mm/mlock.c b/mm/mlock.c
> index c483c5c..941930b 100644
> --- a/mm/mlock.c
> +++ b/mm/mlock.c
> @@ -64,6 +64,7 @@ void clear_page_mlock(struct page *page)
>  			    -hpage_nr_pages(page));
>  	count_vm_event(UNEVICTABLE_PGCLEARED);
>  	if (!isolate_lru_page(page)) {
> +		count_vm_event(UNEVICTABLE_PGMUNLOCKED);
>  		putback_lru_page(page);
>  	} else {
>  		/*
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm: fix mlock incorrent event account

[zhong jiang]

On 2017/5/26 17:06, Vlastimil Babka wrote:
> On 05/26/2017 05:54 AM, zhongjiang wrote:
>> From: zhong jiang <zhongjiang@huawei.com>
>>
>> Recently, when I address in the issue, Subject "mlock: fix mlock count
>> can not decrease in race condition" had been take over, I review
>> the code and find the potential issue. it will result in the incorrect
>> account, it will make us misunderstand straightforward.
>>
>> The following testcase can prove the issue.
>>
>> int main(void)
>> {
>>     char *map;
>>     int fd;
>>
>>     fd = open("test", O_CREAT|O_RDWR);
>>     unlink("test");
>>     ftruncate(fd, 4096);
>>     map = mmap(NULL, 4096, PROT_WRITE, MAP_PRIVATE, fd, 0);
>>     map[0] = 11;
>>     mlock(map, 4096);
>>     ftruncate(fd, 0);
>>     close(fd);
>>     munlock(map, 4096);
>>     munmap(map, 4096);
>>
>>     return 0;
>> }
>>
>> before:
>> unevictable_pgs_mlocked 10589
>> unevictable_pgs_munlocked 10588
>> unevictable_pgs_cleared 1
>>
>> apply the patch;
>> after:
>> unevictable_pgs_mlocked 9497
>> unevictable_pgs_munlocked 9497
>> unevictable_pgs_cleared 1
>>
>> unmap_mapping_range unmap them,  page_remove_rmap will deal with
>> clear_page_mlock situation.  we clear page Mlock flag and successful
>> isolate the page,  the page will putback the evictable list. but it is not
>> record the munlock event.
>>
>> The patch add the event account when successful page isolation.
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> Hi,
>
> I think this is by design. UNEVICTABLE_PGMUNLOCKED is supposed for explicit
> munlock() actions from userspace. Truncation etc is counted by
> UNEVICTABLE_PGCLEARED.
>
> Vlastimil
 it seems make sense. but the we just mmap the specified ragne and mlock it.
 when the process drop out, exit_mmap will call munlock_vma_pages_all to
 unmap the mlock range and make the event normal.
 
 Do you think?

 Thanks
zhongjinag
>> ---
>>  mm/mlock.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/mlock.c b/mm/mlock.c
>> index c483c5c..941930b 100644
>> --- a/mm/mlock.c
>> +++ b/mm/mlock.c
>> @@ -64,6 +64,7 @@ void clear_page_mlock(struct page *page)
>>  			    -hpage_nr_pages(page));
>>  	count_vm_event(UNEVICTABLE_PGCLEARED);
>>  	if (!isolate_lru_page(page)) {
>> +		count_vm_event(UNEVICTABLE_PGMUNLOCKED);
>>  		putback_lru_page(page);
>>  	} else {
>>  		/*
>>
>
> .
>


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm: fix mlock incorrent event account

[Michal Hocko]

On Fri 26-05-17 11:06:31, Vlastimil Babka wrote:
> On 05/26/2017 05:54 AM, zhongjiang wrote:
> > From: zhong jiang <zhongjiang@huawei.com>
> > 
> > Recently, when I address in the issue, Subject "mlock: fix mlock count
> > can not decrease in race condition" had been take over, I review
> > the code and find the potential issue. it will result in the incorrect
> > account, it will make us misunderstand straightforward.
> > 
> > The following testcase can prove the issue.
> > 
> > int main(void)
> > {
> >     char *map;
> >     int fd;
> > 
> >     fd = open("test", O_CREAT|O_RDWR);
> >     unlink("test");
> >     ftruncate(fd, 4096);
> >     map = mmap(NULL, 4096, PROT_WRITE, MAP_PRIVATE, fd, 0);
> >     map[0] = 11;
> >     mlock(map, 4096);
> >     ftruncate(fd, 0);
> >     close(fd);
> >     munlock(map, 4096);
> >     munmap(map, 4096);
> > 
> >     return 0;
> > }
> > 
> > before:
> > unevictable_pgs_mlocked 10589
> > unevictable_pgs_munlocked 10588
> > unevictable_pgs_cleared 1
> > 
> > apply the patch;
> > after:
> > unevictable_pgs_mlocked 9497
> > unevictable_pgs_munlocked 9497
> > unevictable_pgs_cleared 1
> > 
> > unmap_mapping_range unmap them,  page_remove_rmap will deal with
> > clear_page_mlock situation.  we clear page Mlock flag and successful
> > isolate the page,  the page will putback the evictable list. but it is not
> > record the munlock event.
> > 
> > The patch add the event account when successful page isolation.
> > 
> > Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> 
> Hi,
> 
> I think this is by design. UNEVICTABLE_PGMUNLOCKED is supposed for explicit
> munlock() actions from userspace. Truncation etc is counted by
> UNEVICTABLE_PGCLEARED.

I guess we really need to change
$ git grep UNEVICTABLE_PGCLEARED -- Documentation/
$

into something more comprehensive
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] mm: fix mlock incorrent event account

[Vlastimil Babka]

On 05/26/2017 01:05 PM, zhong jiang wrote:
> On 2017/5/26 17:06, Vlastimil Babka wrote:
>> On 05/26/2017 05:54 AM, zhongjiang wrote:
>>> From: zhong jiang <zhongjiang@huawei.com>
>>>
>>> Recently, when I address in the issue, Subject "mlock: fix mlock count
>>> can not decrease in race condition" had been take over, I review
>>> the code and find the potential issue. it will result in the incorrect
>>> account, it will make us misunderstand straightforward.
>>>
>>> The following testcase can prove the issue.
>>>
>>> int main(void)
>>> {
>>>     char *map;
>>>     int fd;
>>>
>>>     fd = open("test", O_CREAT|O_RDWR);
>>>     unlink("test");
>>>     ftruncate(fd, 4096);
>>>     map = mmap(NULL, 4096, PROT_WRITE, MAP_PRIVATE, fd, 0);
>>>     map[0] = 11;
>>>     mlock(map, 4096);
>>>     ftruncate(fd, 0);
>>>     close(fd);
>>>     munlock(map, 4096);
>>>     munmap(map, 4096);
>>>
>>>     return 0;
>>> }
>>>
>>> before:
>>> unevictable_pgs_mlocked 10589
>>> unevictable_pgs_munlocked 10588
>>> unevictable_pgs_cleared 1
>>>
>>> apply the patch;
>>> after:
>>> unevictable_pgs_mlocked 9497
>>> unevictable_pgs_munlocked 9497
>>> unevictable_pgs_cleared 1
>>>
>>> unmap_mapping_range unmap them,  page_remove_rmap will deal with
>>> clear_page_mlock situation.  we clear page Mlock flag and successful
>>> isolate the page,  the page will putback the evictable list. but it is not
>>> record the munlock event.
>>>
>>> The patch add the event account when successful page isolation.
>>>
>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> Hi,
>>
>> I think this is by design. UNEVICTABLE_PGMUNLOCKED is supposed for explicit
>> munlock() actions from userspace. Truncation etc is counted by
>> UNEVICTABLE_PGCLEARED.
>>
>> Vlastimil
>  it seems make sense. but the we just mmap the specified ragne and mlock it.
>  when the process drop out, exit_mmap will call munlock_vma_pages_all to
>  unmap the mlock range and make the event normal.

Uh, so it seems exit counts as explicit munlock.

As Michal suggest, it would be useful to first document the counters
thoroughly before we can debate whether the semantics is inappropriate
and needs some overhaul. It looks like nobody cared much until now.

Vlastimil

>  Do you think?
> 
>  Thanks
> zhongjinag
>>> ---
>>>  mm/mlock.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/mm/mlock.c b/mm/mlock.c
>>> index c483c5c..941930b 100644
>>> --- a/mm/mlock.c
>>> +++ b/mm/mlock.c
>>> @@ -64,6 +64,7 @@ void clear_page_mlock(struct page *page)
>>>  			    -hpage_nr_pages(page));
>>>  	count_vm_event(UNEVICTABLE_PGCLEARED);
>>>  	if (!isolate_lru_page(page)) {
>>> +		count_vm_event(UNEVICTABLE_PGMUNLOCKED);
>>>  		putback_lru_page(page);
>>>  	} else {
>>>  		/*
>>>
>>
>> .
>>
> 
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>