fs: fix up error handlers for insert_inode_locked

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set. However, error handlers still call unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

Patches follow to skip the call to unlock_new_inode() if insert_inode_locked() fails.

-Eric

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set. However, the error handler still calls unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

(We also were doing dquot_drop, etc before we had initialized the inode, that gets skipped as well)

---

index c4e81df..769351a 100644
@@ -574,7 +574,7 @@ got:
 	spin_unlock(&sbi->s_next_gen_lock);
 	if (insert_inode_locked(inode) < 0) {
 		err = -EINVAL;
-		goto fail_drop;
+		goto fail_put;
 	}
 
 	dquot_initialize(inode);

@@ -601,8 +601,9 @@ fail_free_drop:
 fail_drop:
 	dquot_drop(inode);
 	inode->i_flags |= S_NOQUOTA;
-	clear_nlink(inode);
 	unlock_new_inode(inode);
+fail_put:
+	clear_nlink(inode);
 	iput(inode);
 	return ERR_PTR(err);
 

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set on failure. However, the error handler still calls unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

(We also were doing dquot_drop, etc before we had initialized the quota, that gets skipped as well)

---

V2: don't rearrange clear_nlink, and edit commit message.

index c4e81df..06cbc22 100644
@@ -574,7 +574,7 @@ got:
 	spin_unlock(&sbi->s_next_gen_lock);
 	if (insert_inode_locked(inode) < 0) {
 		err = -EINVAL;
-		goto fail_drop;
+		goto fail_put;
 	}
 
 	dquot_initialize(inode);

@@ -603,6 +603,7 @@ fail_drop:
 	inode->i_flags |= S_NOQUOTA;
 	clear_nlink(inode);
 	unlock_new_inode(inode);
+fail_put:
 	iput(inode);
 	return ERR_PTR(err);
 

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set. However, the error handler still calls unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

(We also were doing dquot_drop, etc before we had initialized the quota, that gets skipped as well)

---

index 5c866e0..7d0493b 100644
@@ -526,7 +526,7 @@ got:
 		handle->h_sync = 1;
 	if (insert_inode_locked(inode) < 0) {
 		err = -EINVAL;
-		goto fail_drop;
+		goto fail_put;
 	}
 	spin_lock(&sbi->s_next_gen_lock);
 	inode->i_generation = sbi->s_next_generation++;

@@ -582,8 +582,9 @@ fail_free_drop:
 fail_drop:
 	dquot_drop(inode);
 	inode->i_flags |= S_NOQUOTA;
-	clear_nlink(inode);
 	unlock_new_inode(inode);
+fail_put:
+	clear_nlink(inode);
 	iput(inode);
 	brelse(bitmap_bh);
 	return ERR_PTR(err);

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set on failure. However, the error handler still calls unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

(We also were doing dquot_drop, etc before we had initialized the quota, that gets skipped as well)

---

V2: don't rearrange clear_nlink, and edit commit message.

index 5c866e0..5563d99 100644
@@ -526,7 +526,7 @@ got:
 		handle->h_sync = 1;
 	if (insert_inode_locked(inode) < 0) {
 		err = -EINVAL;
-		goto fail_drop;
+		goto fail_put;
 	}
 	spin_lock(&sbi->s_next_gen_lock);
 	inode->i_generation = sbi->s_next_generation++;

@@ -584,6 +584,7 @@ fail_drop:
 	inode->i_flags |= S_NOQUOTA;
 	clear_nlink(inode);
 	unlock_new_inode(inode);
+fail_put:
 	iput(inode);
 	brelse(bitmap_bh);
 	return ERR_PTR(err);

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set on failure. However, the error handler still calls unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

(We also were doing dquot_drop, etc before we had initialized the quota, that gets skipped as well)

---

index 00beb4f..f00c19a 100644
@@ -886,7 +886,7 @@ got:
 		ext4_handle_sync(handle);
 	if (insert_inode_locked(inode) < 0) {
 		err = -EINVAL;
-		goto fail_drop;
+		goto fail_put;
 	}
 	spin_lock(&sbi->s_next_gen_lock);
 	inode->i_generation = sbi->s_next_generation++;

@@ -948,8 +948,9 @@ fail_free_drop:
 fail_drop:
 	dquot_drop(inode);
 	inode->i_flags |= S_NOQUOTA;
-	clear_nlink(inode);
 	unlock_new_inode(inode);
+fail_put:
+	clear_nlink(inode);
 	iput(inode);
 	brelse(inode_bitmap_bh);
 	return ERR_PTR(err);

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set on failure. However, the error handler still calls unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

(We also were doing dquot_drop, etc before we had initialized the quota, that gets skipped as well)

---

V2: don't rearrange clear_nlink, and edit commit message.

index 00beb4f..1e0c3bf 100644
@@ -886,7 +886,7 @@ got:
 		ext4_handle_sync(handle);
 	if (insert_inode_locked(inode) < 0) {
 		err = -EINVAL;
-		goto fail_drop;
+		goto fail_put;
 	}
 	spin_lock(&sbi->s_next_gen_lock);
 	inode->i_generation = sbi->s_next_generation++;

@@ -950,6 +950,7 @@ fail_drop:
 	inode->i_flags |= S_NOQUOTA;
 	clear_nlink(inode);
 	unlock_new_inode(inode);
+fail_put:
 	iput(inode);
 	brelse(inode_bitmap_bh);
 	return ERR_PTR(err);

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set on failure. However, the error handler still calls unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

We can just drop the unlock_new_inode() if insert_inode_locked() fails here.

---

index 4b8afe3..2e01238 100644
@@ -466,7 +466,6 @@ struct inode *jffs2_new_inode (struct inode *dir_i, umode_t mode, struct jffs2_r
 
 	if (insert_inode_locked(inode) < 0) {
 		make_bad_inode(inode);
-		unlock_new_inode(inode);
 		iput(inode);
 		return ERR_PTR(-EINVAL);
 	}

Pushed to l2-mtd-2.6.git tree, thanks!

Artem.

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set on failure. However, the error handler still calls unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

---

index c1a3e60..7f464c5 100644
@@ -95,7 +95,7 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
 
 	if (insert_inode_locked(inode) < 0) {
 		rc = -EINVAL;
-		goto fail_unlock;
+		goto fail_put;
 	}
 
 	inode_init_owner(inode, parent, mode);

@@ -156,7 +156,6 @@ struct inode *ialloc(struct inode *parent, umode_t mode)
 fail_drop:
 	dquot_drop(inode);
 	inode->i_flags |= S_NOQUOTA;
-fail_unlock:
 	clear_nlink(inode);
 	unlock_new_inode(inode);
 fail_put:

after 250df6ed274d767da844a5d9f05720b804240197
(fs: protect inode->i_state with inode->i_lock), insert_inode_locked() no longer returns the inode with I_NEW set on failure. However, the error handler still calls unlock_new_inode() on failure, which does a WARN_ON if I_NEW is not set, so any failure spews a lot of warnings.

---

(I had a hard time unraveling the gotos, so punted with an "inode_locked" var... prettier replacements are fine with me!)

index 950f13a..b0cab7c 100644
@@ -1779,6 +1779,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
 	struct cpu_key key;
 	struct item_head ih;
 	struct stat_data sd;
+	int inode_locked = 0;
 	int retval;
 	int err;
 

@@ -1816,6 +1817,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
 		err = -EINVAL;
 		goto out_bad_inode;
 	}
+	inode_locked = 1;
 	if (old_format_only(sb))
 		/* not a perfect generation count, as object ids can be reused, but
 		 ** this is as good as reiserfs can do right now.

@@ -1989,7 +1991,8 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
       out_inserted_sd:
 	clear_nlink(inode);
 	th->t_trans_id = 0;	/* so the caller can't use this handle later */
-	unlock_new_inode(inode); /* OK to do even if we hadn't locked it */
+	if (inode_locked)
+		unlock_new_inode(inode);
 	iput(inode);
 	return err;
 }

Whoops that should say "with I_NEW set on failure"

Will resend for the fs patches I copied that text to :(

-Eric