[PATCH 1/2] jffs2: solving deadlock between GC and new inodes

[PATCH 1/2] jffs2: solving deadlock between GC and new inodes

[Wagner Popov dos Santos]

Correct AB-BA deadlock between the GC kthread and a process
that is creating a new file, symlink, and etc.

Create a new JFFS2 inode state to signalizing that the inode
can't be acquired by the GC because is being created. When this
state happens, the GC releases the alloc_sem, sleep, and then
tries again.

To create a new file, the JFSS2 request a new locked inode
to the Linux's FS layer and then insert inside of some JFFS2's
Erase Block. To finish the creation, the process needs to
acquire the alloc_sem semaphore. Just at the end that the
inode is released by the JFFS2. If the GC starts to process an
Erase Block that contains an inode that is being created, can
occur an AB-BA deadlock if the GC acquires the alloc_sem before
the end of the inode's creation because the CG needs to acquire
all inodes inside the EB to finish the iteration.

This deadlock can occur more frequently with partition mounted
with sync flag (not usual).

Signed-off-by: Wagner Popov dos Santos <wpopov@gmail.com>
---
 fs/jffs2/dir.c       |  4 ++++
 fs/jffs2/gc.c        | 10 ++++++++++
 fs/jffs2/nodelist.h  |  1 +
 fs/jffs2/readinode.c |  1 +
 fs/jffs2/wbuf.c      |  3 ++-
 fs/jffs2/write.c     |  6 +++++-
 6 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/fs/jffs2/dir.c b/fs/jffs2/dir.c
index f20cff1194bb..57299cd8a0d7 100644
--- a/fs/jffs2/dir.c
+++ b/fs/jffs2/dir.c
@@ -210,6 +210,7 @@ static int jffs2_create(struct inode *dir_i, struct dentry *dentry,
 		  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:
@@ -430,6 +431,7 @@ static int jffs2_symlink (struct inode *dir_i, struct dentry *dentry, const char
 	jffs2_complete_reservation(c);
 
 	d_instantiate_new(dentry, inode);
+	jffs2_set_inocache_state(c, f->inocache, INO_STATE_PRESENT);
 	return 0;
 
  fail:
@@ -574,6 +576,7 @@ static int jffs2_mkdir (struct inode *dir_i, struct dentry *dentry, umode_t mode
 	jffs2_complete_reservation(c);
 
 	d_instantiate_new(dentry, inode);
+	jffs2_set_inocache_state(c, f->inocache, INO_STATE_PRESENT);
 	return 0;
 
  fail:
@@ -745,6 +748,7 @@ static int jffs2_mknod (struct inode *dir_i, struct dentry *dentry, umode_t mode
 	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 9ed0f26cf023..72090843fa19 100644
--- a/fs/jffs2/gc.c
+++ b/fs/jffs2/gc.c
@@ -208,6 +208,8 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c)
 			spin_unlock(&c->inocache_lock);
 			BUG();
 
+			/* fall through */
+		case INO_STATE_CREATING:
 		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
@@ -394,6 +396,14 @@ int jffs2_garbage_collect_pass(struct jffs2_sb_info *c)
 		spin_unlock(&c->inocache_lock);
 		BUG();
 
+		/* fall through */
+	case INO_STATE_CREATING:
+		/* We can't process this inode while it is being created
+		 * to avoid a deadlock condition the inode is locked.
+		 * However, 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.
+		 */
 	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
diff --git a/fs/jffs2/nodelist.h b/fs/jffs2/nodelist.h
index 0637271f3770..0033eeee27e4 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	/* Inode locked! GC can't touch */
 
 #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 389ea53ea487..3a841645bc86 100644
--- a/fs/jffs2/readinode.c
+++ b/fs/jffs2/readinode.c
@@ -1336,6 +1336,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 c6821a509481..860fe878c324 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..ec48947ac916 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;
+	/* The initial state can't be INO_STATE_PRESENT to avoid a deadlock in
+	 * GC because the new inode is still locked and the GC needs to lock
+	 * the inode to get it.
+	 */
 
 	jffs2_add_ino_cache(c, f->inocache);
 	jffs2_dbg(1, "%s(): Assigned ino# %d\n", __func__, f->inocache->ino);
-- 
2.20.1

[PATCH 2/2] jffs2: solving deadlock on sync function

[Wagner Popov dos Santos]

Correcting AB-BA deadlock in jffs2_fsync() involving alloc_sem
semaphore and inodes.

The function jffs2_fsync() can't lock the inode because some
process, or even the same process, that call the CG will acquire
alloc_sem semaphore and will try to acquire the inode if it is
inside the Erase Block that is marked to be processed.

Fixes: 02c24a82187d ("fs: push i_mutex and filemap_write_and_wait down into ->fsync() handlers")

Signed-off-by: Wagner Popov dos Santos <wpopov@gmail.com>
---
 fs/jffs2/file.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c
index 7d8654a1472e..7f139704cb8d 100644
--- a/fs/jffs2/file.c
+++ b/fs/jffs2/file.c
@@ -39,10 +39,14 @@ int jffs2_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
 	if (ret)
 		return ret;
 
-	inode_lock(inode);
-	/* Trigger GC to flush any pending writes for this inode */
+	/* Trigger GC to flush any pending writes for this inode
+	 *
+	 * We need to leave the inode unlocked to avoid a deadlock condition
+	 * because the function jffs2_garbage_collect_pass() can try to lock
+	 * the same inode if it is inside the erase block that GC is
+	 * processing.
+	 */
 	jffs2_flush_wbuf_gc(c, inode->i_ino);
-	inode_unlock(inode);
 
 	return 0;
 }
-- 
2.20.1