JFS: Prevent hang in __lock_metapage

Remove the hold_metapage call from txLog to prevent a hang.
While investigating this one, I audited all functions that held
metapage locks and found several error paths that did not release
them correctly.  These are fixed as well.

fs/jfs/jfs_dmap.c

@@ -1526,6 +1526,7 @@ dbAllocAG(struct bmap * bmp, int agno, s64 nblocks, int l2nb, s64 * results)
 			if (n == 4) {
 				jfs_error(bmp->db_ipbmap->i_sb,
 					  "dbAllocAG: failed descending stree");
+				release_metapage(mp);
 				return -EIO;
 			}
 		}
@@ -3310,7 +3311,7 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno,	s64 nblocks)
 	int i, i0 = TRUE, j, j0 = TRUE, k, n;
 	s64 newsize;
 	s64 p;
-	struct metapage *mp, *l2mp, *l1mp, *l0mp;
+	struct metapage *mp, *l2mp, *l1mp = NULL, *l0mp = NULL;
 	struct dmapctl *l2dcp, *l1dcp, *l0dcp;
 	struct dmap *dp;
 	s8 *l0leaf, *l1leaf, *l2leaf;
@@ -3513,6 +3514,7 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno,	s64 nblocks)
 			 */
 			*l1leaf = dbInitDmapCtl(l0dcp, 0, ++i);
 			write_metapage(l0mp);
+			l0mp = NULL;
 
 			if (nblocks)
 				l1leaf++;	/* continue for next L0 */
@@ -3536,6 +3538,7 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno,	s64 nblocks)
 		 */
 		*l2leaf = dbInitDmapCtl(l1dcp, 1, ++j);
 		write_metapage(l1mp);
+		l1mp = NULL;
 
 		if (nblocks)
 			l2leaf++;	/* continue for next L1 */
@@ -3554,17 +3557,20 @@ int dbExtendFS(struct inode *ipbmap, s64 blkno,	s64 nblocks)
 
 	jfs_error(ipbmap->i_sb,
 		  "dbExtendFS: function has not returned as expected");
+errout:
+	if (l0mp)
+		release_metapage(l0mp);
+	if (l1mp)
+		release_metapage(l1mp);
+	release_metapage(l2mp);
 	return -EIO;
 
 	/*
 	 *      finalize bmap control page
 	 */
-      finalize:
+finalize:
 
 	return 0;
-
-      errout:
-	return -EIO;
 }
 
 

fs/jfs/jfs_dtree.c

@@ -1423,8 +1423,10 @@ static int dtSplitPage(tid_t tid, struct inode *ip, struct dtsplit * split,
 	 */
 	if (nextbn != 0) {
 		DT_GETPAGE(ip, nextbn, mp, PSIZE, p, rc);
-		if (rc)
+		if (rc) {
+			discard_metapage(rmp);
 			return rc;
+		}
 
 		BT_MARK_DIRTY(mp, ip);
 		/*
@@ -2235,8 +2237,10 @@ static int dtDeleteUp(tid_t tid, struct inode *ip,
 	pxdlock->index = 1;
 
 	/* update sibling pointers */
-	if ((rc = dtRelink(tid, ip, fp)))
+	if ((rc = dtRelink(tid, ip, fp))) {
+		BT_PUTPAGE(fmp);
 		return rc;
+	}
 
 	xlen = lengthPXD(&fp->header.self);
 	ip->i_blocks -= LBLK2PBLK(ip->i_sb, xlen);
@@ -2307,8 +2311,10 @@ static int dtDeleteUp(tid_t tid, struct inode *ip,
 				pxdlock->index = 1;
 
 				/* update sibling pointers */
-				if ((rc = dtRelink(tid, ip, p)))
+				if ((rc = dtRelink(tid, ip, p))) {
+					DT_PUTPAGE(mp);
 					return rc;
+				}
 
 				xlen = lengthPXD(&p->header.self);
 				ip->i_blocks -= LBLK2PBLK(ip->i_sb, xlen);
@@ -2621,8 +2627,10 @@ static int dtSearchNode(struct inode *ip, s64 lmxaddr, pxd_t * kpxd,
 		/*
 		 * descend down to leftmost child page
 		 */
-		if (p->header.flag & BT_LEAF)
+		if (p->header.flag & BT_LEAF) {
+			DT_PUTPAGE(mp);
 			return -ESTALE;
+		}
 
 		/* get the leftmost entry */
 		stbl = DT_GETSTBL(p);

fs/jfs/jfs_imap.c

@@ -1546,6 +1546,7 @@ int diAlloc(struct inode *pip, boolean_t dir, struct inode *ip)
 						 0);
 				if (rem >= INOSPEREXT) {
 					IREAD_UNLOCK(ipimap);
+					release_metapage(mp);
 					AG_UNLOCK(imap, agno);
 					jfs_error(ip->i_sb,
 						  "diAlloc: can't find free bit "
@@ -1840,6 +1841,7 @@ static int diAllocIno(struct inomap * imap, int agno, struct inode *ip)
 	 */
 	if (!iagp->nfreeinos) {
 		IREAD_UNLOCK(imap->im_ipimap);
+		release_metapage(mp);
 		jfs_error(ip->i_sb,
 			  "diAllocIno: nfreeinos = 0, but iag on freelist");
 		return -EIO;
@@ -1851,6 +1853,7 @@ static int diAllocIno(struct inomap * imap, int agno, struct inode *ip)
 	for (sword = 0;; sword++) {
 		if (sword >= SMAPSZ) {
 			IREAD_UNLOCK(imap->im_ipimap);
+			release_metapage(mp);
 			jfs_error(ip->i_sb,
 				  "diAllocIno: free inode not found in summary map");
 			return -EIO;
@@ -1866,6 +1869,7 @@ static int diAllocIno(struct inomap * imap, int agno, struct inode *ip)
 	rem = diFindFree(le32_to_cpu(iagp->inosmap[sword]), 0);
 	if (rem >= EXTSPERSUM) {
 		IREAD_UNLOCK(imap->im_ipimap);
+		release_metapage(mp);
 		jfs_error(ip->i_sb, "diAllocIno: no free extent found");
 		return -EIO;
 	}
@@ -1876,6 +1880,7 @@ static int diAllocIno(struct inomap * imap, int agno, struct inode *ip)
 	rem = diFindFree(le32_to_cpu(iagp->wmap[extno]), 0);
 	if (rem >= INOSPEREXT) {
 		IREAD_UNLOCK(imap->im_ipimap);
+		release_metapage(mp);
 		jfs_error(ip->i_sb, "diAllocIno: free inode not found");
 		return -EIO;
 	}
@@ -2839,12 +2844,14 @@ diUpdatePMap(struct inode *ipimap,
 		 * and should be free in persistent map;
 		 */
 		if (!(le32_to_cpu(iagp->wmap[extno]) & mask)) {
+			release_metapage(mp);
 			jfs_error(ipimap->i_sb,
 				  "diUpdatePMap: the inode is not allocated in "
 				  "the working map");
 			return -EIO;
 		}
 		if ((le32_to_cpu(iagp->pmap[extno]) & mask) != 0) {
+			release_metapage(mp);
 			jfs_error(ipimap->i_sb,
 				  "diUpdatePMap: the inode is not free in the "
 				  "persistent map");

fs/jfs/jfs_txnmgr.c

@@ -1356,9 +1356,6 @@ static int txLog(struct jfs_log * log, struct tblock * tblk, struct commit * cd)
 		lrd->log.redopage.fileset = cpu_to_le32(JFS_IP(ip)->fileset);
 		lrd->log.redopage.inode = cpu_to_le32(ip->i_ino);
 
-		if (tlck->mp)
-			hold_metapage(tlck->mp, 0);
-
 		/* write log record of page from the tlock */
 		switch (tlck->type & tlckTYPE) {
 		case tlckXTREE:
@@ -1384,8 +1381,6 @@ static int txLog(struct jfs_log * log, struct tblock * tblk, struct commit * cd)
 		default:
 			jfs_err("UFO tlock:0x%p", tlck);
 		}
-		if (tlck->mp)
-			release_metapage(tlck->mp);
 	}
 
 	return rc;
@@ -1535,6 +1530,7 @@ static int dataLog(struct jfs_log * log, struct tblock * tblk, struct lrd * lrd,
 		 * the last entry, so don't bother logging this
 		 */
 		mp->lid = 0;
+		hold_metapage(mp, 0);
 		atomic_dec(&mp->nohomeok);
 		discard_metapage(mp);
 		tlck->mp = 0;

fs/jfs/jfs_xtree.c

@@ -1032,11 +1032,11 @@ xtSplitUp(tid_t tid,
 	rc = (sp->header.flag & BT_ROOT) ?
 	    xtSplitRoot(tid, ip, split, &rmp) :
 	    xtSplitPage(tid, ip, split, &rmp, &rbn);
-	if (rc)
-		return -EIO;
 
 	XT_PUTPAGE(smp);
 
+	if (rc)
+		return -EIO;
 	/*
 	 * propagate up the router entry for the leaf page just split
 	 *
@@ -1611,14 +1611,16 @@ int xtExtend(tid_t tid,		/* transaction id */
 	/* there must exist extent to be extended */
 	if ((rc = xtSearch(ip, xoff - 1, &cmp, &btstack, XT_INSERT)))
 		return rc;
+
+	/* retrieve search result */
+	XT_GETSEARCH(ip, btstack.top, bn, mp, p, index);
+
 	if (cmp != 0) {
+		XT_PUTPAGE(mp);
 		jfs_error(ip->i_sb, "xtExtend: xtSearch did not find extent");
 		return -EIO;
 	}
 
-	/* retrieve search result */
-	XT_GETSEARCH(ip, btstack.top, bn, mp, p, index);
-
 	/* extension must be contiguous */
 	xad = &p->xad[index];
 	if ((offsetXAD(xad) + lengthXAD(xad)) != xoff) {
@@ -1677,25 +1679,27 @@ int xtExtend(tid_t tid,		/* transaction id */
 		 * resides on the new child page;
 		 */
 		if (rootsplit) {
-			if (p->header.nextindex ==
-			    cpu_to_le16(XTENTRYSTART + 1)) {
-				xad = &p->xad[XTENTRYSTART];
-				bn = addressXAD(xad);
-
-				/* get new child page */
-				XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
-
-				BT_MARK_DIRTY(mp, ip);
-				if (!test_cflag(COMMIT_Nolink, ip)) {
-					tlck = txLock(tid, ip, mp,
-						      tlckXTREE |
-						      tlckGROW);
-					xtlck = (struct xtlock *) & tlck->lock;
-				}
+			ASSERT(p->header.nextindex ==
+			       cpu_to_le16(XTENTRYSTART + 1));
+			xad = &p->xad[XTENTRYSTART];
+			bn = addressXAD(xad);
+
+			/* get new child page */
+			XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
+			if (rc)
+				return rc;
+
+			BT_MARK_DIRTY(mp, ip);
+			if (!test_cflag(COMMIT_Nolink, ip)) {
+				tlck = txLock(tid, ip, mp, tlckXTREE|tlckGROW);
+				xtlck = (struct xtlock *) & tlck->lock;
 			}
-		} else
+		} else {
 			/* get back old page */
 			XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
+			if (rc)
+				return rc;
+		}
 	}
 	/*
 	 *      insert the new entry into the leaf page
@@ -1778,14 +1782,16 @@ printf("xtTailgate: nxoff:0x%lx nxlen:0x%x nxaddr:0x%lx\n",
 	/* there must exist extent to be tailgated */
 	if ((rc = xtSearch(ip, xoff, &cmp, &btstack, XT_INSERT)))
 		return rc;
+
+	/* retrieve search result */
+	XT_GETSEARCH(ip, btstack.top, bn, mp, p, index);
+
 	if (cmp != 0) {
+		XT_PUTPAGE(mp);
 		jfs_error(ip->i_sb, "xtTailgate: couldn't find extent");
 		return -EIO;
 	}
 
-	/* retrieve search result */
-	XT_GETSEARCH(ip, btstack.top, bn, mp, p, index);
-
 	/* entry found must be last entry */
 	nextindex = le16_to_cpu(p->header.nextindex);
 	if (index != nextindex - 1) {
@@ -1843,25 +1849,27 @@ printf("xtTailgate: xoff:0x%lx xlen:0x%x xaddr:0x%lx\n",
 		 * resides on the new child page;
 		 */
 		if (rootsplit) {
-			if (p->header.nextindex ==
-			    cpu_to_le16(XTENTRYSTART + 1)) {
-				xad = &p->xad[XTENTRYSTART];
-				bn = addressXAD(xad);
-
-				/* get new child page */
-				XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
-
-				BT_MARK_DIRTY(mp, ip);
-				if (!test_cflag(COMMIT_Nolink, ip)) {
-					tlck = txLock(tid, ip, mp,
-						      tlckXTREE |
-						      tlckGROW);
-					xtlck = (struct xtlock *) & tlck->lock;
-				}
+			ASSERT(p->header.nextindex ==
+			       cpu_to_le16(XTENTRYSTART + 1));
+			xad = &p->xad[XTENTRYSTART];
+			bn = addressXAD(xad);
+
+			/* get new child page */
+			XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
+			if (rc)
+				return rc;
+
+			BT_MARK_DIRTY(mp, ip);
+			if (!test_cflag(COMMIT_Nolink, ip)) {
+				tlck = txLock(tid, ip, mp, tlckXTREE|tlckGROW);
+				xtlck = (struct xtlock *) & tlck->lock;
 			}
-		} else
+		} else {
 			/* get back old page */
 			XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
+			if (rc)
+				return rc;
+		}
 	}
 	/*
 	 *      insert the new entry into the leaf page
@@ -1960,14 +1968,15 @@ int xtUpdate(tid_t tid, struct inode *ip, xad_t * nxad)
 	if ((rc = xtSearch(ip, nxoff, &cmp, &btstack, XT_INSERT)))
 		return rc;
 
+	/* retrieve search result */
+	XT_GETSEARCH(ip, btstack.top, bn, mp, p, index0);
+
 	if (cmp != 0) {
+		XT_PUTPAGE(mp);
 		jfs_error(ip->i_sb, "xtUpdate: Could not find extent");
 		return -EIO;
 	}
 
-	/* retrieve search result */
-	XT_GETSEARCH(ip, btstack.top, bn, mp, p, index0);
-
 	BT_MARK_DIRTY(mp, ip);
 	/*
 	 * acquire tlock of the leaf page containing original entry
@@ -2175,25 +2184,26 @@ int xtUpdate(tid_t tid, struct inode *ip, xad_t * nxad)
 		 * resides on the new child page;
 		 */
 		if (rootsplit) {
-			if (p->header.nextindex ==
-			    cpu_to_le16(XTENTRYSTART + 1)) {
-				xad = &p->xad[XTENTRYSTART];
-				bn = addressXAD(xad);
-
-				/* get new child page */
-				XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
-
-				BT_MARK_DIRTY(mp, ip);
-				if (!test_cflag(COMMIT_Nolink, ip)) {
-					tlck = txLock(tid, ip, mp,
-						      tlckXTREE |
-						      tlckGROW);
-					xtlck = (struct xtlock *) & tlck->lock;
-				}
+			ASSERT(p->header.nextindex ==
+			       cpu_to_le16(XTENTRYSTART + 1));
+			xad = &p->xad[XTENTRYSTART];
+			bn = addressXAD(xad);
+
+			/* get new child page */
+			XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
+			if (rc)
+				return rc;
+
+			BT_MARK_DIRTY(mp, ip);
+			if (!test_cflag(COMMIT_Nolink, ip)) {
+				tlck = txLock(tid, ip, mp, tlckXTREE|tlckGROW);
+				xtlck = (struct xtlock *) & tlck->lock;
 			}
 		} else {
 			/* get back old page */
 			XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
+			if (rc)
+				return rc;
 
 			/* is nXAD on new page ? */
 			if (newindex >
@@ -2247,6 +2257,8 @@ int xtUpdate(tid_t tid, struct inode *ip, xad_t * nxad)
 
 		/* get new right page */
 		XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
+		if (rc)
+			return rc;
 
 		BT_MARK_DIRTY(mp, ip);
 		if (!test_cflag(COMMIT_Nolink, ip)) {
@@ -2270,13 +2282,16 @@ int xtUpdate(tid_t tid, struct inode *ip, xad_t * nxad)
 
 		if ((rc = xtSearch(ip, nxoff, &cmp, &btstack, XT_INSERT)))
 			return rc;
+
+		/* retrieve search result */
+		XT_GETSEARCH(ip, btstack.top, bn, mp, p, index0);
+
 		if (cmp != 0) {
+			XT_PUTPAGE(mp);
 			jfs_error(ip->i_sb, "xtUpdate: xtSearch failed");
 			return -EIO;
 		}
 
-		/* retrieve search result */
-		XT_GETSEARCH(ip, btstack.top, bn, mp, p, index0);
 		if (index0 != index) {
 			XT_PUTPAGE(mp);
 			jfs_error(ip->i_sb,
@@ -2325,25 +2340,27 @@ printf("xtUpdate.updateLeft.split p:0x%p\n", p);
 		 * resides on the new child page;
 		 */
 		if (rootsplit) {
-			if (p->header.nextindex ==
-			    cpu_to_le16(XTENTRYSTART + 1)) {
-				xad = &p->xad[XTENTRYSTART];
-				bn = addressXAD(xad);
-
-				/* get new child page */
-				XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
-
-				BT_MARK_DIRTY(mp, ip);
-				if (!test_cflag(COMMIT_Nolink, ip)) {
-					tlck = txLock(tid, ip, mp,
-						      tlckXTREE |
-						      tlckGROW);
-					xtlck = (struct xtlock *) & tlck->lock;
-				}
+			ASSERT(p->header.nextindex ==
+			       cpu_to_le16(XTENTRYSTART + 1));
+			xad = &p->xad[XTENTRYSTART];
+			bn = addressXAD(xad);
+
+			/* get new child page */
+			XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
+			if (rc)
+				return rc;
+
+			BT_MARK_DIRTY(mp, ip);
+			if (!test_cflag(COMMIT_Nolink, ip)) {
+				tlck = txLock(tid, ip, mp, tlckXTREE|tlckGROW);
+				xtlck = (struct xtlock *) & tlck->lock;
 			}
-		} else
+		} else {
 			/* get back old page */
 			XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
+			if (rc)
+				return rc;
+		}
 	} else {
 		/* if insert into middle, shift right remaining entries */
 		if (newindex < nextindex)
@@ -2660,8 +2677,10 @@ xtDeleteUp(tid_t tid, struct inode *ip,
 	/*
 	 * free non-root leaf page
 	 */
-	if ((rc = xtRelink(tid, ip, fp)))
+	if ((rc = xtRelink(tid, ip, fp))) {
+		XT_PUTPAGE(fmp);
 		return rc;
+	}
 
 	xaddr = addressPXD(&fp->header.self);
 	xlen = lengthPXD(&fp->header.self);
@@ -2703,7 +2722,7 @@ xtDeleteUp(tid_t tid, struct inode *ip,
 				p->header.nextindex =
 				    cpu_to_le16(XTENTRYSTART);
 
-				/* XT_PUTPAGE(fmp); */
+				/* XT_PUTPAGE(mp); */
 
 				break;
 			} else {
@@ -2717,7 +2736,7 @@ xtDeleteUp(tid_t tid, struct inode *ip,
 				       (s64) JFS_SBI(ip->i_sb)->nbperpage);
 
 				/* unpin/free the buffer page */
-				discard_metapage(fmp);
+				discard_metapage(mp);
 
 				/* propagate up */
 				continue;
@@ -2826,14 +2845,15 @@ xtRelocate(tid_t tid, struct inode * ip, xad_t * oxad,	/* old XAD */
 		rc = xtSearch(ip, xoff, &cmp, &btstack, 0);
 		if (rc)
 			return rc;
+
+		/* retrieve search result */
+		XT_GETSEARCH(ip, btstack.top, bn, pmp, pp, index);
+
 		if (cmp) {
 			XT_PUTPAGE(pmp);
 			return -ESTALE;
 		}
 
-		/* retrieve search result */
-		XT_GETSEARCH(ip, btstack.top, bn, pmp, pp, index);
-
 		/* validate for exact match with a single entry */
 		xad = &pp->xad[index];
 		if (addressXAD(xad) != oxaddr || lengthXAD(xad) != xlen) {
@@ -2846,14 +2866,15 @@ xtRelocate(tid_t tid, struct inode * ip, xad_t * oxad,	/* old XAD */
 		rc = xtSearchNode(ip, oxad, &cmp, &btstack, 0);
 		if (rc)
 			return rc;
+
+		/* retrieve search result */
+		XT_GETSEARCH(ip, btstack.top, bn, pmp, pp, index);
+
 		if (cmp) {
 			XT_PUTPAGE(pmp);
 			return -ESTALE;
 		}
 
-		/* retrieve search result */
-		XT_GETSEARCH(ip, btstack.top, bn, pmp, pp, index);
-
 		/* xtSearchNode() validated for exact match with a single entry
 		 */
 		xad = &pp->xad[index];
@@ -2927,7 +2948,9 @@ xtRelocate(tid_t tid, struct inode * ip, xad_t * oxad,	/* old XAD */
 		}
 
 		/* get back parent page */
-		rc = xtSearch(ip, xoff, &cmp, &btstack, 0);
+		if ((rc = xtSearch(ip, xoff, &cmp, &btstack, 0)))
+			return rc;
+
 		XT_GETSEARCH(ip, btstack.top, bn, pmp, pp, index);
 		jfs_info("xtRelocate: target data extent relocated.");
 	} else {		/* (xtype  == XTPAGE) */
@@ -3150,8 +3173,10 @@ static int xtSearchNode(struct inode *ip, xad_t * xad,	/* required XAD entry */
 		XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
 		if (rc)
 			return rc;
-		if (p->header.flag & BT_LEAF)
+		if (p->header.flag & BT_LEAF) {
+			XT_PUTPAGE(mp);
 			return -ESTALE;
+		}
 
 		lim = le16_to_cpu(p->header.nextindex) - XTENTRYSTART;
 
@@ -3949,12 +3974,15 @@ s64 xtTruncate_pmap(tid_t tid, struct inode *ip, s64 committed_size)
 		rc = xtSearch(ip, xoff, &cmp, &btstack, 0);
 		if (rc)
 			return rc;
+
+		XT_GETSEARCH(ip, btstack.top, bn, mp, p, index);
+
 		if (cmp != 0) {
+			XT_PUTPAGE(mp);
 			jfs_error(ip->i_sb,
 				  "xtTruncate_pmap: did not find extent");
 			return -EIO;
 		}
-		XT_GETSEARCH(ip, btstack.top, bn, mp, p, index);
 	} else {
 		/*
 		 * start with root
@@ -4202,17 +4230,10 @@ int xtDisplayTree(struct inode *ip)
 int xtDisplayPage(struct inode *ip, s64 bn, xtpage_t * p)
 {
 	int rc = 0;
-	struct metapage *mp;
 	xad_t *xad;
 	s64 xaddr, xoff;
 	int xlen, i, j;
 
-	if (p == NULL) {
-		XT_GETPAGE(ip, bn, mp, PSIZE, p, rc);
-		if (rc)
-			return rc;
-	}
-
 	/* display page control */
 	printf("bn:0x%lx flag:0x%x nextindex:%d\n",
 	       (ulong) bn, p->header.flag,