Re: [PATCH v2] fix mlocked page counter mistmatch

KOSAKI Motohiro
2009-02-05T02:18:00Z

> > 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.
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.

What do you think it?