Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-6297

Move rename is_subdir check from MDD to MDT

Details

    • Bug
    • Resolution: Unresolved
    • Minor
    • None
    • Lustre 2.8.0
    • 3
    • 17632

    Description

      Move rename is_subdir check (mdo_is_subdir) from MDD to MDT to avoid duplicate lu object find for the same fid, which might cause lu_obect_find() hang.

      Attachments

        Issue Links

          Activity

            [LU-6297] Move rename is_subdir check from MDD to MDT

            well, if _is_subdir() is called with LDLM lock held and we checked the passed object does exists, then how deadlock is possible - all "parents" must exist by definition?

            bzzz Alex Zhuravlev added a comment - well, if _is_subdir() is called with LDLM lock held and we checked the passed object does exists, then how deadlock is possible - all "parents" must exist by definition?

            This is not urgent, but probably need it for final racer pass. Please only work on it if you have extra time. Thanks

            di.wang Di Wang (Inactive) added a comment - This is not urgent, but probably need it for final racer pass. Please only work on it if you have extra time. Thanks

            hmm, seems for old fid as well

                   mold = mdt_object_find(info->mti_env, info->mti_mdt, old_fid);
                    if (IS_ERR(mold))
                            GOTO(out_unlock_parents, rc = PTR_ERR(mold));
            
                    /* Check if @mtgtdir is subdir of @mold, before locking child
                     * to avoid reverse locking. */
                    rc = mdt_is_subdir(info, mtgtdir, old_fid);
                    if (rc)
                            GOTO(out_put_old, rc);
            
            di.wang Di Wang (Inactive) added a comment - hmm, seems for old fid as well mold = mdt_object_find(info->mti_env, info->mti_mdt, old_fid); if (IS_ERR(mold)) GOTO(out_unlock_parents, rc = PTR_ERR(mold)); /* Check if @mtgtdir is subdir of @mold, before locking child * to avoid reverse locking. */ rc = mdt_is_subdir(info, mtgtdir, old_fid); if (rc) GOTO(out_put_old, rc);

            oh, mostly this is because mdt_is_subdir is being called multiple times during rename, which can be avoided if we move subdir check to MDT. And also

               mnew = mdt_object_find(info->mti_env, info->mti_mdt, new_fid);
                            if (IS_ERR(mnew))
                                    GOTO(out_put_old, rc = PTR_ERR(mnew));
            
                            /* Before locking the target dir, check we do not replace
                             * a dir with a non-dir, otherwise it may deadlock with
                             * link op which tries to create a link in this dir
                             * back to this non-dir. */
                            if (S_ISDIR(lu_object_attr(&mnew->mot_obj)) &&
                                !S_ISDIR(lu_object_attr(&mold->mot_obj)))
                                    GOTO(out_put_new, rc = -EISDIR);
            
                            lh_oldp = &info->mti_lh[MDT_LH_OLD];
                            mdt_lock_reg_init(lh_oldp, LCK_EX);
                            rc = mdt_object_lock(info, mold, lh_oldp, MDS_INODELOCK_LOOKUP |
                                                 MDS_INODELOCK_XATTR);
                            if (rc != 0)
                                    GOTO(out_put_new, rc);
            
                            /* Check if @msrcdir is subdir of @mnew, before locking child
                             * to avoid reverse locking. */
                            rc = mdt_is_subdir(info, msrcdir, new_fid);    <---- new obj might be found again here.
                            if (rc)
                                    GOTO(out_unlock_old, rc);
            
            di.wang Di Wang (Inactive) added a comment - oh, mostly this is because mdt_is_subdir is being called multiple times during rename, which can be avoided if we move subdir check to MDT. And also mnew = mdt_object_find(info->mti_env, info->mti_mdt, new_fid); if (IS_ERR(mnew)) GOTO(out_put_old, rc = PTR_ERR(mnew)); /* Before locking the target dir, check we do not replace * a dir with a non-dir, otherwise it may deadlock with * link op which tries to create a link in this dir * back to this non-dir. */ if (S_ISDIR(lu_object_attr(&mnew->mot_obj)) && !S_ISDIR(lu_object_attr(&mold->mot_obj))) GOTO(out_put_new, rc = -EISDIR); lh_oldp = &info->mti_lh[MDT_LH_OLD]; mdt_lock_reg_init(lh_oldp, LCK_EX); rc = mdt_object_lock(info, mold, lh_oldp, MDS_INODELOCK_LOOKUP | MDS_INODELOCK_XATTR); if (rc != 0) GOTO(out_put_new, rc); /* Check if @msrcdir is subdir of @mnew, before locking child * to avoid reverse locking. */ rc = mdt_is_subdir(info, msrcdir, new_fid); <---- new obj might be found again here. if (rc) GOTO(out_unlock_old, rc);

            can you please describe the exact sequence?

            bzzz Alex Zhuravlev added a comment - can you please describe the exact sequence?

            People

              laisiyao Lai Siyao
              di.wang Di Wang (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated: