mtd/fs/jffs2 gc.c,1.52.2.6,1.52.2.7 nodelist.h,1.46.2.4,1.46.2.5 readinode.c,1.58.2.7,1.58.2.8

[David Woodhouse]

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

Modified Files:
      Tag: jffs2-2_4-branch
	gc.c nodelist.h readinode.c 
Log Message:
GC and deletion race fix

Index: gc.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/gc.c,v
retrieving revision 1.52.2.6
retrieving revision 1.52.2.7
diff -u -r1.52.2.6 -r1.52.2.7
--- gc.c	2 Nov 2003 13:51:18 -0000	1.52.2.6
+++ gc.c	2 Nov 2003 13:54:20 -0000	1.52.2.7
@@ -109,8 +109,8 @@
 	struct jffs2_node_frag *frag;
 	struct jffs2_full_dnode *fn = NULL;
 	struct jffs2_full_dirent *fd;
+	struct jffs2_inode_cache *ic;
 	__u32 start = 0, end = 0, nrfrags = 0;
-	__u32 inum;
 	struct inode *inode;
 	int ret = 0;
 
@@ -162,16 +162,47 @@
 		goto eraseit_lock;
 	}
 						     
-	inum = jffs2_raw_ref_to_inum(raw);
-	D1(printk(KERN_DEBUG "Inode number is #%u\n", inum));
+	ic = jffs2_raw_ref_to_ic(raw);
+	D1(printk(KERN_DEBUG "Inode number is #%u\n", ic->ino));
 
 	spin_unlock_bh(&c->erase_completion_lock);
 
-	D1(printk(KERN_DEBUG "jffs2_garbage_collect_pass collecting from block @0x%08x. Node @0x%08x, ino #%u\n", jeb->offset, raw->flash_offset&~3, inum));
-
-	inode = iget(OFNI_BS_2SFFJ(c), inum);
+	D1(printk(KERN_DEBUG "jffs2_garbage_collect_pass collecting from block @0x%08x. Node @0x%08x, ino #%u\n", jeb->offset, raw->flash_offset&~3, ic->ino));
+	if (!ic->nlink) {
+		/* The inode has zero nlink but its nodes weren't yet marked
+		   obsolete. This has to be because we're still waiting for 
+		   the final (close() and) iput() to happen.
+
+		   There's a possibility that the final iput() could have 
+		   happened while we were contemplating. In order to ensure
+		   that we don't cause a new read_inode() (which would fail)
+		   for the inode in question, we use ilookup() in this case
+		   instead of iget().
+
+		   The nlink can't _become_ zero at this point because we're 
+		   holding the alloc_sem, and jffs2_do_unlink() would also
+		   need that while decrementing nlink on any inode.
+		*/
+		inode = ilookup(OFNI_BS_2SFFJ(c), ic->ino);
+		if (!inode) {
+			D1(printk(KERN_DEBUG "ilookup() failed for ino #%u; inode is probably deleted.\n",
+				  ic->ino));
+			up(&c->alloc_sem);
+			return 0;
+		}
+	} else {
+		/* Inode has links to it still; they're not going away because
+		   jffs2_do_unlink() would need the alloc_sem and we have it.
+		   Just iget() it, and if read_inode() is necessary that's OK.
+		*/
+		inode = iget(OFNI_BS_2SFFJ(c), ic->ino);
+		if (!inode) {
+			up(&c->alloc_sem);
+			return -ENOMEM;
+		}
+	}
 	if (is_bad_inode(inode)) {
-		printk(KERN_NOTICE "Eep. read_inode() failed for ino #%u\n", inum);
+		printk(KERN_NOTICE "Eep. read_inode() failed for ino #%u\n", ic->ino);
 		/* NB. This will happen again. We need to do something appropriate here. */
 		up(&c->alloc_sem);
 		iput(inode);

Index: nodelist.h
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/nodelist.h,v
retrieving revision 1.46.2.4
retrieving revision 1.46.2.5
diff -u -r1.46.2.4 -r1.46.2.5
--- nodelist.h	24 Feb 2003 21:49:33 -0000	1.46.2.4
+++ nodelist.h	2 Nov 2003 13:54:20 -0000	1.46.2.5
@@ -232,13 +232,13 @@
 
 #define PAD(x) (((x)+3)&~3)
 
-static inline int jffs2_raw_ref_to_inum(struct jffs2_raw_node_ref *raw)
+static inline struct jffs2_inode_cache *jffs2_raw_ref_to_ic(struct jffs2_raw_node_ref *raw)
 {
 	while(raw->next_in_ino) {
 		raw = raw->next_in_ino;
 	}
 
-	return ((struct jffs2_inode_cache *)raw)->ino;
+	return ((struct jffs2_inode_cache *)raw);
 }
 
 /* nodelist.c */

Index: readinode.c
===================================================================
RCS file: /home/cvs/mtd/fs/jffs2/readinode.c,v
retrieving revision 1.58.2.7
retrieving revision 1.58.2.8
diff -u -r1.58.2.7 -r1.58.2.8
--- readinode.c	2 Nov 2003 13:51:18 -0000	1.58.2.7
+++ readinode.c	2 Nov 2003 13:54:20 -0000	1.58.2.8
@@ -468,23 +468,12 @@
 	struct jffs2_node_frag *frag, *frags;
 	struct jffs2_full_dirent *fd, *fds;
 	struct jffs2_inode_info *f = JFFS2_INODE_INFO(inode);
-        /* 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;
+        int deleted;
 
 	D1(printk(KERN_DEBUG "jffs2_clear_inode(): ino #%lu mode %o\n", inode->i_ino, inode->i_mode));
 
-	/* 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.
-	*/
-	if (deleted)
-		down(&c->alloc_sem);
-
 	down(&f->sem);
+	deleted = f->inocache && !f->inocache->nlink;
 
 	frags = f->fraglist;
 	fds = f->dents;
@@ -515,8 +504,5 @@
 	}
 
 	up(&f->sem);
-
-	if(deleted)
-		up(&c->alloc_sem);
 };