LU-2275 mdc: Don't leak requests with "strange" dispositions

Make sure that a request with no transno will get its rq_replay reset,
also ensure that on error path from ll_file_open we drop reference to
request occupied by open if needed.

Change-Id: If4d59274d2574698567970821b94ef1c5c35385b
Signed-off-by: Oleg Drokin <green@whamcloud.com>
Reviewed-on: http://review.whamcloud.com/4458
Reviewed-by: Peng Tao <bergwolf@gmail.com>
Tested-by: Hudson
Tested-by: Maloo <whamcloud.maloo@gmail.com>
Reviewed-by: Fan Yong <yong.fan@whamcloud.com>

diff --git a/lustre/include/obd_support.h b/lustre/include/obd_support.h
index 56485d5..4981e35 100644 (file)
--- a/lustre/include/obd_support.h
+++ b/lustre/include/obd_support.h
@@ -232,6 +232,7 @@ int obd_alloc_fail(const void *ptr, const char *name, const char *type,
 #define OBD_FAIL_MDS_PDO_LOCK            0x145
 #define OBD_FAIL_MDS_PDO_LOCK2           0x146
 #define OBD_FAIL_MDS_OSC_CREATE_FAIL     0x147
+#define OBD_FAIL_MDS_NEGATIVE_POSITIVE  0x148
 
 /* CMD */
 #define OBD_FAIL_MDS_IS_SUBDIR_NET       0x180

diff --git a/lustre/llite/file.c b/lustre/llite/file.c
index 21011ae..4249f6d 100644 (file)
--- a/lustre/llite/file.c
+++ b/lustre/llite/file.c
@@ -651,11 +651,6 @@ restart:
         GOTO(out_och_free, rc);
 
 out_och_free:
-        if (it && it_disposition(it, DISP_ENQ_OPEN_REF)) {
-                ptlrpc_req_finished(it->d.lustre.it_data);
-                it_clear_disposition(it, DISP_ENQ_OPEN_REF);
-        }
-
         if (rc) {
                 if (och_p && *och_p) {
                         OBD_FREE(*och_p, sizeof (struct obd_client_handle));
@@ -673,6 +668,11 @@ out_openerr:
                 ll_stats_ops_tally(ll_i2sbi(inode), LPROC_LL_OPEN, 1);
         }
 
+       if (it && it_disposition(it, DISP_ENQ_OPEN_REF)) {
+               ptlrpc_req_finished(it->d.lustre.it_data);
+               it_clear_disposition(it, DISP_ENQ_OPEN_REF);
+       }
+
         return rc;
 }
 

diff --git a/lustre/mdc/mdc_locks.c b/lustre/mdc/mdc_locks.c
index 46d2bf5..925c2e5 100644 (file)
--- a/lustre/mdc/mdc_locks.c
+++ b/lustre/mdc/mdc_locks.c
@@ -505,7 +505,9 @@ static int mdc_finish_enqueue(struct obd_export *exp,
        intent->it_lock_handle = lockh->cookie;
        intent->it_data = req;
 
-       if (intent->it_status < 0 && req->rq_replay)
+       /* Technically speaking rq_transno must already be zero if
+        * it_status is in error, so the check is a bit redundant */
+       if ((!req->rq_transno || intent->it_status < 0) && req->rq_replay)
                mdc_clear_replay_flag(req, intent->it_status);
 
         /* If we're doing an IT_OPEN which did not result in an actual

diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c
index d1fe097..94b2531 100644 (file)
--- a/lustre/mdt/mdt_open.c
+++ b/lustre/mdt/mdt_open.c
@@ -827,6 +827,17 @@ int mdt_finish_open(struct mdt_thread_info *info,
         islnk = S_ISLNK(la->la_mode);
         mdt_pack_attr2body(info, repbody, la, mdt_object_fid(o));
 
+       /* LU-2275, simulate broken behaviour (esp. prevalent in
+        * pre-2.4 servers where a very strange reply is sent on error
+        * that looks like it was actually almost succesful and a failure at the
+        * same time */
+       if (OBD_FAIL_CHECK(OBD_FAIL_MDS_NEGATIVE_POSITIVE)) {
+               mdt_set_disposition(info, rep, DISP_OPEN_LOCK | \
+                                   DISP_OPEN_OPEN | DISP_LOOKUP_NEG | \
+                                   DISP_LOOKUP_POS);
+               RETURN(-ENOENT);
+       }
+
         if (exp_connect_rmtclient(exp)) {
                 void *buf = req_capsule_server_get(info->mti_pill, &RMF_ACL);
 

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index f3752a1..1485418 100644 (file)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -8794,6 +8794,27 @@ test_182() {
 }
 run_test 182 "Disable MDC RPCs semaphore wouldn't crash client ================"
 
+test_183() { # LU-2275
+       mkdir -p $DIR/$tdir || error "creating dir $DIR/$tdir"
+       echo aaa > $DIR/$tdir/$tfile
+
+#define OBD_FAIL_MDS_NEGATIVE_POSITIVE  0x148
+       do_facet $SINGLEMDS $LCTL set_param fail_loc=0x148
+
+       ls -l $DIR/$tdir && error "ls succeeded, should have failed"
+       cat $DIR/$tdir/$tfile && error "cat succeeded, should have failed"
+
+       do_facet $SINGLEMDS $LCTL set_param fail_loc=0
+
+       # Flush negative dentry cache
+       touch $DIR/$tdir/$tfile
+
+       # We are not checking for any leaked references here, they'll
+       # become evident next time we do cleanup with module unload.
+       rm -rf $DIR/$tdir
+}
+run_test 183 "No crash or request leak in case of strange dispositions ========"
+
 # OST pools tests
 check_file_in_pool()
 {