LU-3202 osd: no inode::i_mutex inside osd_object_destroy

Originally, to control the race between the OI scrub inserting
OI mapping and the osd_object_destroy() removing OI mapping on
the same target, the inode::i_mutex was used.

But the unlink thread which called osd_object_destroy() already
started transaction handle. Such order is different from others
as to may cause some deadlock between transaction start and the
obtain inode::i_mutex.

So now, the osd_object_destroy() will not obtain inode::i_mutex,
instead, the OI scrub will check whether someone unlinked the
inode or not during the OI scrub rebuilding the OI mapping, and
remove the new-inserted OI mapping if the race happened.

Signed-off-by: Fan Yong <fan.yong@intel.com>
Change-Id: Ic5c2e2b1d967e52a2b980d4f6bcaed4bdcf8368b
Reviewed-on: http://review.whamcloud.com/6124
Tested-by: Hudson
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Tested-by: Cliff White <cliff.white@intel.com>
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/osd-ldiskfs/osd_handler.c b/lustre/osd-ldiskfs/osd_handler.c
index 8dbcf11..5fdccb3 100644 (file)
--- a/lustre/osd-ldiskfs/osd_handler.c
+++ b/lustre/osd-ldiskfs/osd_handler.c
@@ -2180,9 +2180,6 @@ static int osd_object_destroy(const struct lu_env *env,
        if (unlikely(fid_is_acct(fid)))
                RETURN(-EPERM);
 
-       /* Parallel control for OI scrub. For most of cases, there is no
-        * lock contention. So it will not affect unlink performance. */
-       mutex_lock(&inode->i_mutex);
        if (S_ISDIR(inode->i_mode)) {
                LASSERT(osd_inode_unlinked(inode) || inode->i_nlink == 1);
                /* it will check/delete the inode from remote parent,
@@ -2201,7 +2198,6 @@ static int osd_object_destroy(const struct lu_env *env,
        osd_trans_exec_op(env, th, OSD_OT_DESTROY);
 
         result = osd_oi_delete(osd_oti_get(env), osd, fid, th);
-       mutex_unlock(&inode->i_mutex);
 
         /* XXX: add to ext3 orphan list */
         /* rc = ext3_orphan_add(handle_t *handle, struct inode *inode) */

diff --git a/lustre/osd-ldiskfs/osd_scrub.c b/lustre/osd-ldiskfs/osd_scrub.c
index 1954b12..2da1191 100644 (file)
--- a/lustre/osd-ldiskfs/osd_scrub.c
+++ b/lustre/osd-ldiskfs/osd_scrub.c
@@ -79,6 +79,13 @@ static inline int osd_scrub_has_window(struct osd_scrub *scrub,
        return scrub->os_pos_current < ooc->ooc_pos_preload + SCRUB_WINDOW_SIZE;
 }
 
+/**
+ * update/insert/delete the specified OI mapping (@fid @id) according to the ops
+ *
+ * \retval   1, changed nothing
+ * \retval   0, changed successfully
+ * \retval -ve, on error
+ */
 static int osd_scrub_refresh_mapping(struct osd_thread_info *info,
                                     struct osd_device *dev,
                                     const struct lu_fid *fid,
@@ -112,13 +119,19 @@ static int osd_scrub_refresh_mapping(struct osd_thread_info *info,
                RETURN(-ENOMEM);
        }
 
-       if (ops == DTO_INDEX_UPDATE) {
+       switch (ops) {
+       case DTO_INDEX_UPDATE:
                rc = iam_update(jh, bag, (const struct iam_key *)oi_fid,
                                (struct iam_rec *)oi_id, ipd);
-       } else {
+               if (unlikely(rc == -ENOENT)) {
+                       /* Some unlink thread mat removed the OI mapping. */
+                       rc = 1;
+               }
+               break;
+       case DTO_INDEX_INSERT:
                rc = iam_insert(jh, bag, (const struct iam_key *)oi_fid,
                                (struct iam_rec *)oi_id, ipd);
-               if (rc == -EEXIST) {
+               if (unlikely(rc == -EEXIST)) {
                        rc = 1;
                        /* XXX: There are trouble things when adding OI
                         *      mapping for IGIF object, which may cause
@@ -152,6 +165,18 @@ static int osd_scrub_refresh_mapping(struct osd_thread_info *info,
                         *
                         *      Anyway, it is rare, only exists in theory. */
                }
+               break;
+       case DTO_INDEX_DELETE:
+               rc = iam_delete(jh, bag, (const struct iam_key *)oi_fid, ipd);
+               if (rc == -ENOENT) {
+                       /* It is normal that the unlink thread has removed the
+                        * OI mapping already. */
+                       rc = 1;
+               }
+               break;
+       default:
+               LASSERTF(0, "Unexpected ops %d\n", ops);
+               break;
        }
        osd_ipd_put(info->oti_env, bag, ipd);
        ldiskfs_journal_stop(jh);
@@ -444,10 +469,8 @@ iget:
                        GOTO(out, rc);
                }
 
-               /* Prevent the inode to be unlinked during OI scrub. */
-               mutex_lock(&inode->i_mutex);
+               /* Check whether the inode to be unlinked during OI scrub. */
                if (unlikely(inode->i_nlink == 0)) {
-                       mutex_unlock(&inode->i_mutex);
                        iput(inode);
                        GOTO(out, rc = 0);
                }
@@ -494,7 +517,11 @@ out:
        }
 
        if (ops == DTO_INDEX_INSERT) {
-               mutex_unlock(&inode->i_mutex);
+               /* There may be conflict unlink during the OI scrub,
+                * if happend, then remove the new added OI mapping. */
+               if (unlikely(inode->i_nlink == 0))
+                       osd_scrub_refresh_mapping(info, dev, fid, lid,
+                                                 DTO_INDEX_DELETE);
                iput(inode);
        }
        up_write(&scrub->os_rwsem);