From 020941416419ab282f3d9b694014b2059d299d51 Mon Sep 17 00:00:00 2001 From: Bobi Jam Date: Mon, 14 Jul 2014 13:51:05 +0800 Subject: [PATCH 1/1] 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/11085 Tested-by: Jenkins Reviewed-by: Niu Yawei Reviewed-by: Fan Yong Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/osd-ldiskfs/osd_handler.c | 80 ++++++++++++++++++++++----------------- lustre/osd-ldiskfs/osd_internal.h | 6 ++- lustre/osd-ldiskfs/osd_io.c | 10 ++--- lustre/osd-ldiskfs/osd_quota.c | 41 ++++++++++++-------- 4 files changed, 80 insertions(+), 57 deletions(-) diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c index 03f1d3f..b542546 100644 --- a/lustre/osd-ldiskfs/osd_handler.c +++ b/lustre/osd-ldiskfs/osd_handler.c @@ -1742,7 +1742,7 @@ static int osd_declare_attr_set(const struct lu_env *env, qid_t gid; long long bspace; int rc = 0; - bool allocated; + bool enforce; ENTRY; LASSERT(dt != NULL); @@ -1770,18 +1770,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 != (uid = i_uid_read(obj->oo_inode))) { + if (attr->la_valid & LA_UID || attr->la_valid & LA_GID) { + /* USERQUOTA */ + uid = i_uid_read(obj->oo_inode); qi->lqi_type = USRQUOTA; - + enforce = (attr->la_valid & LA_UID) && (attr->la_uid != 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) @@ -1790,7 +1791,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 = 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) @@ -1799,38 +1800,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 = 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 != (gid = i_gid_read(obj->oo_inode))) { + /* GROUP QUOTA */ + gid = i_gid_read(obj->oo_inode); qi->lqi_type = GRPQUOTA; + enforce = (attr->la_valid & LA_GID) && (attr->la_gid != 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) @@ -1839,7 +1842,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 = 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) @@ -1848,20 +1851,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 = 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) @@ -1927,6 +1929,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; @@ -1991,7 +1994,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) @@ -2381,7 +2383,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); @@ -2452,12 +2454,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, i_uid_read(inode), i_gid_read(inode), - -1, oh, false, true, NULL, false); + -1, oh, obj, false, NULL, false); if (rc) RETURN(rc); /* data to be truncated */ rc = osd_declare_inode_qid(env, i_uid_read(inode), i_gid_read(inode), - 0, oh, true, true, NULL, false); + 0, oh, obj, true, NULL, false); RETURN(rc); } @@ -2943,6 +2945,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); @@ -2958,13 +2961,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); @@ -3074,6 +3080,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); @@ -3083,6 +3090,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; } @@ -3552,7 +3564,7 @@ static int osd_index_declare_ea_delete(const struct lu_env *env, LASSERT(inode); rc = osd_declare_inode_qid(env, i_uid_read(inode), i_gid_read(inode), - 0, oh, true, true, NULL, false); + 0, oh, osd_dt_obj(dt), true, NULL, false); RETURN(rc); } @@ -4404,8 +4416,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, i_uid_read(inode), - i_gid_read(inode), 0, - oh, true, true, NULL, false); + i_gid_read(inode), 0, 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 ce78da1..af14812 100644 --- a/lustre/osd-ldiskfs/osd_internal.h +++ b/lustre/osd-ldiskfs/osd_internal.h @@ -707,10 +707,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 ccb210c..9765595 100644 --- a/lustre/osd-ldiskfs/osd_io.c +++ b/lustre/osd-ldiskfs/osd_io.c @@ -1116,8 +1116,8 @@ static int osd_declare_write_commit(const struct lu_env *env, lnb[0].lnb_flags &= ~(OBD_BRW_OVER_USRQUOTA | OBD_BRW_OVER_GRPQUOTA); rc = osd_declare_inode_qid(env, i_uid_read(inode), i_gid_read(inode), - 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 @@ -1536,8 +1536,8 @@ out: * objects, so always set the lqi_space as 0. */ if (inode != NULL) rc = osd_declare_inode_qid(env, i_uid_read(inode), - i_gid_read(inode), 0, oh, true, - true, NULL, false); + i_gid_read(inode), 0, oh, obj, true, + NULL, false); RETURN(rc); } @@ -1702,7 +1702,7 @@ static int osd_declare_punch(const struct lu_env *env, struct dt_object *dt, LASSERT(inode); rc = osd_declare_inode_qid(env, i_uid_read(inode), i_gid_read(inode), - 0, oh, true, true, NULL, false); + 0, oh, 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 8513bfa..4051640 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 - * group quota, or sync commit flags to the caller + * \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