On Wed 19-05-10 10:01:58, Dmitry Monakhov wrote:
> Due to previous IO error or other bugs an inode may not has quota
> pointer. This definite sign of quota inconsistency/corruption.
> In fact this condition must being checked in all charge/uncharge
> functions. And if error was detected it is reasonable to fail whole
> operation. But uncharge(free_space/claim_space/free_inode) functions
> has no fail nature. This is because caller can't handle errors from
> such functions. How can you handle error from following call-trace?
> write_page()->quota_claim_space()
>
> So alloc_space() and alloc_inode() are the only functions which we may
> reliably abort in case of any errors.
>
> This patch add corresponding checks only for charge functions.
Actually, I've realized that this patch has a problem if
dquot_alloc_space is called while add_dquot_ref() is running. Then it would
return EIO although it should just silently succeed (and yes, quotas will
be inconsistent but that's "life" when quota isn't filesystem metadata...).
So I don't think this is a move in the right direction at least for now...
Honza
>
> Signed-off-by: Dmitry Monakhov <dmonakhov@xxxxxxxxxx>
> ---
> fs/quota/dquot.c | 41 +++++++++++++++++++++++++++++++++++------
> 1 files changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
> index 95aa445..f0e68b3 100644
> --- a/fs/quota/dquot.c
> +++ b/fs/quota/dquot.c
> @@ -1541,7 +1541,7 @@ static void inode_decr_space(struct inode *inode, qsize_t number, int reserve)
> int __dquot_alloc_space(struct inode *inode, qsize_t number,
> int warn, int reserve)
> {
> - int cnt, ret = 0;
> + int cnt, active, ret = 0;
> char warntype[MAXQUOTAS];
>
> /*
> @@ -1554,13 +1554,28 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number,
> }
>
> down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> + /* Now recheck reliably when holding dqptr_sem */
> + active = sb_any_quota_active(inode->i_sb);
> + if (!active || IS_NOQUOTA(inode)) {
> + inode_incr_space(inode, number, reserve);
> + goto out;
> + }
> +
> for (cnt = 0; cnt < MAXQUOTAS; cnt++)
> warntype[cnt] = QUOTA_NL_NOWARN;
>
> spin_lock(&dq_data_lock);
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> - if (!inode->i_dquot[cnt])
> + if (!(active & (1 << cnt)))
> continue;
> + /*
> + * Given quota type is active, so quota must being
> + * initialized for this inode
> + */
> + if (!inode->i_dquot[cnt]) {
> + ret = -EIO;
> + goto out_flush_warn;
> + }
> ret = check_bdq(inode->i_dquot[cnt], number, !warn,
> warntype+cnt);
> if (ret) {
> @@ -1569,7 +1584,7 @@ int __dquot_alloc_space(struct inode *inode, qsize_t number,
> }
> }
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> - if (!inode->i_dquot[cnt])
> + if (!(active & (1 << cnt)))
> continue;
> if (reserve)
> dquot_resv_space(inode->i_dquot[cnt], number);
> @@ -1595,7 +1610,7 @@ EXPORT_SYMBOL(__dquot_alloc_space);
> */
> int dquot_alloc_inode(const struct inode *inode)
> {
> - int cnt, ret = 0;
> + int cnt, active, ret = 0;
> char warntype[MAXQUOTAS];
>
> /* First test before acquiring mutex - solves deadlocks when we
> @@ -1605,17 +1620,31 @@ int dquot_alloc_inode(const struct inode *inode)
> for (cnt = 0; cnt < MAXQUOTAS; cnt++)
> warntype[cnt] = QUOTA_NL_NOWARN;
> down_read(&sb_dqopt(inode->i_sb)->dqptr_sem);
> + /* Now recheck reliably when holding dqptr_sem */
> + active = sb_any_quota_active(inode->i_sb);
> + if (!active || IS_NOQUOTA(inode)) {
> + return 0;
> + }
> +
> spin_lock(&dq_data_lock);
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> - if (!inode->i_dquot[cnt])
> + if (!(active & (1 < cnt)))
> continue;
> + /*
> + * Given quota type is active, so quota must being
> + * initialized for this inode
> + */
> + if (!inode->i_dquot[cnt]) {
> + ret = -EIO;
> + goto warn_put_all;
> + }
> ret = check_idq(inode->i_dquot[cnt], 1, warntype + cnt);
> if (ret)
> goto warn_put_all;
> }
>
> for (cnt = 0; cnt < MAXQUOTAS; cnt++) {
> - if (!inode->i_dquot[cnt])
> + if ((!active & (1 < cnt)))
> continue;
> dquot_incr_inodes(inode->i_dquot[cnt], 1);
> }
> --
> 1.6.6.1
>
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
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
