From 57028a9a759484524d84933543849e2b8479608c Mon Sep 17 00:00:00 2001 From: Bobi Jam Date: Mon, 14 Jul 2014 13:51:05 +0800 Subject: [PATCH] LU-5040 osd: fix osd declare credit for quota osd_attr_set() always calls ll_vfs_dq_init() to initialize dquot for the inode, while in some cases osd_declare_attr_set() does not reserve credit for it. This patch fixes this issue. This patch also corrects the quota credit accounting in osd_declare_qid() by judging whether the inode's i_dquot[quota_type] has been allocated, while old code judgement is based on the file uid/gid existence, which is not correct. Signed-off-by: Bobi Jam Change-Id: I16555cb1097e1a3e75cdcb4852a2c5e1772ddd88 Reviewed-on: http://review.whamcloud.com/11097 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Jian Yu Reviewed-by: Niu Yawei Reviewed-by: James Simmons Reviewed-by: Oleg Drokin --- lustre/osd-ldiskfs/osd_handler.c | 79 +++++++++++++++++++++++---------------- lustre/osd-ldiskfs/osd_internal.h | 6 ++- lustre/osd-ldiskfs/osd_io.c | 8 ++-- lustre/osd-ldiskfs/osd_quota.c | 39 +++++++++++-------- 4 files changed, 78 insertions(+), 54 deletions(-) diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c index 5c2fcc7..e7cef42 100644 --- a/lustre/osd-ldiskfs/osd_handler.c +++ b/lustre/osd-ldiskfs/osd_handler.c @@ -1661,7 +1661,7 @@ static int osd_declare_attr_set(const struct lu_env *env, struct lquota_id_info *qi = &info->oti_qi; long long bspace; int rc = 0; - bool allocated; + bool enforce; ENTRY; LASSERT(dt != NULL); @@ -1689,18 +1689,19 @@ static int osd_declare_attr_set(const struct lu_env *env, * We still need to call the osd_declare_qid() to calculate the journal * credits for updating quota accounting files and to trigger quota * space adjustment once the operation is completed.*/ - if ((attr->la_valid & LA_UID) != 0 && - attr->la_uid != obj->oo_inode->i_uid) { + if (attr->la_valid & LA_UID || attr->la_valid & LA_GID) { + /* USERQUOTA */ qi->lqi_type = USRQUOTA; - + enforce = (attr->la_valid & LA_UID) && + (attr->la_uid != obj->oo_inode->i_uid); /* inode accounting */ qi->lqi_is_blk = false; - /* one more inode for the new owner ... */ + /* one more inode for the new uid ... */ qi->lqi_id.qid_uid = attr->la_uid; qi->lqi_space = 1; - allocated = (attr->la_uid == 0) ? true : false; - rc = osd_declare_qid(env, oh, qi, allocated, NULL); + /* Reserve credits for the new uid */ + rc = osd_declare_qid(env, oh, qi, NULL, enforce, NULL); if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) @@ -1709,7 +1710,7 @@ static int osd_declare_attr_set(const struct lu_env *env, /* and one less inode for the current uid */ qi->lqi_id.qid_uid = obj->oo_inode->i_uid; qi->lqi_space = -1; - rc = osd_declare_qid(env, oh, qi, true, NULL); + rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) @@ -1718,38 +1719,40 @@ static int osd_declare_attr_set(const struct lu_env *env, /* block accounting */ qi->lqi_is_blk = true; - /* more blocks for the new owner ... */ + /* more blocks for the new uid ... */ qi->lqi_id.qid_uid = attr->la_uid; qi->lqi_space = bspace; - allocated = (attr->la_uid == 0) ? true : false; - rc = osd_declare_qid(env, oh, qi, allocated, NULL); + /* + * Credits for the new uid has been reserved, re-use "obj" + * to save credit reservation. + */ + rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) RETURN(rc); - /* and finally less blocks for the current owner */ + /* and finally less blocks for the current uid */ qi->lqi_id.qid_uid = obj->oo_inode->i_uid; qi->lqi_space = -bspace; - rc = osd_declare_qid(env, oh, qi, true, NULL); + rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) RETURN(rc); - } - if (attr->la_valid & LA_GID && - attr->la_gid != obj->oo_inode->i_gid) { + /* GROUP QUOTA */ qi->lqi_type = GRPQUOTA; + enforce = (attr->la_valid & LA_GID) && + (attr->la_gid != obj->oo_inode->i_gid); /* inode accounting */ qi->lqi_is_blk = false; - /* one more inode for the new group owner ... */ + /* one more inode for the new gid ... */ qi->lqi_id.qid_gid = attr->la_gid; qi->lqi_space = 1; - allocated = (attr->la_gid == 0) ? true : false; - rc = osd_declare_qid(env, oh, qi, allocated, NULL); + rc = osd_declare_qid(env, oh, qi, NULL, enforce, NULL); if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) @@ -1758,7 +1761,7 @@ static int osd_declare_attr_set(const struct lu_env *env, /* and one less inode for the current gid */ qi->lqi_id.qid_gid = obj->oo_inode->i_gid; qi->lqi_space = -1; - rc = osd_declare_qid(env, oh, qi, true, NULL); + rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) @@ -1767,20 +1770,19 @@ static int osd_declare_attr_set(const struct lu_env *env, /* block accounting */ qi->lqi_is_blk = true; - /* more blocks for the new owner ... */ + /* more blocks for the new gid ... */ qi->lqi_id.qid_gid = attr->la_gid; qi->lqi_space = bspace; - allocated = (attr->la_gid == 0) ? true : false; - rc = osd_declare_qid(env, oh, qi, allocated, NULL); + rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) RETURN(rc); - /* and finally less blocks for the current owner */ + /* and finally less blocks for the current gid */ qi->lqi_id.qid_gid = obj->oo_inode->i_gid; qi->lqi_space = -bspace; - rc = osd_declare_qid(env, oh, qi, true, NULL); + rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) @@ -1841,6 +1843,7 @@ static int osd_quota_transfer(struct inode *inode, const struct lu_attr *attr) struct iattr iattr; int rc; + ll_vfs_dq_init(inode); iattr.ia_valid = 0; if (attr->la_valid & LA_UID) iattr.ia_valid |= ATTR_UID; @@ -1905,7 +1908,6 @@ static int osd_attr_set(const struct lu_env *env, } inode = obj->oo_inode; - ll_vfs_dq_init(inode); rc = osd_quota_transfer(inode, attr); if (rc) @@ -2299,7 +2301,7 @@ static int osd_declare_object_create(const struct lu_env *env, RETURN(0); rc = osd_declare_inode_qid(env, attr->la_uid, attr->la_gid, 1, oh, - false, false, NULL, false); + osd_dt_obj(dt), false, NULL, false); if (rc != 0) RETURN(rc); @@ -2380,12 +2382,12 @@ static int osd_declare_object_destroy(const struct lu_env *env, osd_dto_credits_noquota[DTO_INDEX_DELETE] + 3); /* one less inode */ rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, -1, oh, - false, true, NULL, false); + obj, false, NULL, false); if (rc) RETURN(rc); /* data to be truncated */ rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, 0, oh, - true, true, NULL, false); + obj, true, NULL, false); RETURN(rc); } @@ -2858,6 +2860,7 @@ static int osd_declare_xattr_set(const struct lu_env *env, { struct osd_thandle *oh; int credits; + struct super_block *sb = osd_sb(osd_dev(dt->do_lu.lo_dev)); LASSERT(handle != NULL); @@ -2873,13 +2876,16 @@ static int osd_declare_xattr_set(const struct lu_env *env, } else if (strcmp(name, XATTR_NAME_VERSION) == 0) { credits = 1; } else { - struct osd_device *osd = osd_dev(dt->do_lu.lo_dev); - struct super_block *sb = osd_sb(osd); credits = osd_dto_credits_noquota[DTO_XATTR_SET]; if (buf && buf->lb_len > sb->s_blocksize) { credits *= (buf->lb_len + sb->s_blocksize - 1) >> sb->s_blocksize_bits; } + /* + * xattr set may involve inode quota change, reserve credits for + * dquot_initialize() + */ + oh->ot_credits += LDISKFS_MAXQUOTAS_INIT_BLOCKS(sb); } osd_trans_declare_op(env, oh, OSD_OT_XATTR_SET, credits); @@ -2971,6 +2977,7 @@ static int osd_declare_xattr_del(const struct lu_env *env, struct thandle *handle) { struct osd_thandle *oh; + struct super_block *sb = osd_sb(osd_dev(dt->do_lu.lo_dev)); LASSERT(dt_object_exists(dt) && !dt_object_remote(dt)); LASSERT(handle != NULL); @@ -2980,6 +2987,11 @@ static int osd_declare_xattr_del(const struct lu_env *env, osd_trans_declare_op(env, oh, OSD_OT_XATTR_SET, osd_dto_credits_noquota[DTO_XATTR_SET]); + /* + * xattr del may involve inode quota change, reserve credits for + * dquot_initialize() + */ + oh->ot_credits += LDISKFS_MAXQUOTAS_INIT_BLOCKS(sb); return 0; } @@ -3440,7 +3452,7 @@ static int osd_index_declare_ea_delete(const struct lu_env *env, LASSERT(inode); rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, 0, oh, - true, true, NULL, false); + osd_dt_obj(dt), true, NULL, false); RETURN(rc); } @@ -4231,7 +4243,8 @@ static int osd_index_declare_ea_insert(const struct lu_env *env, * calculate how many blocks will be consumed by this index * insert */ rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, 0, - oh, true, true, NULL, false); + oh, osd_dt_obj(dt), true, NULL, + false); } if (fid == NULL) diff --git a/lustre/osd-ldiskfs/osd_internal.h b/lustre/osd-ldiskfs/osd_internal.h index 904ef5b..b1b05f9 100644 --- a/lustre/osd-ldiskfs/osd_internal.h +++ b/lustre/osd-ldiskfs/osd_internal.h @@ -712,10 +712,12 @@ loff_t find_tree_dqentry(const struct lu_env *env, struct osd_it_quota *it); /* osd_quota.c */ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh, - struct lquota_id_info *qi, bool allocated, int *flags); + struct lquota_id_info *qi, struct osd_object *obj, + bool enforce, int *flags); int osd_declare_inode_qid(const struct lu_env *env, qid_t uid, qid_t gid, long long space, struct osd_thandle *oh, - bool is_blk, bool allocated, int *flags, bool force); + struct osd_object *obj, bool is_blk, int *flags, + bool force); const struct dt_rec *osd_quota_pack(struct osd_object *obj, const struct dt_rec *rec, union lquota_rec *quota_rec); diff --git a/lustre/osd-ldiskfs/osd_io.c b/lustre/osd-ldiskfs/osd_io.c index 7e0bab8..4c59cee 100644 --- a/lustre/osd-ldiskfs/osd_io.c +++ b/lustre/osd-ldiskfs/osd_io.c @@ -711,8 +711,8 @@ static int osd_declare_write_commit(const struct lu_env *env, lnb[0].flags &= ~(OBD_BRW_OVER_USRQUOTA | OBD_BRW_OVER_GRPQUOTA); rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, - quota_space, oh, true, true, &flags, - ignore_quota); + quota_space, oh, osd_dt_obj(dt), true, + &flags, ignore_quota); /* we need only to store the overquota flags in the first lnb for * now, once we support multiple objects BRW, this code needs be @@ -1119,7 +1119,7 @@ out: * objects, so always set the lqi_space as 0. */ if (inode != NULL) rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, - 0, oh, true, true, NULL, false); + 0, oh, obj, true, NULL, false); RETURN(rc); } @@ -1284,7 +1284,7 @@ static int osd_declare_punch(const struct lu_env *env, struct dt_object *dt, LASSERT(inode); rc = osd_declare_inode_qid(env, inode->i_uid, inode->i_gid, 0, oh, - true, true, NULL, false); + osd_dt_obj(dt), true, NULL, false); RETURN(rc); } diff --git a/lustre/osd-ldiskfs/osd_quota.c b/lustre/osd-ldiskfs/osd_quota.c index 917491d..c18d672 100644 --- a/lustre/osd-ldiskfs/osd_quota.c +++ b/lustre/osd-ldiskfs/osd_quota.c @@ -493,23 +493,26 @@ static inline void osd_qid_set_type(struct osd_thandle *oh, int i, int type) * Reserve journal credits for quota files update first, then call * ->op_begin() to perform quota enforcement. * - * \param env - the environment passed by the caller - * \param oh - osd transaction handle - * \param qi - quota id & space required for this operation - * \param allocated - dquot entry in quota accounting file has been allocated - * \param flags - if the operation is write, return no user quota, no + * \param env - the environment passed by the caller + * \param oh - osd transaction handle + * \param qi - quota id & space required for this operation + * \param obj - osd object, could be NULL when it's under create + * \param enforce - whether to perform quota enforcement + * \param flags - if the operation is write, return no user quota, no * group quota, or sync commit flags to the caller * - * \retval 0 - success - * \retval -ve - failure + * \retval 0 - success + * \retval -ve - failure */ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh, - struct lquota_id_info *qi, bool allocated, int *flags) + struct lquota_id_info *qi, struct osd_object *obj, + bool enforce, int *flags) { struct osd_thread_info *info = osd_oti_get(env); struct osd_device *dev = info->oti_dev; struct qsd_instance *qsd = dev->od_quota_slave; - int i, rc; + struct inode *inode = NULL; + int i, rc = 0; bool found = false; ENTRY; @@ -532,8 +535,12 @@ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh, RETURN(-EOVERFLOW); } + if (obj != NULL) + inode = obj->oo_inode; osd_trans_declare_op(env, oh, OSD_OT_QUOTA, - (allocated || qi->lqi_id.qid_uid == 0) ? + (qi->lqi_id.qid_uid == 0 || + (inode != NULL && + inode->i_dquot[qi->lqi_type] != NULL)) ? 1: LDISKFS_QUOTA_INIT_BLOCKS(osd_sb(dev))); oh->ot_id_array[i] = qi->lqi_id.qid_uid; @@ -546,7 +553,8 @@ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh, RETURN(0); /* check quota */ - rc = qsd_op_begin(env, qsd, oh->ot_quota_trans, qi, flags); + if (enforce) + rc = qsd_op_begin(env, qsd, oh->ot_quota_trans, qi, flags); RETURN(rc); } @@ -558,8 +566,8 @@ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh, * \param gid - group id of the inode * \param space - how many blocks/inodes will be consumed/released * \param oh - osd transaction handle + * \param obj - osd object, could be NULL when it's under create * \param is_blk - block quota or inode quota? - * \param allocated - dquot entry in quota accounting file has been allocated * \param flags - if the operation is write, return no user quota, no * group quota, or sync commit flags to the caller * \param force - set to 1 when changes are performed by root user and thus @@ -570,7 +578,8 @@ int osd_declare_qid(const struct lu_env *env, struct osd_thandle *oh, */ int osd_declare_inode_qid(const struct lu_env *env, qid_t uid, qid_t gid, long long space, struct osd_thandle *oh, - bool is_blk, bool allocated, int *flags, bool force) + struct osd_object *obj, bool is_blk, int *flags, + bool force) { struct osd_thread_info *info = osd_oti_get(env); struct lquota_id_info *qi = &info->oti_qi; @@ -582,7 +591,7 @@ int osd_declare_inode_qid(const struct lu_env *env, qid_t uid, qid_t gid, qi->lqi_type = USRQUOTA; qi->lqi_space = space; qi->lqi_is_blk = is_blk; - rcu = osd_declare_qid(env, oh, qi, allocated, flags); + rcu = osd_declare_qid(env, oh, qi, obj, true, flags); if (force && (rcu == -EDQUOT || rcu == -EINPROGRESS)) /* ignore EDQUOT & EINPROGRESS when changes are done by root */ @@ -598,7 +607,7 @@ int osd_declare_inode_qid(const struct lu_env *env, qid_t uid, qid_t gid, /* and now group quota */ qi->lqi_id.qid_gid = gid; qi->lqi_type = GRPQUOTA; - rcg = osd_declare_qid(env, oh, qi, allocated, flags); + rcg = osd_declare_qid(env, oh, qi, obj, true, flags); if (force && (rcg == -EDQUOT || rcg == -EINPROGRESS)) /* as before, ignore EDQUOT & EINPROGRESS for root */ -- 1.8.3.1