Branch HEAD

b=20098
i=tappro
i=green

mod refcount is added to avoid usage after freeing

diff --git a/lustre/include/obd.h b/lustre/include/obd.h
index 3ff60f0..789ad5c 100644 (file)
--- a/lustre/include/obd.h
+++ b/lustre/include/obd.h
@@ -1485,6 +1485,7 @@ struct md_open_data {
         struct obd_client_handle *mod_och;
         struct ptlrpc_request    *mod_open_req;
         struct ptlrpc_request    *mod_close_req;
+        atomic_t                  mod_refcount;
 };
 
 struct lookup_intent;
@@ -1672,4 +1673,21 @@ static inline struct lustre_capa *oinfo_capa(struct obd_info *oinfo)
         return oinfo->oi_capa;
 }
 
+static inline struct md_open_data *obd_mod_alloc(void)
+{
+        struct md_open_data *mod;
+        OBD_ALLOC_PTR(mod);
+        if (mod == NULL)
+                return NULL;
+        atomic_set(&mod->mod_refcount, 1);
+        return mod;
+}
+
+#define obd_mod_get(mod) atomic_inc(&mod->mod_refcount)
+#define obd_mod_put(mod)                                \
+({                                                      \
+        if (atomic_dec_and_test(&mod->mod_refcount))    \
+                OBD_FREE_PTR(mod);                      \
+})
+
 #endif /* __OBD_H */

diff --git a/lustre/mdc/mdc_reint.c b/lustre/mdc/mdc_reint.c
index 58b97de..8354bcc 100644 (file)
--- a/lustre/mdc/mdc_reint.c
+++ b/lustre/mdc/mdc_reint.c
@@ -176,7 +176,7 @@ int mdc_setattr(struct obd_export *exp, struct md_op_data *op_data,
         {
                 LASSERT(*mod == NULL);
 
-                OBD_ALLOC_PTR(*mod);
+                *mod = obd_mod_alloc();
                 if (*mod == NULL) {
                         DEBUG_REQ(D_ERROR, req, "Can't allocate "
                                   "md_open_data");

diff --git a/lustre/mdc/mdc_request.c b/lustre/mdc/mdc_request.c
index c4de6cf..0708d1d 100644 (file)
--- a/lustre/mdc/mdc_request.c
+++ b/lustre/mdc/mdc_request.c
@@ -678,11 +678,15 @@ void mdc_commit_open(struct ptlrpc_request *req)
         if (mod == NULL)
                 return;
 
-        if (mod->mod_och != NULL)
-                mod->mod_och->och_mod = NULL;
+        /**
+         * No need to touch md_open_data::mod_och, it holds a reference on
+         * \var mod and will zero references to each other, \var mod will be
+         * freed after that when md_open_data::mod_och will put the reference.
+         */
 
-        OBD_FREE(mod, sizeof(*mod));
         req->rq_cb_data = NULL;
+        mod->mod_open_req = NULL;
+        obd_mod_put(mod);
 }
 
 int mdc_set_open_replay_data(struct obd_export *exp,
@@ -707,7 +711,7 @@ int mdc_set_open_replay_data(struct obd_export *exp,
 
         /* Only if the import is replayable, we set replay_open data */
         if (och && imp->imp_replayable) {
-                OBD_ALLOC_PTR(mod);
+                mod = obd_mod_alloc();
                 if (mod == NULL) {
                         DEBUG_REQ(D_ERROR, open_req,
                                   "Can't allocate md_open_data");
@@ -717,6 +721,7 @@ int mdc_set_open_replay_data(struct obd_export *exp,
                 spin_lock(&open_req->rq_lock);
                 och->och_mod = mod;
                 mod->mod_och = och;
+                obd_mod_get(mod);
                 mod->mod_open_req = open_req;
                 open_req->rq_cb_data = mod;
                 open_req->rq_commit_cb = mdc_commit_open;
@@ -743,17 +748,12 @@ int mdc_clear_open_replay_data(struct obd_export *exp,
         struct md_open_data *mod = och->och_mod;
         ENTRY;
 
-        /*
-         * Don't free the structure now (it happens in mdc_commit_open(), after
-         * we're sure we won't need to fix up the close request in the future),
-         * but make sure that replay doesn't poke at the och, which is about to
-         * be freed.
-         */
-        LASSERT(mod != LP_POISON);
-        if (mod != NULL)
-                mod->mod_och = NULL;
+        LASSERT(mod != LP_POISON && mod != NULL);
 
+        mod->mod_och = NULL;
         och->och_mod = NULL;
+        obd_mod_put(mod);
+
         RETURN(0);
 }
 
@@ -790,6 +790,8 @@ int mdc_close(struct obd_export *exp, struct md_op_data *op_data,
                          "POISONED open %p!\n", mod->mod_open_req);
 
                 mod->mod_close_req = req;
+                obd_mod_get(mod);
+
                 DEBUG_REQ(D_HA, mod->mod_open_req, "matched open");
                 /* We no longer want to preserve this open for replay even
                  * though the open was committed. b=3632, b=3633 */
@@ -841,9 +843,13 @@ int mdc_close(struct obd_export *exp, struct md_op_data *op_data,
                         rc = 0;
         }
 
-        if (rc != 0 && mod)
-                 mod->mod_close_req = NULL;
-
+        if (mod) {
+                if (rc != 0)
+                        mod->mod_close_req = NULL;
+                /* Since now, mod is accessed through open_req only,
+                 * thus close req does not keep a reference on mod anymore. */
+                obd_mod_put(mod);
+        }
         *request = req;
         RETURN(rc);
 }
@@ -873,6 +879,7 @@ int mdc_done_writing(struct obd_export *exp, struct md_op_data *op_data,
                          "POISONED setattr %p!\n", mod->mod_open_req);
 
                 mod->mod_close_req = req;
+                obd_mod_get(mod);
                 DEBUG_REQ(D_HA, mod->mod_open_req, "matched setattr");
                 /* We no longer want to preserve this setattr for replay even
                  * though the open was committed. b=3632, b=3633 */
@@ -898,6 +905,13 @@ int mdc_done_writing(struct obd_export *exp, struct md_op_data *op_data,
                         rc = 0;
         }
 
+        if (mod) {
+                if (rc != 0)
+                        mod->mod_close_req = NULL;
+                /* Since now, mod is accessed through setattr req only,
+                 * thus DW req does not keep a reference on mod anymore. */
+                obd_mod_put(mod);
+        }
         ptlrpc_req_finished(req);
         RETURN(rc);
 }
@@ -1437,7 +1451,7 @@ static int mdc_pin(struct obd_export *exp, const struct lu_fid *fid,
         handle->och_fh = body->handle;
         handle->och_magic = OBD_CLIENT_HANDLE_MAGIC;
 
-        OBD_ALLOC_PTR(handle->och_mod);
+        handle->och_mod = obd_mod_alloc();
         if (handle->och_mod == NULL) {
                 DEBUG_REQ(D_ERROR, req, "can't allocate md_open_data");
                 GOTO(err_out, rc = -ENOMEM);
@@ -1480,7 +1494,7 @@ static int mdc_unpin(struct obd_export *exp, struct obd_client_handle *handle,
         ptlrpc_req_finished(req);
         ptlrpc_req_finished(handle->och_mod->mod_open_req);
 
-        OBD_FREE(handle->och_mod, sizeof(*handle->och_mod));
+        obd_mod_put(handle->och_mod);
         RETURN(rc);
 }