[PATCH 2/6] quota: Add proper error handling on quota initialization.

[Dmitry Monakhov <dmonakhov@openvz.org>]

Currently dqget_initialize() is a black-box so IO errors are simply
ignored. In order to pass to the caller real error codes quota
interface must being redesigned in following way.

- return real error code from dqget()
- return real error code from dquot_initialize()

Now filesystem it is filesystem's responsibility to dquot_initialize()
return value.

Signed-off-by: Dmitry Monakhov <dmonakhov@openvz.org>
---
 fs/ocfs2/file.c          |   18 ++++++--
 fs/ocfs2/quota_local.c   |   10 ++--
 fs/quota/dquot.c         |  101 +++++++++++++++++++++++++++++++---------------
 include/linux/quotaops.h |    5 +-
 4 files changed, 90 insertions(+), 44 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index b26df86..3c32f40 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1032,10 +1032,15 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
 		    OCFS2_FEATURE_RO_COMPAT_USRQUOTA)) {
 			transfer_to[USRQUOTA] = dqget(sb, attr->ia_uid,
 						      USRQUOTA);
+			if (IS_ERR(transfer_to[USRQUOTA])) {
+				status = PTR_ERR(transfer_to[USRQUOTA]);
+				goto bail_unlock;
+			}
+
 			transfer_from[USRQUOTA] = dqget(sb, inode->i_uid,
 							USRQUOTA);
-			if (!transfer_to[USRQUOTA] || !transfer_from[USRQUOTA]) {
-				status = -ESRCH;
+			if (IS_ERR(transfer_from[USRQUOTA])) {
+				status = PTR_ERR(transfer_from[USRQUOTA]);
 				goto bail_unlock;
 			}
 		}
@@ -1044,10 +1049,15 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
 		    OCFS2_FEATURE_RO_COMPAT_GRPQUOTA)) {
 			transfer_to[GRPQUOTA] = dqget(sb, attr->ia_gid,
 						      GRPQUOTA);
+			if (IS_ERR(transfer_to[GRPQUOTA])) {
+				status = PTR_ERR(transfer_to[GRPQUOTA]);
+				goto bail_unlock;
+			}
+
 			transfer_from[GRPQUOTA] = dqget(sb, inode->i_gid,
 							GRPQUOTA);
-			if (!transfer_to[GRPQUOTA] || !transfer_from[GRPQUOTA]) {
-				status = -ESRCH;
+			if (IS_ERR(transfer_from[GRPQUOTA])) {
+				status = PTR_ERR(transfer_from[GRPQUOTA]);
 				goto bail_unlock;
 			}
 		}
diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c
index 21f9e71..6827ff6 100644
--- a/fs/ocfs2/quota_local.c
+++ b/fs/ocfs2/quota_local.c
@@ -469,13 +469,13 @@ static int ocfs2_recover_local_quota_file(struct inode *lqinode,
 			dqblk = (struct ocfs2_local_disk_dqblk *)(qbh->b_data +
 				ol_dqblk_block_off(sb, chunk, bit));
 			dquot = dqget(sb, le64_to_cpu(dqblk->dqb_id), type);
-			if (!dquot) {
-				status = -EIO;
+			if (IS_ERR(dquot)) {
+				status = PTR_ERR(dquot);
 				mlog(ML_ERROR, "Failed to get quota structure "
-				     "for id %u, type %d. Cannot finish quota "
-				     "file recovery.\n",
+				     "for id %u, type %d err %d. Cannot finish"
+				     " quota file recovery.\n",
 				     (unsigned)le64_to_cpu(dqblk->dqb_id),
-				     type);
+				     type, status);
 				goto out_put_bh;
 			}
 			status = ocfs2_lock_global_qf(oinfo, 1);
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index a168189..3f4541e 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -230,7 +230,7 @@ struct dqstats dqstats;
 EXPORT_SYMBOL(dqstats);
 
 static qsize_t inode_get_rsv_space(struct inode *inode);
-static void __dquot_initialize(struct inode *inode, int type);
+static int __dquot_initialize(struct inode *inode, int type);
 
 static inline unsigned int
 hashfn(const struct super_block *sb, unsigned int id, int type)
@@ -702,7 +702,7 @@ void dqput(struct dquot *dquot)
 {
 	int ret;
 
-	if (!dquot)
+	if (!dquot || IS_ERR(dquot))
 		return;
 #ifdef __DQUOT_PARANOIA
 	if (!atomic_read(&dquot->dq_count)) {
@@ -800,14 +800,16 @@ static struct dquot *get_empty_dquot(struct super_block *sb, int type)
  * destroying our dquot by:
  *   a) checking for quota flags under dq_list_lock and
  *   b) getting a reference to dquot before we release dq_list_lock
+ * Return: valid pointer on success, ERR_PTR otherwise.
  */
 struct dquot *dqget(struct super_block *sb, unsigned int id, int type)
 {
 	unsigned int hashent = hashfn(sb, id, type);
-	struct dquot *dquot = NULL, *empty = NULL;
+	struct dquot *dquot = ERR_PTR(-ESRCH);
+	struct dquot *empty = NULL;
 
         if (!sb_has_quota_active(sb, type))
-		return NULL;
+		goto out;
 we_slept:
 	spin_lock(&dq_list_lock);
 	spin_lock(&dq_state_lock);
@@ -848,11 +850,13 @@ we_slept:
 	 * already finished or it will be canceled due to dq_count > 1 test */
 	wait_on_dquot(dquot);
 	/* Read the dquot / allocate space in quota file */
-	if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags) &&
-	    sb->dq_op->acquire_dquot(dquot) < 0) {
-		dqput(dquot);
-		dquot = NULL;
-		goto out;
+	if (!test_bit(DQ_ACTIVE_B, &dquot->dq_flags)) {
+		int ret = sb->dq_op->acquire_dquot(dquot);
+		if (ret < 0) {
+			dqput(dquot);
+			dquot = ERR_PTR(ret);
+			goto out;
+		}
 	}
 #ifdef __DQUOT_PARANOIA
 	BUG_ON(!dquot->dq_sb);	/* Has somebody invalidated entry under us? */
@@ -1311,18 +1315,19 @@ static int info_bdq_free(struct dquot *dquot, qsize_t space)
  * It is better to call this function outside of any transaction as it
  * might need a lot of space in journal for dquot structure allocation.
  */
-static void __dquot_initialize(struct inode *inode, int type)
+static int __dquot_initialize(struct inode *inode, int type)
 {
 	unsigned int id = 0;
-	int cnt;
+	int cnt, err = 0;
 	struct dquot *got[MAXQUOTAS];
 	struct super_block *sb = inode->i_sb;
 	qsize_t rsv;
 
+repeat:
 	/* First test before acquiring mutex - solves deadlocks when we
          * re-enter the quota code and are already holding the mutex */
 	if (!sb_any_quota_active(inode->i_sb) || IS_NOQUOTA(inode))
-		return;
+		return 0;
 
 	/* First get references to structures we might need. */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
@@ -1341,35 +1346,50 @@ static void __dquot_initialize(struct inode *inode, int type)
 	}
 
 	down_write(&sb_dqopt(sb)->dqptr_sem);
-	if (IS_NOQUOTA(inode))
+	if (IS_NOQUOTA(inode)) {
+		err = 0;
 		goto out_err;
+	}
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
 		if (type != -1 && cnt != type)
 			continue;
 		/* Avoid races with quotaoff() */
 		if (!sb_has_quota_active(sb, cnt))
 			continue;
-		if (!inode->i_dquot[cnt]) {
-			inode->i_dquot[cnt] = got[cnt];
-			got[cnt] = NULL;
-			/*
-			 * Make quota reservation system happy if someone
-			 * did a write before quota was turned on
-			 */
-			rsv = inode_get_rsv_space(inode);
-			if (unlikely(rsv))
-				dquot_resv_space(inode->i_dquot[cnt], rsv);
+		if (inode->i_dquot[cnt])
+			continue;
+		if (unlikely(IS_ERR(got[cnt]))) {
+			err = (int)PTR_ERR(got[cnt]);
+			/* If dqget() was raced with quotaon() then we have to
+			 * repeat lookup. */
+			if (err == -ESRCH) {
+				err = 0;
+				up_write(&sb_dqopt(sb)->dqptr_sem);
+				dqput_all(got);
+				goto repeat;
+			}
+			goto out_err;
 		}
+		inode->i_dquot[cnt] = got[cnt];
+		got[cnt] = NULL;
+		/*
+		 * Make quota reservation system happy if someone
+		 * did a write before quota was turned on
+		 */
+		rsv = inode_get_rsv_space(inode);
+		if (unlikely(rsv))
+			dquot_resv_space(inode->i_dquot[cnt], rsv);
 	}
 out_err:
 	up_write(&sb_dqopt(sb)->dqptr_sem);
 	/* Drop unused references */
 	dqput_all(got);
+	return err;
 }
 
-void dquot_initialize(struct inode *inode)
+int dquot_initialize(struct inode *inode)
 {
-	__dquot_initialize(inode, -1);
+	return __dquot_initialize(inode, -1);
 }
 EXPORT_SYMBOL(dquot_initialize);
 
@@ -1696,6 +1716,7 @@ static int __dquot_transfer(struct inode *inode, qid_t *chid, unsigned long mask
 	char warntype_to[MAXQUOTAS];
 	char warntype_from_inodes[MAXQUOTAS], warntype_from_space[MAXQUOTAS];
 
+repeat:
 	/* First test before acquiring mutex - solves deadlocks when we
          * re-enter the quota code and are already holding the mutex */
 	if (IS_NOQUOTA(inode))
@@ -1721,15 +1742,29 @@ static int __dquot_transfer(struct inode *inode, qid_t *chid, unsigned long mask
 	space = cur_space + rsv_space;
 	/* Build the transfer_from list and check the limits */
 	for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
-		if (!transfer_to[cnt])
+		if (!(mask & (1 << cnt)))
 			continue;
+		if (unlikely(IS_ERR(transfer_to[cnt]))) {
+			ret = (int)PTR_ERR(transfer_to[cnt]);
+			/* If dqget() was raced with quotaon() then we have to
+			 * repeat lookup. */
+			if (ret == -ESRCH) {
+				ret = 0;
+				spin_unlock(&dq_data_lock);
+				up_write(&sb_dqopt(inode->i_sb)->dqptr_sem);
+				dqput_all(transfer_to);
+				goto repeat;
+			}
+			goto bad_quota;
+		}
+
 		transfer_from[cnt] = inode->i_dquot[cnt];
 		ret = check_idq(transfer_to[cnt], 1, warntype_to + cnt);
 		if (ret)
-			goto over_quota;
+			goto bad_quota;
 		ret = check_bdq(transfer_to[cnt], space, 0, warntype_to + cnt);
 		if (ret)
-			goto over_quota;
+			goto bad_quota;
 	}
 
 	/*
@@ -1776,7 +1811,7 @@ put_all:
 	dqput_all(transfer_from);
 	dqput_all(transfer_to);
 	return ret;
-over_quota:
+bad_quota:
 	spin_unlock(&dq_data_lock);
 	up_write(&sb_dqopt(inode->i_sb)->dqptr_sem);
 	/* Clear dquot pointers we don't want to dqput() */
@@ -2302,8 +2337,8 @@ int vfs_get_dqblk(struct super_block *sb, int type, qid_t id,
 	struct dquot *dquot;
 
 	dquot = dqget(sb, id, type);
-	if (!dquot)
-		return -ESRCH;
+	if (IS_ERR(dquot))
+		return PTR_ERR(dquot);
 	do_get_dqblk(dquot, di);
 	dqput(dquot);
 
@@ -2396,8 +2431,8 @@ int vfs_set_dqblk(struct super_block *sb, int type, qid_t id,
 	int rc;
 
 	dquot = dqget(sb, id, type);
-	if (!dquot) {
-		rc = -ESRCH;
+	if (IS_ERR(dquot)) {
+		rc = PTR_ERR(dquot);
 		goto out;
 	}
 	rc = do_set_dqblk(dquot, di);
diff --git a/include/linux/quotaops.h b/include/linux/quotaops.h
index fd8f70b..6b50f98 100644
--- a/include/linux/quotaops.h
+++ b/include/linux/quotaops.h
@@ -31,7 +31,7 @@ void inode_add_rsv_space(struct inode *inode, qsize_t number);
 void inode_claim_rsv_space(struct inode *inode, qsize_t number);
 void inode_sub_rsv_space(struct inode *inode, qsize_t number);
 
-void dquot_initialize(struct inode *inode);
+int dquot_initialize(struct inode *inode);
 void dquot_drop(struct inode *inode);
 struct dquot *dqget(struct super_block *sb, unsigned int id, int type);
 void dqput(struct dquot *dquot);
@@ -206,8 +206,9 @@ static inline int sb_any_quota_active(struct super_block *sb)
 #define sb_dquot_ops				(NULL)
 #define sb_quotactl_ops				(NULL)
 
-static inline void dquot_initialize(struct inode *inode)
+static inline int dquot_initialize(struct inode *inode)
 {
+	return 0;
 }
 
 static inline void dquot_drop(struct inode *inode)
-- 
1.6.6.1