From ef75b59800d643e666d7c44c20287a8002e0a166 Mon Sep 17 00:00:00 2001 From: Oleg Drokin Date: Mon, 5 Nov 2012 00:17:16 -0500 Subject: [PATCH] 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 Reviewed-on: http://review.whamcloud.com/4458 Reviewed-by: Peng Tao Tested-by: Hudson Tested-by: Maloo Reviewed-by: Fan Yong --- lustre/include/obd_support.h | 1 + lustre/llite/file.c | 10 +++++----- lustre/mdc/mdc_locks.c | 4 +++- lustre/mdt/mdt_open.c | 11 +++++++++++ lustre/tests/sanity.sh | 21 +++++++++++++++++++++ 5 files changed, 41 insertions(+), 6 deletions(-) diff --git a/lustre/include/obd_support.h b/lustre/include/obd_support.h index 56485d5..4981e35 100644 --- 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 --- 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 --- 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 --- 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 --- 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() { -- 1.8.3.1