Branch:b1_6

Fix lov_mds_md in mdd_create_objects error handler.
Checking whether obd_fail before fsfilt_start.
Free fcc no matter whether fsfilt_commit success or failed.
b=10818
i=adilger
i=green

diff --git a/lustre/ChangeLog b/lustre/ChangeLog
index 9be0f36..b324048 100644 (file)
--- a/lustre/ChangeLog
+++ b/lustre/ChangeLog
@@ -170,6 +170,18 @@ Description: Short directio read returns full requested size rather than
 Details    : Direct I/O operations should return actual amount of bytes
              transferred rather than requested size.
 
+Severity   : normal
+Frequency  : rare 
+Bugzilla   : 10818
+Description: Memory leak in recovery
+Details    : Lov_mds_md was not free in an error handler in mds_create_object.
+             It should also check obd_fail before fsfilt_start, otherwise if 
+             fsfilt_start return -EROFS,(failover mds during mds recovery).
+             then the req will return with repmsg->transno = 0 and rc = EROFS. 
+             and we met hit the assert LASSERT(req->rq_reqmsg->transno ==
+             req->rq_repmsg->transno) in ptlrpc_replay_interpret.  Fcc should 
+             be freed no matter whether fsfilt_commit success or not.
+
 --------------------------------------------------------------------------------
 
 2007-05-03  Cluster File Systems, Inc. <info@clusterfs.com>

diff --git a/lustre/mds/mds_internal.h b/lustre/mds/mds_internal.h
index af32d2f..c5355e9 100644 (file)
--- a/lustre/mds/mds_internal.h
+++ b/lustre/mds/mds_internal.h
@@ -132,7 +132,8 @@ int enqueue_ordered_locks(struct obd_device *obd, struct ldlm_res_id *p1_res_id,
                           ldlm_policy_data_t *p2_policy);
 void mds_commit_cb(struct obd_device *, __u64 last_rcvd, void *data, int error);
 int mds_finish_transno(struct mds_obd *mds, struct inode *inode, void *handle,
-                       struct ptlrpc_request *req, int rc, __u32 op_data);
+                       struct ptlrpc_request *req, int rc, __u32 op_data, 
+                       int force_sync);
 void mds_reconstruct_generic(struct ptlrpc_request *req);
 void mds_req_from_mcd(struct ptlrpc_request *req, struct mds_client_data *mcd);
 int mds_get_parent_child_locked(struct obd_device *obd, struct mds_obd *mds,

diff --git a/lustre/mds/mds_join.c b/lustre/mds/mds_join.c
index d0bb7f2..a0ab48e 100644 (file)
--- a/lustre/mds/mds_join.c
+++ b/lustre/mds/mds_join.c
@@ -477,7 +477,7 @@ int mds_join_file(struct mds_update_record *rec, struct ptlrpc_request *req,
                       sizeof(struct lov_mds_md_join), "lov");
         mds_finish_join(mds, req, head_inode, head_lmmj);
 cleanup:
-        rc = mds_finish_transno(mds, head_inode, handle, req, rc, 0);
+        rc = mds_finish_transno(mds, head_inode, handle, req, rc, 0, 0);
         switch(cleanup_phase){
         case 3:
                 llog_close(llh_head);

diff --git a/lustre/mds/mds_open.c b/lustre/mds/mds_open.c
index d12865b..0bb9878 100644 (file)
--- a/lustre/mds/mds_open.c
+++ b/lustre/mds/mds_open.c
@@ -475,13 +475,14 @@ static int mds_create_objects(struct ptlrpc_request *req, int offset,
         if (IS_ERR(*handle)) {
                 rc = PTR_ERR(*handle);
                 *handle = NULL;
-                GOTO(out_oa, rc);
+                GOTO(free_diskmd, rc);
         }
 
         rc = fsfilt_set_md(obd, inode, *handle, lmm, lmm_size, "lov");
         lmm_buf = lustre_msg_buf(req->rq_repmsg, offset, lmm_size);
         LASSERT(lmm_buf);
         memcpy(lmm_buf, lmm, lmm_size);
+ free_diskmd:
         obd_free_diskmd(mds->mds_osc_exp, &lmm);
  out_oa:
         oti_free_cookies(&oti);
@@ -797,7 +798,7 @@ static int mds_open_by_fid(struct ptlrpc_request *req, struct ll_fid *fid,
 
         rc = mds_finish_open(req, dchild, body, flags, &handle, rec, rep, NULL);
         rc = mds_finish_transno(mds, dchild->d_inode, handle,
-                                req, rc, rep ? rep->lock_policy_res1 : 0);
+                                req, rc, rep ? rep->lock_policy_res1 : 0, 0);
         /* XXX what do we do here if mds_finish_transno itself failed? */
 
         l_dput(dchild);
@@ -1179,7 +1180,7 @@ found_child:
 
  cleanup:
         rc = mds_finish_transno(mds, dchild ? dchild->d_inode : NULL, handle,
-                                req, rc, rep ? rep->lock_policy_res1 : 0);
+                                req, rc, rep ? rep->lock_policy_res1 : 0, 0);
 
  cleanup_no_trans:
         switch (cleanup_phase) {
@@ -1390,7 +1391,7 @@ out:
 
  cleanup:
         if (req != NULL && reply_body != NULL) {
-                rc = mds_finish_transno(mds, pending_dir, handle, req, rc, 0);
+                rc = mds_finish_transno(mds, pending_dir, handle, req, rc, 0, 0);
         } else if (handle) {
                 int err = fsfilt_commit(obd, pending_dir, handle, 0);
                 if (err) {

diff --git a/lustre/mds/mds_reint.c b/lustre/mds/mds_reint.c
index 75b9dbe..503d2b7 100644 (file)
--- a/lustre/mds/mds_reint.c
+++ b/lustre/mds/mds_reint.c
@@ -102,7 +102,8 @@ static void mds_cancel_cookies_cb(struct obd_device *obd, __u64 transno,
 
 /* Assumes caller has already pushed us into the kernel context. */
 int mds_finish_transno(struct mds_obd *mds, struct inode *inode, void *handle,
-                       struct ptlrpc_request *req, int rc, __u32 op_data)
+                       struct ptlrpc_request *req, int rc, __u32 op_data, 
+                       int force_sync)
 {
         struct mds_export_data *med = &req->rq_export->exp_mds_data;
         struct mds_client_data *mcd = med->med_mcd;
@@ -119,7 +120,7 @@ int mds_finish_transno(struct mds_obd *mds, struct inode *inode, void *handle,
         }
 
         /* if the export has already been failed, we have no last_rcvd slot */
-        if (req->rq_export->exp_failed) {
+        if (req->rq_export->exp_failed || obd->obd_fail) {
                 CWARN("commit transaction for disconnected client %s: rc %d\n",
                       req->rq_export->exp_client_uuid.uuid, rc);
                 if (rc == 0)
@@ -184,11 +185,18 @@ int mds_finish_transno(struct mds_obd *mds, struct inode *inode, void *handle,
                 CERROR("client idx %d has offset %lld\n", med->med_lr_idx, off);
                 err = -EINVAL;
         } else {
-                fsfilt_add_journal_cb(req->rq_export->exp_obd, transno, handle,
-                                      mds_commit_cb, NULL);
+                struct obd_export *exp = req->rq_export;
+
+                if (!force_sync)
+                        force_sync = fsfilt_add_journal_cb(exp->exp_obd,transno, 
+                                                          handle, mds_commit_cb,
+                                                          NULL);
+
                 err = fsfilt_write_record(obd, mds->mds_rcvd_filp, mcd,
-                                          sizeof(*mcd), &off,
-                                          req->rq_export->exp_need_sync);
+                                          sizeof(*mcd), &off, 
+                                          force_sync | exp->exp_need_sync);
+                if (force_sync)
+                        mds_commit_cb(obd, transno, NULL, err);
         }
 
         if (err) {
@@ -495,7 +503,7 @@ static int mds_reint_setattr(struct mds_update_record *rec, int offset,
         struct lov_mds_md *lmm = NULL;
         struct llog_cookie *logcookies = NULL;
         int lmm_size = 0, need_lock = 1, cookie_size = 0;
-        int rc = 0, cleanup_phase = 0, err, locked = 0;
+        int rc = 0, cleanup_phase = 0, err, locked = 0, sync = 0;
         unsigned int qcids[MAXQUOTAS] = { 0, 0 };
         unsigned int qpids[MAXQUOTAS] = { rec->ur_iattr.ia_uid, 
                                           rec->ur_iattr.ia_gid };
@@ -674,9 +682,9 @@ static int mds_reint_setattr(struct mds_update_record *rec, int offset,
         EXIT;
  cleanup:
         if (mlcd != NULL)
-                fsfilt_add_journal_cb(req->rq_export->exp_obd, 0, handle,
-                                      mds_cancel_cookies_cb, mlcd);
-        err = mds_finish_transno(mds, inode, handle, req, rc, 0);
+                sync = fsfilt_add_journal_cb(req->rq_export->exp_obd, 0, handle,
+                                             mds_cancel_cookies_cb, mlcd);
+        err = mds_finish_transno(mds, inode, handle, req, rc, 0, sync);
         /* do mds to ost setattr if needed */
         if (!rc && !err && lmm_size)
                 mds_osc_setattr_async(obd, inode, lmm, lmm_size,
@@ -933,7 +941,7 @@ static int mds_reint_create(struct mds_update_record *rec, int offset,
         EXIT;
 
 cleanup:
-        err = mds_finish_transno(mds, dir, handle, req, rc, 0);
+        err = mds_finish_transno(mds, dir, handle, req, rc, 0, 0);
 
         if (rc && created) {
                 /* Destroy the file we just created.  This should not need
@@ -1724,7 +1732,7 @@ cleanup:
         }
 
         rc = mds_finish_transno(mds, dparent ? dparent->d_inode : NULL,
-                                handle, req, rc, 0);
+                                handle, req, rc, 0, 0);
         if (!rc)
                 (void)obd_set_info_async(mds->mds_osc_exp, strlen("unlinked"),
                                          "unlinked", 0, NULL, NULL);
@@ -1874,7 +1882,7 @@ static int mds_reint_link(struct mds_update_record *rec, int offset,
                 CERROR("vfs_link error %d\n", rc);
 cleanup:
         rc = mds_finish_transno(mds, de_tgt_dir ? de_tgt_dir->d_inode : NULL,
-                                handle, req, rc, 0);
+                                handle, req, rc, 0, 0);
         EXIT;
 
         switch (cleanup_phase) {
@@ -2266,7 +2274,7 @@ no_unlink:
         GOTO(cleanup, rc);
 cleanup:
         rc = mds_finish_transno(mds, de_tgtdir ? de_tgtdir->d_inode : NULL,
-                                handle, req, rc, 0);
+                                handle, req, rc, 0, 0);
 
         switch (cleanup_phase) {
         case 4:

diff --git a/lustre/mds/mds_xattr.c b/lustre/mds/mds_xattr.c
index 5b16864..8a049d8 100644 (file)
--- a/lustre/mds/mds_xattr.c
+++ b/lustre/mds/mds_xattr.c
@@ -310,7 +310,7 @@ int mds_setxattr_internal(struct ptlrpc_request *req, struct mds_body *body)
 
         LASSERT(rc <= 0);
 out_trans:
-        err = mds_finish_transno(mds, inode, handle, req, rc, 0);
+        err = mds_finish_transno(mds, inode, handle, req, rc, 0, 0);
 
 out_dput:
         l_dput(de);

diff --git a/lustre/obdfilter/filter.c b/lustre/obdfilter/filter.c
index 38b80ad..c643b1b 100644 (file)
--- a/lustre/obdfilter/filter.c
+++ b/lustre/obdfilter/filter.c
@@ -73,7 +73,7 @@ static void filter_commit_cb(struct obd_device *obd, __u64 transno,
 
 /* Assumes caller has already pushed us into the kernel context. */
 int filter_finish_transno(struct obd_export *exp, struct obd_trans_info *oti,
-                          int rc)
+                          int rc, int force_sync)
 {
         struct filter_obd *filter = &exp->exp_obd->u.filter;
         struct filter_export_data *fed = &exp->exp_filter_data;
@@ -115,11 +115,18 @@ int filter_finish_transno(struct obd_export *exp, struct obd_trans_info *oti,
                        fed->fed_lr_idx, fed->fed_lr_off);
                 err = -EINVAL;
         } else {
-                fsfilt_add_journal_cb(exp->exp_obd, last_rcvd, oti->oti_handle,
-                                      filter_commit_cb, NULL);
+                if (!force_sync)
+                        force_sync = fsfilt_add_journal_cb(exp->exp_obd, 
+                                                           last_rcvd,
+                                                           oti->oti_handle,
+                                                           filter_commit_cb,
+                                                           NULL);
+
                 err = fsfilt_write_record(exp->exp_obd, filter->fo_rcvd_filp,
                                           fcd, sizeof(*fcd), &off,
-                                          exp->exp_need_sync);
+                                          force_sync | exp->exp_need_sync);
+                if (force_sync)
+                        filter_commit_cb(exp->exp_obd, last_rcvd, NULL, err);
         }
         if (err) {
                 log_pri = D_ERROR;
@@ -2341,7 +2348,7 @@ int filter_setattr_internal(struct obd_export *exp, struct dentry *dentry,
         unsigned int orig_ids[MAXQUOTAS] = {0, 0};
         struct llog_cookie *fcc = NULL;
         struct filter_obd *filter;
-        int rc, err, locked = 0;
+        int rc, err, locked = 0, sync = 0;
         unsigned int ia_valid;
         struct inode *inode;
         struct iattr iattr;
@@ -2421,18 +2428,11 @@ int filter_setattr_internal(struct obd_export *exp, struct dentry *dentry,
                                       EXT3_IOC_SETFLAGS, (long)&oa->o_flags);
         } else {
                 rc = fsfilt_setattr(exp->exp_obd, dentry, handle, &iattr, 1);
-                if (fcc != NULL) {
+                if (fcc != NULL)
                         /* set cancel cookie callback function */
-                        if (fsfilt_add_journal_cb(exp->exp_obd, 0, handle,
-                                                  filter_cancel_cookies_cb,
-                                                  fcc)) {
-                                spin_lock(&exp->exp_lock);
-                                exp->exp_need_sync = 1;
-                                spin_unlock(&exp->exp_lock);
-                        } else {
-                                fcc = NULL;
-                }
-        }
+                        sync = fsfilt_add_journal_cb(exp->exp_obd, 0, handle,
+                                                     filter_cancel_cookies_cb,
+                                                     fcc);
         }
 
         if (OBD_FAIL_CHECK(OBD_FAIL_OST_SETATTR_CREDITS))
@@ -2441,13 +2441,19 @@ int filter_setattr_internal(struct obd_export *exp, struct dentry *dentry,
         /* The truncate might have used up our transaction credits.  Make
          * sure we have one left for the last_rcvd update. */
         err = fsfilt_extend(exp->exp_obd, inode, 1, handle);
-        rc = filter_finish_transno(exp, oti, rc);
+        rc = filter_finish_transno(exp, oti, rc, sync);
+        if (sync) {
+                filter_cancel_cookies_cb(exp->exp_obd, 0, fcc, rc);
+                fcc = NULL;
+        }
 
         err = fsfilt_commit(exp->exp_obd, inode, handle, 0);
         if (err) {
                 CERROR("error on commit, err = %d\n", err);
                 if (!rc)
                         rc = err;
+        } else {
+                fcc = NULL;
         }
 
         if (locked) {
@@ -2993,7 +2999,7 @@ int filter_destroy(struct obd_export *exp, struct obdo *oa,
         struct lvfs_run_ctxt saved;
         void *handle = NULL;
         struct llog_cookie *fcc = NULL;
-        int rc, rc2, cleanup_phase = 0;
+        int rc, rc2, cleanup_phase = 0, sync = 0;
         struct iattr iattr;
         ENTRY;
 
@@ -3088,19 +3094,28 @@ int filter_destroy(struct obd_export *exp, struct obdo *oa,
 cleanup:
         switch(cleanup_phase) {
         case 4:
-                if (fcc != NULL) {
-                        if (fsfilt_add_journal_cb(obd, 0, oti ?
-                                                  oti->oti_handle : handle,
-                                                  filter_cancel_cookies_cb,
-                                                  fcc) == 0)
-                                fcc = NULL;
+                if (fcc != NULL)
+                        sync = fsfilt_add_journal_cb(obd, 0, oti ?
+                                                     oti->oti_handle : handle,
+                                                     filter_cancel_cookies_cb,
+                                                     fcc);
+                /* If add_journal_cb failed, then filter_finish_transno
+                 * will commit the handle and we will do a sync 
+                 * on commit. then we call callback directly to free 
+                 * the fcc. 
+                 */
+                rc = filter_finish_transno(exp, oti, rc, sync);
+                if (sync) {
+                        filter_cancel_cookies_cb(obd, 0, fcc, rc); 
+                        fcc = NULL;
                 }
-                rc = filter_finish_transno(exp, oti, rc);
                 rc2 = fsfilt_commit(obd, dparent->d_inode, handle, 0);
                 if (rc2) {
                         CERROR("error on commit, err = %d\n", rc2);
                         if (!rc)
                                 rc = rc2;
+                } else {
+                        fcc = NULL;
                 }
         case 3:
                 filter_parent_unlock(dparent);

diff --git a/lustre/obdfilter/filter_internal.h b/lustre/obdfilter/filter_internal.h
index 1c5d0f5..093976f 100644 (file)
--- a/lustre/obdfilter/filter_internal.h
+++ b/lustre/obdfilter/filter_internal.h
@@ -98,7 +98,8 @@ struct dentry *__filter_oa2dentry(struct obd_device *obd, struct obdo *oa,
                                   const char *what, int quiet);
 #define filter_oa2dentry(obd, oa) __filter_oa2dentry(obd, oa, __FUNCTION__, 0)
 
-int filter_finish_transno(struct obd_export *, struct obd_trans_info *, int rc);
+int filter_finish_transno(struct obd_export *, struct obd_trans_info *, int rc,
+                          int force_sync);
 __u64 filter_next_id(struct filter_obd *, struct obdo *);
 __u64 filter_last_id(struct filter_obd *, obd_gr group);
 int filter_update_fidea(struct obd_export *exp, struct inode *inode,

diff --git a/lustre/obdfilter/filter_io_24.c b/lustre/obdfilter/filter_io_24.c
index 49ca0fe..19645c8 100644 (file)
--- a/lustre/obdfilter/filter_io_24.c
+++ b/lustre/obdfilter/filter_io_24.c
@@ -255,7 +255,7 @@ int filter_direct_io(int rw, struct dentry *dchild, struct filter_iobuf *buf,
                 up(&inode->i_sem);
                 cleanup_phase = 3;
 
-                rc = filter_finish_transno(exp, oti, 0);
+                rc = filter_finish_transno(exp, oti, 0, 0);
                 if (rc)
                         GOTO(cleanup, rc);
 

diff --git a/lustre/obdfilter/filter_io_26.c b/lustre/obdfilter/filter_io_26.c
index 73bb316..b8b9a7c 100644 (file)
--- a/lustre/obdfilter/filter_io_26.c
+++ b/lustre/obdfilter/filter_io_26.c
@@ -532,7 +532,7 @@ int filter_direct_io(int rw, struct dentry *dchild, struct filter_iobuf *iobuf,
 
                 UNLOCK_INODE_MUTEX(inode);
 
-                rc2 = filter_finish_transno(exp, oti, 0);
+                rc2 = filter_finish_transno(exp, oti, 0, 0);
                 if (rc2 != 0) {
                         CERROR("can't close transaction: %d\n", rc2);
                         if (rc == 0)

diff --git a/lustre/obdfilter/filter_log.c b/lustre/obdfilter/filter_log.c
index 8cda048..0ffa396 100644 (file)
--- a/lustre/obdfilter/filter_log.c
+++ b/lustre/obdfilter/filter_log.c
@@ -107,6 +107,7 @@ void filter_cancel_cookies_cb(struct obd_device *obd, __u64 transno,
         if (error != 0) {
                 CDEBUG(D_INODE, "not cancelling llog cookie on error %d\n",
                        error);
+                OBD_FREE(cookie, sizeof(*cookie));
                 return;
         }
 

diff --git a/lustre/tests/replay-single.sh b/lustre/tests/replay-single.sh
index 35e5714..bf11532 100755 (executable)
--- a/lustre/tests/replay-single.sh
+++ b/lustre/tests/replay-single.sh
@@ -740,10 +740,12 @@ run_test 32 "close() notices client eviction; close() after client eviction"
 # Abort recovery before client complete
 test_33() {
     replay_barrier mds
-    touch $DIR/$tfile
+    createmany -o $DIR/$tfile-%d 100 
     fail_abort mds
     # this file should be gone, because the replay was aborted
     $CHECKSTAT -t file $DIR/$tfile && return 3
+    $CHECKSTAT -t file $DIR/$tfile-* && return 3 
+    unlinkmany $DIR/$tfile-%d 0 100
     return 0
 }
 run_test 33 "abort recovery before client does replay"

diff --git a/lustre/tests/test-framework.sh b/lustre/tests/test-framework.sh
index 2d9a6ee..9fc7125 100644 (file)
--- a/lustre/tests/test-framework.sh
+++ b/lustre/tests/test-framework.sh
@@ -188,7 +188,6 @@ unload_modules() {
         echo "$LEAK_PORTALS" 1>&2
         mv $TMP/debug $TMP/debug-leak.`date +%s` || true
         echo "Memory leaks detected"
-       [ "$LEAK_LUSTRE" -a $(echo $LEAK_LUSTRE | awk 'leaked=$8 {print leaked % 56}') == 0 ] && echo "ignoring known bug 10818" && return 0
         return 254
     fi
     echo "modules unloaded."