From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753640AbZBDOHO (ORCPT ); Wed, 4 Feb 2009 09:07:14 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751338AbZBDOHA (ORCPT ); Wed, 4 Feb 2009 09:07:00 -0500 Received: from g1t0029.austin.hp.com ([15.216.28.36]:27094 "EHLO g1t0029.austin.hp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751117AbZBDOG7 (ORCPT ); Wed, 4 Feb 2009 09:06:59 -0500 Subject: Re: [PATCH v2] fix mlocked page counter mistmatch From: Lee Schermerhorn To: KOSAKI Motohiro Cc: MinChan Kim , Andrew Morton , linux mm , linux kernel , Nick Piggin , Rik van Riel In-Reply-To: <20090204171639.ECCE.KOSAKI.MOTOHIRO@jp.fujitsu.com> References: <20090204115047.ECB5.KOSAKI.MOTOHIRO@jp.fujitsu.com> <20090204045745.GC6212@barrios-desktop> <20090204171639.ECCE.KOSAKI.MOTOHIRO@jp.fujitsu.com> Content-Type: text/plain Organization: HP/OSLO Date: Wed, 04 Feb 2009 09:07:16 -0500 Message-Id: <1233756436.14819.13.camel@lts-notebook> Mime-Version: 1.0 X-Mailer: Evolution 2.22.3.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2009-02-04 at 19:28 +0900, KOSAKI Motohiro wrote: > > With '29-rc3-git5', I found, > > > > static int try_to_mlock_page(struct page *page, struct vm_area_struct *vma) > > { > > int mlocked = 0; > > > > if (down_read_trylock(&vma->vm_mm->mmap_sem)) { > > if (vma->vm_flags & VM_LOCKED) { > > mlock_vma_page(page); > > mlocked++; /* really mlocked the page */ > > } > > up_read(&vma->vm_mm->mmap_sem); > > } > > return mlocked; > > } > > > > It still try to downgrade mmap_sem. > > Do I miss something ? > > sorry, I misunderstood your "downgrade". I said linus removed downgrade_write(&mma_sem). > > Now, I understand this issue perfectly. I agree you and lee-san's fix is correct. > Acked-by: KOSAKI Motohiro > > > 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. > Also worth noting that down_read_trylock() does not "downgrade" the semaphore. It only tries to acquire it in read mode. As Kosaki-san says, try_to_unmap() doesn't normally hold the mmap_sem. It needs to acquire it here to stabilize the vma [vm_flags] while mlocking the pages. This is the place where a page mapped in a VM_LOCKED vma that vmscan found on the normal lru list--e.g., because we couldn't isolate them in mlock_vma_page()--get marked mlocked, if not already marked. mlock_vma_page() is a no-op if page is already mlocked. If we successsfully acquire the mmap_sem and the vma is still VM_LOCKED, we know that the page is mlocked and try_to_unmap() will return SWAP_MLOCK. This allows vmscan [shrink_page_list()] to move the page to the unevictable list and not need to bother with it in subsequent scans until it becomes munlocked. Lee