From 83548f2b373884fcec9575417f0bc6de57abbdc2 Mon Sep 17 00:00:00 2001 From: Andreas Dilger Date: Fri, 16 Nov 2018 13:38:25 -0700 Subject: [PATCH] Revert "LU-5152 quota: enforce block quota for chgrp" This reverts commit 07412234ec60de20cb8d8e45d755297fe6da2d61. This can cause deadlocks between the MDT and OST as seen in LU-11465 and other related problems. Reverting this patch will mean file group changes to not be updated in group quota. Signed-off-by: Andreas Dilger Change-Id: I5ce48424f4d2011ce62e69047ace7f0b7c11a93b Reviewed-on: https://review.whamcloud.com/33682 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Alex Zhuravlev Reviewed-by: Hongchao Zhang Reviewed-by: Oleg Drokin --- lustre/include/lustre/lustre_idl.h | 1 - lustre/mdd/mdd_object.c | 59 +++++++++++--------------------------- lustre/osd-ldiskfs/osd_handler.c | 31 +++++++------------- lustre/osd-zfs/osd_object.c | 20 +++++-------- lustre/osp/osp_object.c | 33 ++------------------- lustre/quota/qsd_handler.c | 7 ----- lustre/target/out_lib.c | 4 --- 7 files changed, 36 insertions(+), 119 deletions(-) diff --git a/lustre/include/lustre/lustre_idl.h b/lustre/include/lustre/lustre_idl.h index e40b90e..1a2180b 100644 --- a/lustre/include/lustre/lustre_idl.h +++ b/lustre/include/lustre/lustre_idl.h @@ -1639,7 +1639,6 @@ enum { * 2. If these flags needs to be stored into inode, they will be * stored in LMA. see LMAI_XXXX */ LUSTRE_ORPHAN_FL = 0x00002000, - LUSTRE_SET_SYNC_FL = 0x00040000, /* Synchronous setattr on OSTs */ LUSTRE_LMA_FL_MASKS = LUSTRE_ORPHAN_FL, }; diff --git a/lustre/mdd/mdd_object.c b/lustre/mdd/mdd_object.c index 7b0f46b..8c29485 100644 --- a/lustre/mdd/mdd_object.c +++ b/lustre/mdd/mdd_object.c @@ -861,11 +861,10 @@ int mdd_attr_set(const struct lu_env *env, struct md_object *obj, { struct mdd_object *mdd_obj = md2mdd_obj(obj); struct mdd_device *mdd = mdo2mdd(obj); - struct thandle *handle = NULL; + struct thandle *handle; struct lu_attr *la_copy = &mdd_env_info(env)->mti_la_for_fix; struct lu_attr *attr = MDD_ENV_VAR(env, cattr); const struct lu_attr *la = &ma->ma_attr; - struct lu_ucred *uc; int rc; ENTRY; @@ -891,38 +890,17 @@ int mdd_attr_set(const struct lu_env *env, struct md_object *obj, RETURN(0); } - /* If an unprivileged user changes group of some file, - * the setattr operation will be processed synchronously to - * honor the quota limit of the corresponding group. see LU-5152 */ - uc = lu_ucred_check(env); - if (S_ISREG(attr->la_mode) && la->la_valid & LA_GID && - la->la_gid != attr->la_gid && uc != NULL && uc->uc_fsuid != 0) { - la_copy->la_valid |= LA_FLAGS; - la_copy->la_flags |= LUSTRE_SET_SYNC_FL; - - /* Flush the possible existing sync requests to OSTs to - * keep the order of sync for the current setattr operation - * will be sent directly to OSTs. see LU-5152 */ - rc = dt_sync(env, mdd->mdd_child); - if (rc) - GOTO(out, rc); - } - - handle = mdd_trans_create(env, mdd); - if (IS_ERR(handle)) { - rc = PTR_ERR(handle); - handle = NULL; - - GOTO(out, rc); - } + handle = mdd_trans_create(env, mdd); + if (IS_ERR(handle)) + RETURN(PTR_ERR(handle)); rc = mdd_declare_attr_set(env, mdd, mdd_obj, la_copy, handle); - if (rc) - GOTO(out, rc); + if (rc) + GOTO(stop, rc); - rc = mdd_trans_start(env, mdd, handle); - if (rc) - GOTO(out, rc); + rc = mdd_trans_start(env, mdd, handle); + if (rc) + GOTO(stop, rc); if (mdd->mdd_sync_permission && permission_needs_sync(attr, la)) handle->th_sync = 1; @@ -932,25 +910,20 @@ int mdd_attr_set(const struct lu_env *env, struct md_object *obj, la->la_mtime, la->la_ctime); mdd_write_lock(env, mdd_obj, MOR_TGT_CHILD); - if (la_copy->la_valid) { + if (la_copy->la_valid & LA_FLAGS) + rc = mdd_attr_set_internal(env, mdd_obj, la_copy, handle, 1); + else if (la_copy->la_valid) /* setattr */ rc = mdd_attr_set_internal(env, mdd_obj, la_copy, handle, 1); - - if (rc == -EDQUOT && la_copy->la_flags & LUSTRE_SET_SYNC_FL) { - /* rollback to the original gid */ - la_copy->la_flags &= ~LUSTRE_SET_SYNC_FL; - la_copy->la_gid = attr->la_gid; - mdd_attr_set_internal(env, mdd_obj, la_copy, handle, 1); - } - } mdd_write_unlock(env, mdd_obj); -out: if (rc == 0) rc = mdd_attr_set_changelog(env, obj, handle, la_copy->la_valid); - if (handle != NULL) - rc = mdd_trans_stop(env, mdd, rc, handle); + GOTO(stop, rc); + +stop: + rc = mdd_trans_stop(env, mdd, rc, handle); return rc; } diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c index a0274c1..6150c6b 100644 --- a/lustre/osd-ldiskfs/osd_handler.c +++ b/lustre/osd-ldiskfs/osd_handler.c @@ -2430,7 +2430,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, struct osd_object *obj, struct osd_thandle *oh, long long bspace, qid_t old_id, qid_t new_id, bool enforce, - unsigned type, bool ignore_edquot) + unsigned type) { int rc; struct osd_thread_info *info = osd_oti_get(env); @@ -2445,7 +2445,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, qi->lqi_space = 1; /* Reserve credits for the new id */ rc = osd_declare_qid(env, oh, qi, NULL, enforce, NULL); - if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) + if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) RETURN(rc); @@ -2454,7 +2454,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, qi->lqi_id.qid_uid = old_id; qi->lqi_space = -1; rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); - if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) + if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) RETURN(rc); @@ -2470,7 +2470,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, * to save credit reservation. */ rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); - if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) + if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) RETURN(rc); @@ -2479,7 +2479,7 @@ static int osd_declare_attr_qid(const struct lu_env *env, qi->lqi_id.qid_uid = old_id; qi->lqi_space = -bspace; rc = osd_declare_qid(env, oh, qi, obj, enforce, NULL); - if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) + if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; RETURN(rc); @@ -2517,30 +2517,22 @@ static int osd_declare_attr_set(const struct lu_env *env, if (attr == NULL || obj->oo_inode == NULL) RETURN(rc); - bspace = obj->oo_inode->i_blocks << 9; + bspace = obj->oo_inode->i_blocks; + bspace <<= obj->oo_inode->i_sb->s_blocksize_bits; bspace = toqb(bspace); /* Changing ownership is always preformed by super user, it should not - * fail with EDQUOT unless required explicitly. + * fail with EDQUOT. * * 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 || attr->la_valid & LA_GID) { - bool ignore_edquot = !(attr->la_flags & LUSTRE_SET_SYNC_FL); - - if (!ignore_edquot) - CDEBUG(D_QUOTA, "%s: enforce quota on UID %u, GID %u" - "(the quota space is %lld)\n", - obj->oo_inode->i_sb->s_id, attr->la_uid, - attr->la_gid, bspace); - /* USERQUOTA */ uid = i_uid_read(obj->oo_inode); enforce = (attr->la_valid & LA_UID) && (attr->la_uid != uid); rc = osd_declare_attr_qid(env, obj, oh, bspace, uid, - attr->la_uid, enforce, USRQUOTA, - true); + attr->la_uid, enforce, USRQUOTA); if (rc) RETURN(rc); @@ -2548,8 +2540,7 @@ static int osd_declare_attr_set(const struct lu_env *env, enforce = (attr->la_valid & LA_GID) && (attr->la_gid != gid); rc = osd_declare_attr_qid(env, obj, oh, bspace, i_gid_read(obj->oo_inode), - attr->la_gid, enforce, GRPQUOTA, - ignore_edquot); + attr->la_gid, enforce, GRPQUOTA); if (rc) RETURN(rc); @@ -2561,7 +2552,7 @@ static int osd_declare_attr_set(const struct lu_env *env, (attr->la_projid != projid); rc = osd_declare_attr_qid(env, obj, oh, bspace, (qid_t)projid, (qid_t)attr->la_projid, - enforce, PRJQUOTA, true); + enforce, PRJQUOTA); if (rc) RETURN(rc); } diff --git a/lustre/osd-zfs/osd_object.c b/lustre/osd-zfs/osd_object.c index f56e8fa..f93d498 100644 --- a/lustre/osd-zfs/osd_object.c +++ b/lustre/osd-zfs/osd_object.c @@ -828,7 +828,7 @@ static inline int qsd_transfer(const struct lu_env *env, struct qsd_instance *qsd, struct lquota_trans *trans, int qtype, __u64 orig_id, __u64 new_id, __u64 bspace, - struct lquota_id_info *qi, bool ignore_edquot) + struct lquota_id_info *qi) { int rc; @@ -845,7 +845,7 @@ static inline int qsd_transfer(const struct lu_env *env, qi->lqi_id.qid_uid = new_id; qi->lqi_space = 1; rc = qsd_op_begin(env, qsd, trans, qi, NULL); - if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) + if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) return rc; @@ -867,7 +867,7 @@ static inline int qsd_transfer(const struct lu_env *env, qi->lqi_id.qid_uid = new_id; qi->lqi_space = bspace; rc = qsd_op_begin(env, qsd, trans, qi, NULL); - if (ignore_edquot && (rc == -EDQUOT || rc == -EINPROGRESS)) + if (rc == -EDQUOT || rc == -EINPROGRESS) rc = 0; if (rc) return rc; @@ -937,11 +937,7 @@ static int osd_declare_attr_set(const struct lu_env *env, if (attr && (attr->la_valid & (LA_UID | LA_GID | LA_PROJID))) { sa_object_size(obj->oo_sa_hdl, &blksize, &bspace); - bspace = toqb(bspace * 512); - - CDEBUG(D_QUOTA, "%s: enforce quota on UID %u, GID %u," - "the quota space is %lld (%u)\n", osd->od_svname, - attr->la_uid, attr->la_gid, bspace, blksize); + bspace = toqb(bspace * blksize); } if (attr && attr->la_valid & LA_UID) { @@ -950,7 +946,7 @@ static int osd_declare_attr_set(const struct lu_env *env, rc = qsd_transfer(env, osd->od_quota_slave, &oh->ot_quota_trans, USRQUOTA, obj->oo_attr.la_uid, attr->la_uid, - bspace, &info->oti_qi, true); + bspace, &info->oti_qi); if (rc) GOTO(out, rc); } @@ -961,9 +957,7 @@ static int osd_declare_attr_set(const struct lu_env *env, rc = qsd_transfer(env, osd->od_quota_slave, &oh->ot_quota_trans, GRPQUOTA, obj->oo_attr.la_gid, attr->la_gid, - bspace, &info->oti_qi, - !(attr->la_flags & - LUSTRE_SET_SYNC_FL)); + bspace, &info->oti_qi); if (rc) GOTO(out, rc); } @@ -994,7 +988,7 @@ static int osd_declare_attr_set(const struct lu_env *env, &oh->ot_quota_trans, PRJQUOTA, obj->oo_attr.la_projid, attr->la_projid, bspace, - &info->oti_qi, true); + &info->oti_qi); if (rc) GOTO(out, rc); } diff --git a/lustre/osp/osp_object.c b/lustre/osp/osp_object.c index 0dc9b22..db5a854 100644 --- a/lustre/osp/osp_object.c +++ b/lustre/osp/osp_object.c @@ -709,37 +709,8 @@ static int osp_attr_set(const struct lu_env *env, struct dt_object *dt, RETURN(0); if (!is_only_remote_trans(th)) { - if (attr->la_flags & LUSTRE_SET_SYNC_FL) { - struct ptlrpc_request *req = NULL; - struct osp_update_request *update = NULL; - struct osp_device *osp = lu2osp_dev(dt->do_lu.lo_dev); - - update = osp_update_request_create(&osp->opd_dt_dev); - if (IS_ERR(update)) - RETURN(PTR_ERR(update)); - - rc = osp_update_rpc_pack(env, attr_set, update, - OUT_ASTTR_SET, - lu_object_fid(&dt->do_lu), - attr); - if (rc != 0) { - CERROR("%s: update error "DFID": rc = %d\n", - osp->opd_obd->obd_name, - PFID(lu_object_fid(&dt->do_lu)), rc); - - osp_update_request_destroy(env, update); - RETURN(rc); - } - - rc = osp_remote_sync(env, osp, update, &req); - if (req != NULL) - ptlrpc_req_finished(req); - - osp_update_request_destroy(env, update); - } else { - rc = osp_sync_add(env, o, MDS_SETATTR64_REC, th, attr); - /* XXX: send new uid/gid to OST ASAP? */ - } + rc = osp_sync_add(env, o, MDS_SETATTR64_REC, th, attr); + /* XXX: send new uid/gid to OST ASAP? */ } else { struct lu_attr *la; diff --git a/lustre/quota/qsd_handler.c b/lustre/quota/qsd_handler.c index 5ccd6f8..a8d842d 100644 --- a/lustre/quota/qsd_handler.c +++ b/lustre/quota/qsd_handler.c @@ -652,13 +652,6 @@ static bool qsd_acquire(const struct lu_env *env, struct lquota_entry *lqe, * rc < 0, something bad happened */ break; - /* if we have gotten some quota and stil wait more quota, - * it's better to give QMT some time to reclaim from clients */ - if (count > 0) { - set_current_state(TASK_INTERRUPTIBLE); - schedule_timeout(cfs_time_seconds(1)); - } - /* need to acquire more quota space from master */ rc = qsd_acquire_remote(env, lqe); } diff --git a/lustre/target/out_lib.c b/lustre/target/out_lib.c index 8e60dfff9..c267ed2 100644 --- a/lustre/target/out_lib.c +++ b/lustre/target/out_lib.c @@ -657,10 +657,6 @@ int out_attr_set_add_exec(const struct lu_env *env, struct dt_object *dt_obj, if (rc != 0) return rc; - if (attr->la_valid & LA_FLAGS && - attr->la_flags & LUSTRE_SET_SYNC_FL) - th->th_sync |= 1; - arg = tx_add_exec(ta, out_tx_attr_set_exec, out_tx_attr_set_undo, file, line); if (IS_ERR(arg)) -- 1.8.3.1