mtd/fs/jffs2 gc.c,1.120,1.121 readinode.c,1.111,1.112

[David Woodhouse]

Update of /home/cvs/mtd/fs/jffs2
In directory phoenix.infradead.org:/tmp/cvs-serv32633

Modified Files:
	gc.c readinode.c 
Log Message:
Fix clear_inode race in 2.5.

Index: gc.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/gc.c,v
retrieving revision 1.120
retrieving revision 1.121
diff -u -r1.120 -r1.121
--- gc.c	31 Oct 2003 14:55:02 -0000	1.120
+++ gc.c	1 Nov 2003 16:28:13 -0000	1.121
@@ -273,41 +273,37 @@
 		   we're at it, so we set the state accordingly */
 		if (ref_flags(raw) == REF_PRISTINE)
 			ic->state = INO_STATE_GC;
-		else if (ic->nlink) {
+		else {
 			D1(printk(KERN_DEBUG "Ino #%u is absent but node not REF_PRISTINE. Reading.\n", 
 				  inum));
-		} else {
-			D1(printk(KERN_WARNING "Waiting for currently-being-deleted ino #%u to go away.\n",
-				  inum));
-			goto wait;
 		}
 		break;
 
 	case INO_STATE_PRESENT:
-	case INO_STATE_UNCHECKED:
-		/* It's in-core or hasn't been checked. GC must iget() it. */
+		/* It's in-core. GC must iget() it. */
 		break;
 
+	case INO_STATE_UNCHECKED:
 	case INO_STATE_CHECKING:
+	case INO_STATE_GC:
 		/* Should never happen. We should have finished checking
-		   by the time we actually start doing any GC. */
+		   by the time we actually start doing any GC, and since 
+		   we're holding the alloc_sem, no other garbage collection 
+		   can happen.
+		*/
+		printk(KERN_CRIT "Inode #%u already in state %d in jffs2_garbage_collect_pass()!\n",
+		       ic->state);
+		up(&c->alloc_sem);
+		spin_unlock(&c->inocache_lock);
 		BUG();
 
-	
-	case INO_STATE_GC:
-		/* Should never happen. We are holding the alloc_sem, 
-		   no other garbage collection can happen. Note that we
-		   do depend on this later when deciding to do a simple
-		   node copy */
-		BUG();
-			 
 	case INO_STATE_READING:
 		/* Someone's currently trying to read it. We must wait for
 		   them to finish and then go through the full iget() route
 		   to do the GC. However, sometimes read_inode() needs to get
 		   the alloc_sem() (for marking nodes invalid) so we must
 		   drop the alloc_sem before sleeping. */
-	wait:
+
 		up(&c->alloc_sem);
 		D1(printk(KERN_DEBUG "jffs2_garbage_collect_pass() waiting for ino #%u in state %d\n",
 			  inum, ic->state));
@@ -323,7 +319,6 @@
 		   A: Small enough that I don't care :) 
 		*/
 		return 0;
-
 	}
 
 	spin_unlock(&c->inocache_lock);
@@ -368,7 +363,7 @@
 
 
 static int jffs2_garbage_collect_live(struct jffs2_sb_info *c,  struct jffs2_eraseblock *jeb,
-			       struct jffs2_raw_node_ref *raw, struct jffs2_inode_cache *ic)
+				      struct jffs2_raw_node_ref *raw, struct jffs2_inode_cache *ic)
 {
 	struct jffs2_inode_info *f;
 	struct jffs2_node_frag *frag;
@@ -378,9 +373,21 @@
 	struct inode *inode;
 	int ret = 0;
 
-	inode = iget(OFNI_BS_2SFFJ(c), ic->ino);
-	if (!inode)
-		return -ENOMEM;
+	if (!ic->nlink) {
+		/* Only GC from it if it's still actually in the
+		   icache and hasn't been deleted while we were
+		   pondering it. */
+		inode = ifind(OFNI_BS_2SFFJ(c), ic->ino);
+		if (!inode) {
+			D1(printk(KERN_WARNING "ifind failed for ino #%u; inode is deleted. Skipping.\n",
+				  ic->ino));
+			return 0;
+		}
+	else {
+		inode = iget(OFNI_BS_2SFFJ(c), ic->ino);
+		if (!inode)
+			return -ENOMEM;
+	}
 	if (is_bad_inode(inode)) {
 		printk(KERN_NOTICE "Eep. read_inode() failed for ino #%u. nlink %d, state %d\n", ic->ino, ic->nlink, ic->state);
 		/* NB. This will happen again. We need to do something appropriate here. */

Index: readinode.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/readinode.c,v
retrieving revision 1.111
retrieving revision 1.112
diff -u -r1.111 -r1.112
--- readinode.c	31 Oct 2003 14:54:01 -0000	1.111
+++ readinode.c	1 Nov 2003 16:28:13 -0000	1.112
@@ -663,24 +663,10 @@
 void jffs2_do_clear_inode(struct jffs2_sb_info *c, struct jffs2_inode_info *f)
 {
 	struct jffs2_full_dirent *fd, *fds;
-	/* I don't think we care about the potential race due to reading this
-	   without f->sem. It can never get undeleted. */
-	int deleted = f->inocache && !f->inocache->nlink;
-
-	/* If it's a deleted inode, grab the alloc_sem. This prevents
-	   jffs2_garbage_collect_pass() from deciding that it wants to
-	   garbage collect one of the nodes we're just about to mark 
-	   obsolete -- by the time we drop alloc_sem and return, all
-	   the nodes are marked obsolete, and jffs2_g_c_pass() won't
-	   call iget() for the inode in question.
-
-	   We also used to do this to keep the temporary BUG() in 
-	   jffs2_mark_node_obsolete() from triggering. 
-	*/
-	if(deleted)
-		down(&c->alloc_sem);
+	int deleted;
 
 	down(&f->sem);
+	deleted = f->inocache && !f->inocache->nlink;
 
 	if (f->metadata) {
 		if (deleted)
@@ -702,7 +688,4 @@
 		jffs2_set_inocache_state(c, f->inocache, INO_STATE_CHECKEDABSENT);
 
 	up(&f->sem);
-
-	if(deleted)
-		up(&c->alloc_sem);
 }