All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@zip.com.au>
To: Linus Torvalds <torvalds@transmeta.com>,
	lkml <linux-kernel@vger.kernel.org>
Subject: [patch 5/19] grab_cache_page_nowait deadlock fix
Date: Sun, 16 Jun 2002 23:51:41 -0700	[thread overview]
Message-ID: <3D0D86FD.E9EF1551@zip.com.au> (raw)



- If grab_cache_page_nowait() is to be called while holding a lock on
  a different page, it must perform memory allocations with GFP_NOFS. 
  Otherwise it could come back onto the locked page (if it's dirty) and
  deadlock.

  Also tidy this function up a bit - the checks in there were overly
  paranoid.

- In a few of places, look to see if we can avoid a buslocked cycle
  and dirtying of a cacheline.


--- 2.5.22/mm/filemap.c~grab_cache_page_nowait	Sun Jun 16 22:50:17 2002
+++ 2.5.22-akpm/mm/filemap.c	Sun Jun 16 22:50:17 2002
@@ -445,8 +445,10 @@ int fail_writepage(struct page *page)
 {
 	/* Only activate on memory-pressure, not fsync.. */
 	if (current->flags & PF_MEMALLOC) {
-		activate_page(page);
-		SetPageReferenced(page);
+		if (!PageActive(page))
+			activate_page(page);
+		if (!PageReferenced(page))
+			SetPageReferenced(page);
 	}
 
 	/* Set the page dirty again, unlock */
@@ -868,55 +870,35 @@ struct page *grab_cache_page(struct addr
  * This is intended for speculative data generators, where the data can
  * be regenerated if the page couldn't be grabbed.  This routine should
  * be safe to call while holding the lock for another page.
+ *
+ * Clear __GFP_FS when allocating the page to avoid recursion into the fs
+ * and deadlock against the caller's locked page.
  */
-struct page *grab_cache_page_nowait(struct address_space *mapping, unsigned long index)
+struct page *
+grab_cache_page_nowait(struct address_space *mapping, unsigned long index)
 {
-	struct page *page;
-
-	page = find_get_page(mapping, index);
-
-	if ( page ) {
-		if ( !TestSetPageLocked(page) ) {
-			/* Page found and locked */
-			/* This test is overly paranoid, but what the heck... */
-			if ( unlikely(page->mapping != mapping || page->index != index) ) {
-				/* Someone reallocated this page under us. */
-				unlock_page(page);
-				page_cache_release(page);
-				return NULL;
-			} else {
-				return page;
-			}
-		} else {
-			/* Page locked by someone else */
-			page_cache_release(page);
-			return NULL;
-		}
-	}
-
-	page = page_cache_alloc(mapping);
-	if (unlikely(!page))
-		return NULL;	/* Failed to allocate a page */
+	struct page *page = find_get_page(mapping, index);
 
-	if (unlikely(add_to_page_cache_unique(page, mapping, index))) {
-		/*
-		 * Someone else grabbed the page already, or
-		 * failed to allocate a radix-tree node
-		 */
+	if (page) {
+		if (!TestSetPageLocked(page))
+			return page;
 		page_cache_release(page);
 		return NULL;
 	}
-
+	page = alloc_pages(mapping->gfp_mask & ~__GFP_FS, 0);
+	if (page && add_to_page_cache_unique(page, mapping, index)) {
+		page_cache_release(page);
+		page = NULL;
+	}
 	return page;
 }
 
 /*
  * Mark a page as having seen activity.
  *
- * If it was already so marked, move it
- * to the active queue and drop the referenced
- * bit. Otherwise, just mark it for future
- * action..
+ * inactive,unreferenced	->	inactive,referenced
+ * inactive,referenced		->	active,unreferenced
+ * active,unreferenced		->	active,referenced
  */
 void mark_page_accessed(struct page *page)
 {
@@ -924,10 +906,9 @@ void mark_page_accessed(struct page *pag
 		activate_page(page);
 		ClearPageReferenced(page);
 		return;
+	} else if (!PageReferenced(page)) {
+		SetPageReferenced(page);
 	}
-
-	/* Mark the page referenced, AFTER checking for previous usage.. */
-	SetPageReferenced(page);
 }
 
 /*
@@ -2286,7 +2267,8 @@ generic_file_write(struct file *file, co
 			}
 		}
 		kunmap(page);
-		SetPageReferenced(page);
+		if (!PageReferenced(page))
+			SetPageReferenced(page);
 		unlock_page(page);
 		page_cache_release(page);
 		if (status < 0)

-

                 reply	other threads:[~2002-06-17  7:20 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3D0D86FD.E9EF1551@zip.com.au \
    --to=akpm@zip.com.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.