[Cluster-devel] [RHEL5] GFS2 deadlock and panic fix.

[Russell Cattelan]

This is fixes two problems that were discovered while
reading from file that was being truncated out and 
back to 0.


Header of the patch has the problem and fix summary.

-- 
Russell Cattelan <cattelan at redhat.com>
-------------- next part --------------
Fix two deadlocks here.
readpages could potentially be called with multiple pages
even if the file was truncated down to a stuffed inode.
In that case simply return and let readpage handle
the stuffed case.

The glock in readpage/readpages could deadlock with the glock/page lock 
in truncate path, change the glock to a trylock and return 
AOP_TRUNCATED_PAGE and let the vfs handle the retry.

Signed-Off-By: Russell Cattelan <cattelan at redhat.com>

Index: linux-2.6.18.noarch/fs/gfs2/ops_address.c
===================================================================
--- linux-2.6.18.noarch.orig/fs/gfs2/ops_address.c	2006-12-07 14:56:29.006254392 -0600
+++ linux-2.6.18.noarch/fs/gfs2/ops_address.c	2006-12-07 14:59:06.448164215 -0600
@@ -220,7 +220,7 @@ static int gfs2_readpage(struct file *fi
 	struct gfs2_sbd *sdp = GFS2_SB(page->mapping->host);
 	struct gfs2_file *gf = NULL;
 	struct gfs2_holder gh;
-	int error;
+	int ret;
 	int do_unlock = 0;
 
 	if (likely(file != &gfs2_internal_file_sentinel)) {
@@ -230,34 +230,46 @@ static int gfs2_readpage(struct file *fi
 				/* gfs2_sharewrite_nopage has grabbed the ip->i_gl already */
 				goto skip_lock;
 		}
-		gfs2_holder_init(ip->i_gl, LM_ST_SHARED, GL_ATIME|GL_AOP, &gh);
+		gfs2_holder_init(ip->i_gl, LM_ST_SHARED,
+				 LM_FLAG_TRY_1CB|GL_ATIME|GL_AOP, &gh);
 		do_unlock = 1;
-		error = gfs2_glock_nq_m_atime(1, &gh);
-		if (unlikely(error))
+		ret = gfs2_glock_nq_m_atime(1, &gh);
+		/* if we couldn't get glock just fail readpage
+		 * otherwise we deadlock with other ops (i.e. truncate)
+		 * on page lock
+		 */
+		if (ret == GLR_TRYFAILED) {
+			ret = AOP_TRUNCATED_PAGE;
+			goto out_unlock;
+		}
+		if (unlikely(ret)) {
 			goto out_unlock;
+		}
 	}
 
 skip_lock:
 	if (gfs2_is_stuffed(ip)) {
-		error = stuffed_readpage(ip, page);
+		ret = stuffed_readpage(ip, page);
 		unlock_page(page);
 	} else
-		error = mpage_readpage(page, gfs2_get_block);
+		ret = mpage_readpage(page, gfs2_get_block);
 
 	if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags)))
-		error = -EIO;
+		ret = -EIO;
 
 	if (do_unlock) {
 		gfs2_glock_dq_m(1, &gh);
 		gfs2_holder_uninit(&gh);
 	}
-out:
-	return error;
+	goto out;
+
 out_unlock:
 	unlock_page(page);
 	if (do_unlock)
 		gfs2_holder_uninit(&gh);
-	goto out;
+
+out:
+	return ret;
 }
 
 /**
@@ -283,8 +295,11 @@ static int gfs2_readpages(struct file *f
 	struct gfs2_sbd *sdp = GFS2_SB(inode);
 	struct gfs2_holder gh;
 	unsigned page_idx;
-	int ret;
+	struct page *page = NULL;
+	struct pagevec lru_pvec;
+	int ret = 0;
 	int do_unlock = 0;
+	int do_un_atime = 0;
 
 	if (likely(file != &gfs2_internal_file_sentinel)) {
 		if (file) {
@@ -293,60 +308,54 @@ static int gfs2_readpages(struct file *f
 				goto skip_lock;
 		}
 		gfs2_holder_init(ip->i_gl, LM_ST_SHARED,
-				 LM_FLAG_TRY_1CB|GL_ATIME|GL_AOP, &gh);
+				 LM_FLAG_TRY_0CB|GL_ATIME|GL_AOP, &gh);
 		do_unlock = 1;
+
 		ret = gfs2_glock_nq_m_atime(1, &gh);
-		if (ret == GLR_TRYFAILED)
-			goto out_noerror;
-		if (unlikely(ret))
-			goto out_unlock;
+		if (ret == GLR_TRYFAILED) {
+			ret = 0;
+			goto out_release_pages;
+		}
+		if (unlikely(ret)) {
+			goto out_release_pages;
+		}
+		do_un_atime = 1;
 	}
+
 skip_lock:
 	if (gfs2_is_stuffed(ip)) {
-		struct pagevec lru_pvec;
-		pagevec_init(&lru_pvec, 0);
-		for (page_idx = 0; page_idx < nr_pages; page_idx++) {
-			struct page *page = list_entry(pages->prev, struct page, lru);
-			prefetchw(&page->flags);
-			list_del(&page->lru);
-			if (!add_to_page_cache(page, mapping,
-					       page->index, GFP_KERNEL)) {
-				ret = stuffed_readpage(ip, page);
-				unlock_page(page);
-				if (!pagevec_add(&lru_pvec, page))
-					 __pagevec_lru_add(&lru_pvec);
-			} else {
-				page_cache_release(page);
-			}
-		}
-		pagevec_lru_add(&lru_pvec);
+		/* we are probably here due to a race with a truncate
+		 * simply fail and let readpage handle the stuffed case
+		 */
 		ret = 0;
-	} else {
-		/* What we really want to do .... */
-		ret = mpage_readpages(mapping, pages, nr_pages, gfs2_get_block);
+		goto out_release_pages;
 	}
 
-	if (do_unlock) {
-		gfs2_glock_dq_m(1, &gh);
-		gfs2_holder_uninit(&gh);
-	}
-out:
-	if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags)))
-		ret = -EIO;
-	return ret;
-out_noerror:
-	ret = 0;
-out_unlock:
-	/* unlock all pages, we can't do any I/O right now */
+	/* What we really want to do .... */
+	ret = mpage_readpages(mapping, pages, nr_pages, gfs2_get_block);
+	goto out;
+
+	/* exception handling */
+
+out_release_pages:
+
+	/* release all pages, we can't do any I/O right now */
 	for (page_idx = 0; page_idx < nr_pages; page_idx++) {
-		struct page *page = list_entry(pages->prev, struct page, lru);
+		page = list_entry(pages->prev, struct page, lru);
 		list_del(&page->lru);
-		unlock_page(page);
 		page_cache_release(page);
 	}
+out:
+	if (do_un_atime)
+		gfs2_glock_dq_m(1, &gh);
+
 	if (do_unlock)
 		gfs2_holder_uninit(&gh);
-	goto out;
+
+	if (unlikely(test_bit(SDF_SHUTDOWN, &sdp->sd_flags)))
+		ret = -EIO;
+
+	return ret;
 }
 
 /**
@@ -373,8 +382,15 @@ static int gfs2_prepare_write(struct fil
 	unsigned int write_len = to - from;
 
 
-	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE, GL_ATIME|GL_AOP, &ip->i_gh);
+	gfs2_holder_init(ip->i_gl, LM_ST_EXCLUSIVE,
+				LM_FLAG_TRY_1CB|GL_ATIME|GL_AOP, &ip->i_gh);
 	error = gfs2_glock_nq_m_atime(1, &ip->i_gh);
+
+	if (error == GLR_TRYFAILD) {
+		error = AOP_TRUNCATED_PAGE;
+		goto out_uninit;
+	}
+
 	if (error)
 		goto out_uninit;