From: Alexander Boyko Date: Tue, 31 Dec 2019 15:17:58 +0000 (-0500) Subject: LU-13093 osd: fix osd_attr_set race X-Git-Tag: 2.13.52~70 X-Git-Url: https://git.whamcloud.com/?p=fs%2Flustre-release.git;a=commitdiff_plain;h=83ac160bd19bcedaa817a419a3912726b9a3bd9d LU-13093 osd: fix osd_attr_set race The race between tgt_brw_write->ofd_write_attr_set and ofd_attr_set took a place, and it could set a wrong attributes. ofd_write_attr_set() does checks and declarations and sleeps on ofd_read_lock. Another thread executes ofd_attr_set() and sets initial uid/gid. After that the first thread wakeups and sets another uid/gid. But ofd_write_attr_set should change attributes for initial time only. This also leads to a bug at credits check cause uid was changed between declaration and attr_set. osd_trans_exec_check(ATTR_SET) has a wrong place when xattr_set is called. Also xattr doesn't have osd_trans_exec_op. lustre-OST0001: opcode 0: used 9, used now 9, reserved 1 create: 0/0/0, destroy: 0/0/0 attr_set: 1/1/9, xattr_set: 2/274/0 write: 0/0/0, punch: 0/0/0, quota 6/6/0 insert: 0/0/0, delete: 0/0/0 ref_add: 0/0/0, ref_del: 0/0/0 LBUG Cray-bug-id: LUS-8133 Fixes: 9f79d4488 ("LU-10048 ofd: take local locks within transaction") Signed-off-by: Alexander Boyko Change-Id: Id36ff633b0d97fff345ec105e0aa1b14fccafce4 Reviewed-on: https://review.whamcloud.com/37117 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Andrew Perepechko Reviewed-by: Andriy Skulysh Reviewed-by: Oleg Drokin --- diff --git a/lustre/ofd/ofd_io.c b/lustre/ofd/ofd_io.c index b17014a..27f379d 100644 --- a/lustre/ofd/ofd_io.c +++ b/lustre/ofd/ofd_io.c @@ -937,11 +937,13 @@ ofd_write_attr_set(const struct lu_env *env, struct ofd_device *ofd, GOTO(out_tx, rc); } - rc = dt_declare_xattr_set(env, dt_obj, &info->fti_buf, - XATTR_NAME_FID, 0, th); - if (rc) - GOTO(out_tx, rc); - + if (oa->o_valid & (OBD_MD_FLFID | OBD_MD_FLOSTLAYOUT | + OBD_MD_LAYOUT_VERSION)) { + rc = dt_declare_xattr_set(env, dt_obj, &info->fti_buf, + XATTR_NAME_FID, 0, th); + if (rc) + GOTO(out_tx, rc); + } /* We don't need a transno for this operation which will be re-executed * anyway when the OST_WRITE (with a transno assigned) is replayed */ rc = dt_trans_start_local(env, ofd->ofd_osd , th); @@ -950,6 +952,17 @@ ofd_write_attr_set(const struct lu_env *env, struct ofd_device *ofd, ofd_read_lock(env, ofd_obj); + rc = ofd_attr_handle_id(env, ofd_obj, la, 0 /* !is_setattr */); + if (rc != 0) + GOTO(out_unlock, rc); + + if (!la->la_valid && !(oa->o_valid & + (OBD_MD_FLFID | OBD_MD_FLOSTLAYOUT | OBD_MD_LAYOUT_VERSION))) + /* no attributes to set */ + GOTO(out_unlock, rc = 0); + + + /* set uid/gid/projid */ if (la->la_valid) { rc = dt_attr_set(env, dt_obj, la, th); diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c index 1b71fc6..5c4756f 100644 --- a/lustre/osd-ldiskfs/osd_handler.c +++ b/lustre/osd-ldiskfs/osd_handler.c @@ -2795,9 +2795,10 @@ static int osd_declare_attr_set(const struct lu_env *env, RETURN(rc); gid = i_gid_read(obj->oo_inode); + CDEBUG(D_QUOTA, "declare uid %d -> %d gid %d -> %d\n", uid, + attr->la_uid, gid, attr->la_gid); 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), + rc = osd_declare_attr_qid(env, obj, oh, bspace, gid, attr->la_gid, enforce, GRPQUOTA, ignore_edquot); if (rc) @@ -2940,6 +2941,11 @@ static int osd_quota_transfer(struct inode *inode, const struct lu_attr *attr) (attr->la_valid & LA_GID && attr->la_gid != i_gid_read(inode))) { struct iattr iattr; + CDEBUG(D_QUOTA, + "executing dquot_transfer inode %ld uid %d -> %d gid %d -> %d\n", + inode->i_ino, i_uid_read(inode), attr->la_uid, + i_gid_read(inode), attr->la_gid); + dquot_initialize(inode); iattr.ia_valid = 0; if (attr->la_valid & LA_UID) @@ -3035,6 +3041,8 @@ static int osd_attr_set(const struct lu_env *env, ll_dirty_inode(inode, I_DIRTY_DATASYNC); + osd_trans_exec_check(env, handle, OSD_OT_ATTR_SET); + if (!(attr->la_valid & LA_FLAGS)) GOTO(out, rc); @@ -3053,6 +3061,9 @@ static int osd_attr_set(const struct lu_env *env, lma->lma_incompat |= lustre_to_lma_flags(attr->la_flags); lustre_lma_swab(lma); + + osd_trans_exec_op(env, handle, OSD_OT_XATTR_SET); + rc = __osd_xattr_set(info, inode, XATTR_NAME_LMA, lma, sizeof(*lma), XATTR_REPLACE); if (rc != 0) { @@ -3068,7 +3079,6 @@ static int osd_attr_set(const struct lu_env *env, osd_trans_exec_check(env, handle, OSD_OT_XATTR_SET); } out: - osd_trans_exec_check(env, handle, OSD_OT_ATTR_SET); return rc; }