[PATCH] mlock: replace stale comments in munlock_vma_page()

[PATCH] mlock: replace stale comments in munlock_vma_page()

[Lee Schermerhorn]

Cleanup stale comments on munlock_vma_page().

Signed-off-by: Lee Schermerhorn <lee.schermerhorn@hp.com>

 mm/mlock.c |   41 +++++++++++++++++++----------------------
 1 files changed, 19 insertions(+), 22 deletions(-)

Index: linux-2.6.32-rc8/mm/mlock.c
===================================================================
--- linux-2.6.32-rc8.orig/mm/mlock.c	2009-11-24 13:19:58.000000000 -0500
+++ linux-2.6.32-rc8/mm/mlock.c	2009-12-01 13:27:25.000000000 -0500
@@ -88,23 +88,20 @@ void mlock_vma_page(struct page *page)
 	}
 }
 
-/*
- * called from munlock()/munmap() path with page supposedly on the LRU.
+/**
+ * munlock_vma_page - munlock a vma page
+ * @page - page to be unlocked
  *
- * Note:  unlike mlock_vma_page(), we can't just clear the PageMlocked
- * [in try_to_munlock()] and then attempt to isolate the page.  We must
- * isolate the page to keep others from messing with its unevictable
- * and mlocked state while trying to munlock.  However, we pre-clear the
- * mlocked state anyway as we might lose the isolation race and we might
- * not get another chance to clear PageMlocked.  If we successfully
- * isolate the page and try_to_munlock() detects other VM_LOCKED vmas
- * mapping the page, it will restore the PageMlocked state, unless the page
- * is mapped in a non-linear vma.  So, we go ahead and SetPageMlocked(),
- * perhaps redundantly.
- * If we lose the isolation race, and the page is mapped by other VM_LOCKED
- * vmas, we'll detect this in vmscan--via try_to_munlock() or try_to_unmap()
- * either of which will restore the PageMlocked state by calling
- * mlock_vma_page() above, if it can grab the vma's mmap sem.
+ * called from munlock()/munmap() path with page supposedly on the LRU.
+ * When we munlock a page, because the vma where we found the page is being
+ * munlock()ed or munmap()ed, we want to check whether other vmas hold the
+ * page locked so that we can leave it on the unevictable lru list and not
+ * bother vmscan with it.  However, to walk the page's rmap list in
+ * try_to_munlock() we must isolate the page from the LRU.  If some other
+ * task has removed the page from the LRU, we won't be able to do that.
+ * So we clear the PageMlocked as we might not get another chance.  If we
+ * can't isolate the page, we leave it for putback_lru_page() and vmscan
+ * [page_referenced()/try_to_unmap()] to deal with.
  */
 static void munlock_vma_page(struct page *page)
 {
@@ -123,12 +120,12 @@ static void munlock_vma_page(struct page
 			putback_lru_page(page);
 		} else {
 			/*
-			 * We lost the race.  let try_to_unmap() deal
-			 * with it.  At least we get the page state and
-			 * mlock stats right.  However, page is still on
-			 * the noreclaim list.  We'll fix that up when
-			 * the page is eventually freed or we scan the
-			 * noreclaim list.
+			 * Some other task has removed the page from the LRU.
+			 * putback_lru_page() will take care of removing the
+			 * page from the unevictable list, if necessary.
+			 * vmscan [page_referenced()] will move the page back
+			 * to the unevictable list if some other vma has it
+			 * mlocked.
 			 */
 			if (PageUnevictable(page))
 				count_vm_event(UNEVICTABLE_PGSTRANDED);


--
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] mlock: replace stale comments in munlock_vma_page()

[Hugh Dickins]

On Wed, 2 Dec 2009, Lee Schermerhorn wrote:
> 
> Cleanup stale comments on munlock_vma_page().
> 
> Signed-off-by: Lee Schermerhorn <lee.schermerhorn@hp.com>

Acked-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>

(and Andrew has already put this into mmotm, thanks)

Sorry for not Cc'ing you earlier, Lee: git blame attributed those
lines to Nick, but I think that was because you'd started from an
old patch of Nick's, and graciously attributed it all to him.

I still don't find the comments give me complete confidence in these
manipulations and races - but that is asking for more than comments
on functions can give, especially to a sceptic like me!
Thanks a lot for updating and improving them.

Hugh

> 
>  mm/mlock.c |   41 +++++++++++++++++++----------------------
>  1 files changed, 19 insertions(+), 22 deletions(-)
> 
> Index: linux-2.6.32-rc8/mm/mlock.c
> ===================================================================
> --- linux-2.6.32-rc8.orig/mm/mlock.c	2009-11-24 13:19:58.000000000 -0500
> +++ linux-2.6.32-rc8/mm/mlock.c	2009-12-01 13:27:25.000000000 -0500
> @@ -88,23 +88,20 @@ void mlock_vma_page(struct page *page)
>  	}
>  }
>  
> -/*
> - * called from munlock()/munmap() path with page supposedly on the LRU.
> +/**
> + * munlock_vma_page - munlock a vma page
> + * @page - page to be unlocked
>   *
> - * Note:  unlike mlock_vma_page(), we can't just clear the PageMlocked
> - * [in try_to_munlock()] and then attempt to isolate the page.  We must
> - * isolate the page to keep others from messing with its unevictable
> - * and mlocked state while trying to munlock.  However, we pre-clear the
> - * mlocked state anyway as we might lose the isolation race and we might
> - * not get another chance to clear PageMlocked.  If we successfully
> - * isolate the page and try_to_munlock() detects other VM_LOCKED vmas
> - * mapping the page, it will restore the PageMlocked state, unless the page
> - * is mapped in a non-linear vma.  So, we go ahead and SetPageMlocked(),
> - * perhaps redundantly.
> - * If we lose the isolation race, and the page is mapped by other VM_LOCKED
> - * vmas, we'll detect this in vmscan--via try_to_munlock() or try_to_unmap()
> - * either of which will restore the PageMlocked state by calling
> - * mlock_vma_page() above, if it can grab the vma's mmap sem.
> + * called from munlock()/munmap() path with page supposedly on the LRU.
> + * When we munlock a page, because the vma where we found the page is being
> + * munlock()ed or munmap()ed, we want to check whether other vmas hold the
> + * page locked so that we can leave it on the unevictable lru list and not
> + * bother vmscan with it.  However, to walk the page's rmap list in
> + * try_to_munlock() we must isolate the page from the LRU.  If some other
> + * task has removed the page from the LRU, we won't be able to do that.
> + * So we clear the PageMlocked as we might not get another chance.  If we
> + * can't isolate the page, we leave it for putback_lru_page() and vmscan
> + * [page_referenced()/try_to_unmap()] to deal with.
>   */
>  static void munlock_vma_page(struct page *page)
>  {
> @@ -123,12 +120,12 @@ static void munlock_vma_page(struct page
>  			putback_lru_page(page);
>  		} else {
>  			/*
> -			 * We lost the race.  let try_to_unmap() deal
> -			 * with it.  At least we get the page state and
> -			 * mlock stats right.  However, page is still on
> -			 * the noreclaim list.  We'll fix that up when
> -			 * the page is eventually freed or we scan the
> -			 * noreclaim list.
> +			 * Some other task has removed the page from the LRU.
> +			 * putback_lru_page() will take care of removing the
> +			 * page from the unevictable list, if necessary.
> +			 * vmscan [page_referenced()] will move the page back
> +			 * to the unevictable list if some other vma has it
> +			 * mlocked.
>  			 */
>  			if (PageUnevictable(page))
>  				count_vm_event(UNEVICTABLE_PGSTRANDED);

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