Messages in this thread | | | From | KOSAKI Motohiro <> | Subject | Re: [PATCH v2] fix mlocked page counter mistmatch | Date | Thu, 5 Feb 2009 11:17:46 +0900 (JST) |
| |
> > 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?
| |