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@xxxxxxxxxx>
---
fs/ocfs2/file.c | 8 ++--
fs/ocfs2/quota_local.c | 10 ++--
fs/quota/dquot.c | 121 +++++++++++++++++++++++++++++++---------------
include/linux/quotaops.h | 5 +-
4 files changed, 93 insertions(+), 51 deletions(-)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 312b8f5..92d91a1 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -1031,8 +1031,8 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
OCFS2_FEATURE_RO_COMPAT_USRQUOTA)) {
transfer_to[USRQUOTA] = dqget(sb, attr->ia_uid,
USRQUOTA);
- if (!transfer_to[USRQUOTA]) {
- status = -ESRCH;
+ if (IS_ERR(transfer_to[USRQUOTA])) {
+ status = PTR_ERR(transfer_to[USRQUOTA]);
goto bail_unlock;
}
}
@@ -1041,8 +1041,8 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr)
OCFS2_FEATURE_RO_COMPAT_GRPQUOTA)) {
transfer_to[GRPQUOTA] = dqget(sb, attr->ia_gid,
GRPQUOTA);
- if (!transfer_to[GRPQUOTA]) {
- status = -ESRCH;
+ if (IS_ERR(transfer_to[GRPQUOTA])) {
+ status = PTR_ERR(transfer_to[GRPQUOTA]);
goto bail_unlock;
}
}
diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c
index 1b55b1d..6cf42b5 100644
--- a/fs/ocfs2/quota_local.c
+++ b/fs/ocfs2/quota_local.c
@@ -504,13 +504,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 7c624e1..95aa445 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -234,7 +234,7 @@ EXPORT_SYMBOL(dqstats_pcpu);
#endif
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)
@@ -720,7 +720,7 @@ void dqput(struct dquot *dquot)
{
int ret;
- if (!dquot)
+ if (!dquot || IS_ERR(dquot))
return;
#ifdef CONFIG_QUOTA_DEBUG
if (!atomic_read(&dquot->dq_count)) {
@@ -815,14 +815,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);
@@ -863,11 +865,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 CONFIG_QUOTA_DEBUG
BUG_ON(!dquot->dq_sb); /* Has somebody invalidated entry under us? */
@@ -895,9 +899,10 @@ static int dqinit_needed(struct inode *inode, int type)
}
/* This routine is guarded by dqonoff_mutex mutex */
-static void add_dquot_ref(struct super_block *sb, int type)
+static int add_dquot_ref(struct super_block *sb, int type)
{
struct inode *inode, *old_inode = NULL;
+ int err = 0;
#ifdef CONFIG_QUOTA_DEBUG
int reserved = 0;
#endif
@@ -919,7 +924,7 @@ static void add_dquot_ref(struct super_block *sb, int type)
spin_unlock(&inode_lock);
iput(old_inode);
- __dquot_initialize(inode, type);
+ err = __dquot_initialize(inode, type);
/* We hold a reference to 'inode' so it couldn't have been
* removed from s_inodes list while we dropped the inode_lock.
* We cannot iput the inode now as we can be holding the last
@@ -927,6 +932,8 @@ static void add_dquot_ref(struct super_block *sb, int type)
* keep the reference and iput it later. */
old_inode = inode;
spin_lock(&inode_lock);
+ if (unlikely(err))
+ break;
}
spin_unlock(&inode_lock);
iput(old_inode);
@@ -938,6 +945,7 @@ static void add_dquot_ref(struct super_block *sb, int type)
"inconsistent. Please run quotacheck(8).\n", sb->s_id);
}
#endif
+ return err;
}
/*
@@ -1332,18 +1340,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++) {
@@ -1362,35 +1371,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 = 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);
@@ -1809,12 +1833,23 @@ int dquot_transfer(struct inode *inode, struct iattr *iattr)
if (!sb_any_quota_active(sb) || IS_NOQUOTA(inode))
return 0;
- if (iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid)
+ if (iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid) {
transfer_to[USRQUOTA] = dqget(sb, iattr->ia_uid, USRQUOTA);
- if (iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid)
+ if (IS_ERR(transfer_to[USRQUOTA])) {
+ ret = PTR_ERR(transfer_to[USRQUOTA]);
+ goto out;
+ }
+ }
+ if (iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid) {
transfer_to[GRPQUOTA] = dqget(sb, iattr->ia_uid, GRPQUOTA);
+ if (IS_ERR(transfer_to[GRPQUOTA])) {
+ ret = PTR_ERR(transfer_to[GRPQUOTA]);
+ goto out;
+ }
+ }
ret = __dquot_transfer(inode, transfer_to);
+out:
dqput_all(transfer_to);
return ret;
}
@@ -1857,7 +1892,7 @@ int dquot_file_open(struct inode *inode, struct file *file)
error = generic_file_open(inode, file);
if (!error && (file->f_mode & FMODE_WRITE))
- dquot_initialize(inode);
+ error = dquot_initialize(inode);
return error;
}
EXPORT_SYMBOL(dquot_file_open);
@@ -2018,7 +2053,7 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
struct super_block *sb = inode->i_sb;
struct quota_info *dqopt = sb_dqopt(sb);
int error;
- int oldflags = -1;
+ unsigned int dqflags, iflags = -1;
if (!fmt)
return -ESRCH;
@@ -2061,7 +2096,7 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
* possible) Also nobody should write to the file - we use
* special IO operations which ignore the immutable bit. */
mutex_lock_nested(&inode->i_mutex, I_MUTEX_QUOTA);
- oldflags = inode->i_flags & (S_NOATIME | S_IMMUTABLE |
+ iflags = inode->i_flags & (S_NOATIME | S_IMMUTABLE |
S_NOQUOTA);
inode->i_flags |= S_NOQUOTA | S_NOATIME | S_IMMUTABLE;
mutex_unlock(&inode->i_mutex);
@@ -2092,24 +2127,30 @@ static int vfs_load_quota_inode(struct inode *inode, int type, int format_id,
}
mutex_unlock(&dqopt->dqio_mutex);
spin_lock(&dq_state_lock);
+ dqflags = dqopt->flags;
dqopt->flags |= dquot_state_flag(flags, type);
spin_unlock(&dq_state_lock);
- add_dquot_ref(sb, type);
+ error = add_dquot_ref(sb, type);
+ if (error)
+ goto out_dquot_init;
mutex_unlock(&dqopt->dqonoff_mutex);
return 0;
-
+out_dquot_init:
+ spin_lock(&dq_state_lock);
+ dqopt->flags = dqflags;
+ spin_unlock(&dq_state_lock);
out_file_init:
dqopt->files[type] = NULL;
iput(inode);
out_lock:
- if (oldflags != -1) {
+ if (iflags != -1) {
mutex_lock_nested(&inode->i_mutex, I_MUTEX_QUOTA);
/* Set the flags back (in the case of accidental quotaon()
* on a wrong file we don't want to mess up the flags) */
inode->i_flags &= ~(S_NOATIME | S_NOQUOTA | S_IMMUTABLE);
- inode->i_flags |= oldflags;
+ inode->i_flags |= iflags;
mutex_unlock(&inode->i_mutex);
}
mutex_unlock(&dqopt->dqonoff_mutex);
@@ -2319,8 +2360,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);
@@ -2432,8 +2473,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 370abb1..4fc5a9c 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);
@@ -209,8 +209,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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
