[PATCH] fs:jffs2: fix deadlock between GC and inode creation

[Lucas Martins Alves]

From: Lucas Martins Alves <lucas.alves@xxxxxxxxxxxxxx>

jffs2_do_new_inode() marked the newly allocated inode cache as
INO_STATE_PRESENT before the inode was actually built. A concurrent
garbage collect pass could pick that inode up and call iget_locked(),
which blocks on the inode still locked by the creating task. That task
in turn waits for alloc_sem, which is held by GC, so both sides deadlock
and neither operation can complete.

Introduce a new INO_STATE_CREATING state and use it in
jffs2_do_new_inode() for the whole window in which the inode is under
construction. The state is moved to INO_STATE_PRESENT only after
d_instantiate_new() in jffs2_create(), jffs2_symlink(), jffs2_mkdir()
and jffs2_mknod().

Teach the garbage collector about the new state. In the CRC check phase
such inodes are not selected, and in both the check and the main inode
selection paths GC now drops alloc_sem, sleeps on inocache_wq and
returns to restart, instead of trying to iget() an inode that is not
ready yet.

Also accept INO_STATE_CREATING as a valid state in
jffs2_wbuf_recover(), so the recovery path no longer hits the BUG() for
inodes being created, and handle it in jffs2_do_read_inode() together
with the other in-core states that must never be read again.

Signed-off-by: Lucas Martins Alves <lucas.alves@xxxxxxxxxxxxxx>
---
 fs/jffs2/dir.c       |  4 ++++
 fs/jffs2/gc.c        | 29 ++++++++++++++++++++++++++++-
 fs/jffs2/nodelist.h  |  1 +
 fs/jffs2/readinode.c |  1 +
 fs/jffs2/wbuf.c      |  3 ++-
 fs/jffs2/write.c     |  6 +++++-
 6 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
index c4088c3b4ac0..3abe28d0cb95 100644
--- a/fs/jffs2/dir.c
+++ b/fs/jffs2/dir.c
@@ -216,6 +216,7 @@ static int jffs2_create(struct mnt_idmap *idmap, struct inode *dir_i,
 		  f->inocache->pino_nlink, inode->i_mapping->nrpages);
 
 	d_instantiate_new(dentry, inode);
+	jffs2_set_inocache_state(c, f->inocache, INO_STATE_PRESENT);
 	return 0;
 
  fail:
@@ -440,6 +441,7 @@ static int jffs2_symlink (struct mnt_idmap *idmap, struct inode *dir_i,
 	jffs2_complete_reservation(c);
 
 	d_instantiate_new(dentry, inode);
+	jffs2_set_inocache_state(c, f->inocache, INO_STATE_PRESENT);
 	return 0;
 
  fail:
@@ -586,6 +588,7 @@ static struct dentry *jffs2_mkdir (struct mnt_idmap *idmap, struct inode *dir_i,
 	jffs2_complete_reservation(c);
 
 	d_instantiate_new(dentry, inode);
+	jffs2_set_inocache_state(c, f->inocache, INO_STATE_PRESENT);
 	return NULL;
 
  fail:
@@ -764,6 +767,7 @@ static int jffs2_mknod (struct mnt_idmap *idmap, struct inode *dir_i,
 	jffs2_complete_reservation(c);
 
 	d_instantiate_new(dentry, inode);
+	jffs2_set_inocache_state(c, f->inocache, INO_STATE_PRESENT);
 	return 0;
 
  fail:
diff --git a/fs/jffs2/gc.c b/fs/jffs2/gc.c
index 1b833bbffcf5..aabf96f7bbd7 100644
--- a/fs/jffs2/gc.c
+++ b/fs/jffs2/gc.c
@@ -161,7 +161,8 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c)
 					continue;
 
 				if (ic->state != INO_STATE_CHECKEDABSENT &&
-				    ic->state != INO_STATE_PRESENT)
+				    ic->state != INO_STATE_PRESENT &&
+				    ic->state != INO_STATE_CREATING)
 					goto got_next; /* with inocache_lock held */
 
 				jffs2_dbg(1, "Skipping ino #%u already checked\n",
@@ -208,6 +209,19 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c)
 			spin_unlock(&c->inocache_lock);
 			BUG();
 
+		case INO_STATE_CREATING:
+			/* We can't process this inode while it is being created
+			   to avoid a deadlock condition because the function
+			   iget_locked() will try to lock the inode to get it.*/
+			jffs2_dbg(1, "Waiting for ino #%u to finish creation\n",
+				  ic->ino);
+			/* We need to come back again for the _same_ inode. We've
+			 made no progress in this case, but that should be OK */
+			c->check_ino = ic->ino;
+			mutex_unlock(&c->alloc_sem);
+			sleep_on_spinunlock(&c->inocache_wq, &c->inocache_lock);
+			return 0;
+
 		case INO_STATE_READING:
 			/* We need to wait for it to finish, lest we move on
 			   and trigger the BUG() above while we haven't yet
@@ -376,6 +390,19 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c)
 		}
 		break;
 
+	case INO_STATE_CREATING:
+		/* We can't process this inode while it is being created
+		   to avoid a deadlock condition because the function
+		   iget_locked() will try to lock the inode to get it.
+		   Hoever, to finish the creation we need to unlock the
+		   alloc_sem() and because we dropped the alloc_sem we must
+		   return to start again from the beginning. */
+		jffs2_dbg(1, "Waiting for ino #%u to finish creation\n",
+			  ic->ino);
+		mutex_unlock(&c->alloc_sem);
+		sleep_on_spinunlock(&c->inocache_wq, &c->inocache_lock);
+		return 0;
+
 	case INO_STATE_PRESENT:
 		/* It's in-core. GC must iget() it. */
 		break;
diff --git a/fs/jffs2/nodelist.h b/fs/jffs2/nodelist.h
index 2e98fa277dab..06fd77b9e3cc 100644
--- a/fs/jffs2/nodelist.h
+++ b/fs/jffs2/nodelist.h
@@ -192,6 +192,7 @@ struct jffs2_inode_cache {
 #define INO_STATE_GC		4	/* GCing a 'pristine' node */
 #define INO_STATE_READING	5	/* In read_inode() */
 #define INO_STATE_CLEARING	6	/* In clear_inode() */
+#define INO_STATE_CREATING	7	/* The inode is locked while it is being created */
 
 #define INO_FLAGS_XATTR_CHECKED	0x01	/* has no duplicate xattr_ref */
 #define INO_FLAGS_IS_DIR	0x02	/* is a directory */
diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c
index 1caabff9dc91..3f1ed7c036f3 100644
--- a/fs/jffs2/readinode.c
+++ b/fs/jffs2/readinode.c
@@ -1352,6 +1352,7 @@ int jffs2_do_read_inode(struct jffs2_sb_info *c, struct jffs2_inode_info *f,
 			goto retry_inocache;
 
 		case INO_STATE_READING:
+		case INO_STATE_CREATING:
 		case INO_STATE_PRESENT:
 			/* Eep. This should never happen. It can
 			happen if Linux calls read_inode() again
diff --git a/fs/jffs2/wbuf.c b/fs/jffs2/wbuf.c
index 8ff7a0b6add2..d99bb2dc0195 100644
--- a/fs/jffs2/wbuf.c
+++ b/fs/jffs2/wbuf.c
@@ -518,7 +518,8 @@ static void jffs2_wbuf_recover(struct jffs2_sb_info *c)
 								      (void *)(buf?:c->wbuf) + (ref_offset(raw) - start));
 			} else if (unlikely(ic->state != INO_STATE_PRESENT &&
 					    ic->state != INO_STATE_CHECKEDABSENT &&
-					    ic->state != INO_STATE_GC)) {
+					    ic->state != INO_STATE_GC &&
+					    ic->state != INO_STATE_CREATING)) {
 				JFFS2_ERROR("Inode #%u is in strange state %d!\n", ic->ino, ic->state);
 				BUG();
 			}
diff --git a/fs/jffs2/write.c b/fs/jffs2/write.c
index cda9a361368e..1dab491b539e 100644
--- a/fs/jffs2/write.c
+++ b/fs/jffs2/write.c
@@ -35,7 +35,11 @@ int jffs2_do_new_inode(struct jffs2_sb_info *c, struct jffs2_inode_info *f,
 	f->inocache = ic;
 	f->inocache->pino_nlink = 1; /* Will be overwritten shortly for directories */
 	f->inocache->nodes = (struct jffs2_raw_node_ref *)f->inocache;
-	f->inocache->state = INO_STATE_PRESENT;
+	f->inocache->state = INO_STATE_CREATING;
+	/* We set the state equal as INO_STATE_CREATING to avoid a deadlock in GC
+	   because the new inode isn´t completely created and GC tries to get the
+	   locked inode. So, the creation function can´t pick up alloc_sem and
+	   deadlock happens between alloc_sem and the specific inode lock. */
 
 	jffs2_add_ino_cache(c, f->inocache);
 	jffs2_dbg(1, "%s(): Assigned ino# %d\n", __func__, f->inocache->ino);
-- 
2.53.0