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 <c17825@cray.com>
Change-Id: Id36ff633b0d97fff345ec105e0aa1b14fccafce4
Reviewed-on: https://review.whamcloud.com/37117
Tested-by: jenkins <devops@whamcloud.com>
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Andrew Perepechko <c17827@cray.com>
Reviewed-by: Andriy Skulysh <c17819@cray.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>

diff --git a/lustre/ofd/ofd_io.c b/lustre/ofd/ofd_io.c
index b17014a..27f379d 100644 (file)
--- 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 (file)
--- 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;
 }