On Thu, 2008-03-13 at 10:12 +1100, Rusty Russell wrote:

On Thursday 13 March 2008 00:21:33 Martin Schwidefsky wrote:

quoted

@@ -957,6 +975,19 @@ struct page *follow_page(struct vm_area_

 	if (flags & FOLL_GET)
 		get_page(page);
+
+	if (flags & FOLL_GET) {
+		/*
+		 * The page is made stable if a reference is acquired.
+		 * If the caller does not get a reference it implies that
+		 * the caller can deal with page faults in case the page
+		 * is swapped out. In this case the caller can deal with
+		 * discard faults as well.
+		 */
+		if (unlikely(!page_make_stable(page)))
+			goto out_discard;
+	}

Dumb comment: seems like this if could be folded into the one above.

In this patch yes, but a later patch adds a condition:

-       if (flags & FOLL_GET) {
+       if ((flags & FOLL_GET) || (vma->vm_flags & VM_LOCKED)) {

quoted

+ * Attempts to change the state of a page to volatile.
+ * If there is something preventing the state change the page stays
+ * int its current state.

Typo "int its current state".

Fixed.

quoted

 		return NULL;

 	pte = pte_offset_map(pmd, address);
+	ptl = pte_lockptr(mm, pmd);
 	/* Make a quick check before getting the lock */
+#ifndef CONFIG_PAGE_STATES
+	/*
+	 * If the page table lock for this pte is taken we have to
+	 * assume that someone might be mapping the page. To solve
+	 * the race of a page discard vs. mapping the page we have
+	 * to serialize the two operations by taking the lock,
+	 * otherwise we end up with a pte for a page that has been
+	 * removed from page cache by the discard fault handler.
+	 */
+	if (!spin_is_locked(ptl))
+#endif
 	if (!pte_present(*pte)) {
 		pte_unmap(pte);
 		return NULL;
 	}

-	ptl = pte_lockptr(mm, pmd);
 	spin_lock(ptl);
 	if (pte_present(*pte) && page_to_pfn(page) == pte_pfn(*pte)) {
 		*ptlp = ptl;

Did you really mean ifndef here?

That is a major nit. This should be an #ifdef. In previous versions the
complete "if (!pte_present(*pte)) { }" is ifdefed, the later versions
use the !spin_is_locked condition. Only I forgot to invert the #ifndef.
Fixed.

(BTW: I'm just reading through the code, not really understanding it, so
this is not a real review).

I take the small review anytime. Already found one major nit.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.