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