Re: [PATCH v2] fix mlocked page counter mistmatch

MinChan Kim
2009-02-05T02:32:47Z

On Thu, Feb 5, 2009 at 11:17 AM, KOSAKI Motohiro
<kosaki.motohiro@jp.fujitsu.com> wrote:
>> > and, I think current try_to_mlock_page() is correct. no need change.
>> > Why?
>> >
>> > 1. Generally, mmap_sem holding is necessary when vma->vm_flags accessed.
>> > that's vma's basic rule.
>> > 2. However, try_to_unmap_one() doesn't held mamp_sem. but that's ok.
>> > it often get incorrect result. but caller consider incorrect value safe.
>> > 3. try_to_mlock_page() need mmap_sem because it obey rule (1).
>> > 4. in try_to_mlock_page(), if down_read_trylock() is failure,
>> > we can't move the page to unevictable list. but that's ok.
>> > the page in evictable list is periodically try to reclaim. and
>> > be called try_to_unmap().
>> > try_to_unmap() (and its caller) also move the unevictable page to unevictable list.
>> > Therefore, in long term view, the page leak is not happend.
>>
>> Thanks for clarification.
>> In long term view, you're right.
>>
>> but My concern is that munlock[all] pathes always hold down of mmap_sem.
>> After all, down_read_trylock always wil fail for such cases.
>>
>> So, current task's mlocked pages only can be reclaimed
>> by background or direct reclaim path if the task don't exit.
>>
>> I think it can increase reclaim overhead unnecessary
>> if there are lots of such tasks.
>>
>> What's your opinion ?
>
> I have 2 comment.
>
> 1. typical application never munlock()ed at all.

Sometime application of embedded can do it.
That's becuase they want deterministic page allocation in some situation.
However, It's not a matter in here.

> and exit() path is already efficient.
> then, I don't like hacky apploach.
> 2. I think we should drop mmap_sem holding in munlock path in the future.
> at that time, this issue disappear automatically.
> it's clean way more.

If we can drop mmap_sem in munlock path, I am happy, too.
Please, CCed me if you make a patch for it.

By that time, I will fold this issue. :)

>
> What do you think it?
>
>
>

--
Thanks,
MinChan Kim