JFFS2 deadlock with alloc_sem

[Dave Kleikamp]

On Tue, 2007-06-19 at 11:11 -0500, Dave Kleikamp wrote:

> We came up with a fix for the hang were seeing on an older kernel.  It's
> hacky and intrusive, but I wanted to offer it up at least to see if it
> fixes Nathan's hang, and if so, maybe it can lead to some nicer patch
> that may be acceptable.
> 
> This version, against linux-2.6.22-rc5 has been compile-tested only.
> Don't let the _async scare you.  Since jffs2's filler function is
> synchronous, so there's no need to wait for the page to become unlocked
> again.

Monte Copeland pointed out that my last patch could pass the -EBUSY
return code up to a calling thread which would be undesirable.  I was
only considering the garbage-collector thread which ignores the return
code.  Here is the fixed patch.  This has been tested against a
2.6.16-based kernel.

Again, this is a bit intrusive, but it may be a step in the right
direction.

diff -Nurp linux-2.6.22-rc5/fs/jffs2/fs.c linux/fs/jffs2/fs.c
--- linux-2.6.22-rc5/fs/jffs2/fs.c	2007-06-17 08:01:52.000000000 -0500
+++ linux/fs/jffs2/fs.c	2007-06-19 14:28:30.000000000 -0500
@@ -627,8 +627,15 @@ unsigned char *jffs2_gc_fetch_page(struc
 	struct inode *inode = OFNI_EDONI_2SFFJ(f);
 	struct page *pg;
 
-	pg = read_cache_page(inode->i_mapping, offset >> PAGE_CACHE_SHIFT,
-			     (void *)jffs2_do_readpage_unlock, inode);
+	/* read_cache_page_async_trylock will return -EBUSY
+	 * if it is not possible to lock the cache page. If we
+	 * get -EBUSY, then avoid a deadlock between
+	 * cache page locks and f->sem.
+	 */
+	pg = read_cache_page_async_trylock(inode->i_mapping,
+					   offset >> PAGE_CACHE_SHIFT,
+					   (void *)jffs2_do_readpage_unlock,
+					   inode);
 	if (IS_ERR(pg))
 		return (void *)pg;
 
diff -Nurp linux-2.6.22-rc5/fs/jffs2/gc.c linux/fs/jffs2/gc.c
--- linux-2.6.22-rc5/fs/jffs2/gc.c	2007-06-17 08:01:52.000000000 -0500
+++ linux/fs/jffs2/gc.c	2007-06-19 14:30:00.000000000 -0500
@@ -1221,8 +1221,13 @@ static int jffs2_garbage_collect_dnode(s
 	pg_ptr = jffs2_gc_fetch_page(c, f, start, &pg);
 
 	if (IS_ERR(pg_ptr)) {
-		printk(KERN_WARNING "read_cache_page() returned error: %ld\n", PTR_ERR(pg_ptr));
-		return PTR_ERR(pg_ptr);
+		if ( PTR_ERR(pg_ptr) == -EBUSY ) {
+			printk(KERN_WARNING "jffs2_gc_fetch_page() rc -EBUSY. Deadlock avoided.\n" );
+			return 0;
+		} else {
+			printk(KERN_WARNING "jffs2_gc_fetch_page() rc %ld\n", PTR_ERR(pg_ptr));
+			return PTR_ERR(pg_ptr);
+		}
 	}
 
 	offset = start;
diff -Nurp linux-2.6.22-rc5/include/linux/pagemap.h linux/include/linux/pagemap.h
--- linux-2.6.22-rc5/include/linux/pagemap.h	2007-06-17 08:02:01.000000000 -0500
+++ linux/include/linux/pagemap.h	2007-06-19 14:30:12.000000000 -0500
@@ -112,6 +112,10 @@ extern struct page * read_cache_page_asy
 extern struct page * read_cache_page(struct address_space *mapping,
 				unsigned long index, filler_t *filler,
 				void *data);
+extern struct page * read_cache_page_async_trylock(
+				struct address_space *mapping,
+				unsigned long index, filler_t *filler,
+				void *data);
 extern int read_cache_pages(struct address_space *mapping,
 		struct list_head *pages, filler_t *filler, void *data);
 
diff -Nurp linux-2.6.22-rc5/mm/filemap.c linux/mm/filemap.c
--- linux-2.6.22-rc5/mm/filemap.c	2007-06-17 08:02:02.000000000 -0500
+++ linux/mm/filemap.c	2007-06-19 14:31:36.000000000 -0500
@@ -1843,6 +1843,51 @@ struct page *read_cache_page(struct addr
 }
 EXPORT_SYMBOL(read_cache_page);
 
+
+/*
+ * Same as read_cache_page, but abort if the page is locked.
+ */
+struct page *read_cache_page_async_trylock(struct address_space *mapping,
+				unsigned long index,
+				int (*filler)(void *, struct page *),
+				void *data)
+{
+	struct page *page;
+	int err;
+
+retry:
+	page = __read_cache_page(mapping, index, filler, data);
+	if (IS_ERR(page))
+		goto out;
+	mark_page_accessed(page);
+	if (PageUptodate(page))
+		goto out;
+
+	if (TestSetPageLocked(page)) {
+		page_cache_release(page);
+		return ERR_PTR(-EBUSY);
+	}
+
+	if (!page->mapping) {
+		unlock_page(page);
+		page_cache_release(page);
+		goto retry;
+	}
+	if (PageUptodate(page)) {
+		unlock_page(page);
+		goto out;
+	}
+	err = filler(data, page);
+	if (err < 0) {
+		page_cache_release(page);
+		page = ERR_PTR(err);
+	}
+ out:
+	mark_page_accessed(page);
+	return page;
+}
+EXPORT_SYMBOL(read_cache_page_async_trylock);
+
 /*
  * If the page was newly created, increment its refcount and add it to the
  * caller's lru-buffering pagevec.  This function is specifically for

-- 
David Kleikamp
IBM Linux Technology Center