From b9ab6348153fd8d236871f75e5bd3793a49c9a45 Mon Sep 17 00:00:00 2001 From: Liang Zhen Date: Tue, 29 Mar 2011 00:05:16 +0800 Subject: [PATCH] LU-170 oscc_grow_count will never grow We are using req::rq_async_args.space[0] to store original value of oscc_grow_count, and using req::rq_async_args.pointer_arg[0] to store oscc, however, ptlrpc_async_args is a union, which means req::rq_async_args.space[0] will always be overwritten by a ossc (pointer), and osc_interpret_create will always get true on this condition "if (diff < (int) req->rq_async_args.space[0])" and reset oscc_grow_count to OST_MIN_PRECREATE and set OSCC_FLAG_LOW. Because it's very unsafe to use raw scratchpad directly, I also cleaned up all using of raw scratchpad in this patch. Change-Id: I56348c2ebaf27acb493185db73f3992a17610d98 Signed-off-by: Liang Zhen Reviewed-on: http://review.whamcloud.com/371 Tested-by: Hudson Reviewed-by: Oleg Drokin Reviewed-by: Johann Lombardi Reviewed-by: Fan Yong --- lustre/include/lustre_net.h | 2 +- lustre/ldlm/ldlm_lockd.c | 30 ++++++++++++++++++++---------- lustre/mdc/mdc_locks.c | 25 ++++++++++++++++++------- lustre/mdc/mdc_request.c | 19 +++++++++++++------ lustre/osc/osc_create.c | 17 ++++++++++------- lustre/ptlrpc/recov_thread.c | 17 ++++++++++++++--- 6 files changed, 76 insertions(+), 34 deletions(-) diff --git a/lustre/include/lustre_net.h b/lustre/include/lustre_net.h index a38059e..a361f14 100644 --- a/lustre/include/lustre_net.h +++ b/lustre/include/lustre_net.h @@ -236,7 +236,7 @@ struct ptlrpc_client { union ptlrpc_async_args { /** * Scratchpad for passing args to completion interpreter. Users - * cast to the struct of their choosing, and LASSERT that this is + * cast to the struct of their choosing, and CLASSERT that this is * big enough. For _tons_ of context, OBD_ALLOC a struct and store * a pointer to it here. The pointer_arg ensures this struct is at * least big enough for that. diff --git a/lustre/ldlm/ldlm_lockd.c b/lustre/ldlm/ldlm_lockd.c index cfedb83..c72f4b6 100644 --- a/lustre/ldlm/ldlm_lockd.c +++ b/lustre/ldlm/ldlm_lockd.c @@ -66,6 +66,11 @@ extern cfs_mem_cache_t *ldlm_lock_slab; static cfs_semaphore_t ldlm_ref_sem; static int ldlm_refcount; +struct ldlm_cb_async_args { + struct ldlm_cb_set_arg *ca_set_arg; + struct ldlm_lock *ca_lock; +}; + /* LDLM state */ static struct ldlm_state *ldlm_state; @@ -641,14 +646,11 @@ static int ldlm_handle_ast_error(struct ldlm_lock *lock, static int ldlm_cb_interpret(const struct lu_env *env, struct ptlrpc_request *req, void *data, int rc) { - struct ldlm_cb_set_arg *arg; - struct ldlm_lock *lock; + struct ldlm_cb_async_args *ca = data; + struct ldlm_cb_set_arg *arg = ca->ca_set_arg; + struct ldlm_lock *lock = ca->ca_lock; ENTRY; - LASSERT(data != NULL); - - arg = req->rq_async_args.pointer_arg[0]; - lock = req->rq_async_args.pointer_arg[1]; LASSERT(lock != NULL); if (rc != 0) { rc = ldlm_handle_ast_error(lock, req, rc, @@ -723,6 +725,7 @@ int ldlm_server_blocking_ast(struct ldlm_lock *lock, struct ldlm_lock_desc *desc, void *data, int flag) { + struct ldlm_cb_async_args *ca; struct ldlm_cb_set_arg *arg = data; struct ldlm_request *body; struct ptlrpc_request *req; @@ -749,8 +752,11 @@ int ldlm_server_blocking_ast(struct ldlm_lock *lock, if (req == NULL) RETURN(-ENOMEM); - req->rq_async_args.pointer_arg[0] = arg; - req->rq_async_args.pointer_arg[1] = lock; + CLASSERT(sizeof(*ca) <= sizeof(req->rq_async_args)); + ca = ptlrpc_req_async_args(req); + ca->ca_set_arg = arg; + ca->ca_lock = lock; + req->rq_interpret_reply = ldlm_cb_interpret; req->rq_no_resend = 1; @@ -811,6 +817,7 @@ int ldlm_server_completion_ast(struct ldlm_lock *lock, int flags, void *data) struct ldlm_cb_set_arg *arg = data; struct ldlm_request *body; struct ptlrpc_request *req; + struct ldlm_cb_async_args *ca; long total_enqueue_wait; int instant_cancel = 0; int rc = 0; @@ -839,8 +846,11 @@ int ldlm_server_completion_ast(struct ldlm_lock *lock, int flags, void *data) RETURN(rc); } - req->rq_async_args.pointer_arg[0] = arg; - req->rq_async_args.pointer_arg[1] = lock; + CLASSERT(sizeof(*ca) <= sizeof(req->rq_async_args)); + ca = ptlrpc_req_async_args(req); + ca->ca_set_arg = arg; + ca->ca_lock = lock; + req->rq_interpret_reply = ldlm_cb_interpret; req->rq_no_resend = 1; body = req_capsule_client_get(&req->rq_pill, &RMF_DLM_REQ); diff --git a/lustre/mdc/mdc_locks.c b/lustre/mdc/mdc_locks.c index a85dbfe..c3d242d 100644 --- a/lustre/mdc/mdc_locks.c +++ b/lustre/mdc/mdc_locks.c @@ -56,6 +56,12 @@ #include #include "mdc_internal.h" +struct mdc_getattr_args { + struct obd_export *ga_exp; + struct md_enqueue_info *ga_minfo; + struct ldlm_enqueue_info *ga_einfo; +}; + int it_disposition(struct lookup_intent *it, int flag) { return it->d.lustre.it_disposition & flag; @@ -958,11 +964,12 @@ int mdc_intent_lock(struct obd_export *exp, struct md_op_data *op_data, static int mdc_intent_getattr_async_interpret(const struct lu_env *env, struct ptlrpc_request *req, - void *unused, int rc) + void *args, int rc) { - struct obd_export *exp = req->rq_async_args.pointer_arg[0]; - struct md_enqueue_info *minfo = req->rq_async_args.pointer_arg[1]; - struct ldlm_enqueue_info *einfo = req->rq_async_args.pointer_arg[2]; + struct mdc_getattr_args *ga = args; + struct obd_export *exp = ga->ga_exp; + struct md_enqueue_info *minfo = ga->ga_minfo; + struct ldlm_enqueue_info *einfo = ga->ga_einfo; struct lookup_intent *it; struct lustre_handle *lockh; struct obd_device *obddev; @@ -1006,6 +1013,7 @@ int mdc_intent_getattr_async(struct obd_export *exp, struct md_op_data *op_data = &minfo->mi_data; struct lookup_intent *it = &minfo->mi_it; struct ptlrpc_request *req; + struct mdc_getattr_args *ga; struct obd_device *obddev = class_exp2obd(exp); struct ldlm_res_id res_id; /*XXX: Both MDS_INODELOCK_LOOKUP and MDS_INODELOCK_UPDATE are needed @@ -1036,9 +1044,12 @@ int mdc_intent_getattr_async(struct obd_export *exp, RETURN(rc); } - req->rq_async_args.pointer_arg[0] = exp; - req->rq_async_args.pointer_arg[1] = minfo; - req->rq_async_args.pointer_arg[2] = einfo; + CLASSERT(sizeof(*ga) <= sizeof(req->rq_async_args)); + ga = ptlrpc_req_async_args(req); + ga->ga_exp = exp; + ga->ga_minfo = minfo; + ga->ga_einfo = einfo; + req->rq_interpret_reply = mdc_intent_getattr_async_interpret; ptlrpcd_add_req(req, PSCOPE_OTHER); diff --git a/lustre/mdc/mdc_request.c b/lustre/mdc/mdc_request.c index 92c453c..e3eae10 100644 --- a/lustre/mdc/mdc_request.c +++ b/lustre/mdc/mdc_request.c @@ -60,6 +60,11 @@ #define REQUEST_MINOR 244 +struct mdc_renew_capa_args { + struct obd_capa *ra_oc; + renew_capa_cb_t ra_cb; +}; + static quota_interface_t *quota_interface; extern quota_interface_t mdc_quota_interface; @@ -2165,11 +2170,10 @@ int mdc_get_remote_perm(struct obd_export *exp, const struct lu_fid *fid, } static int mdc_interpret_renew_capa(const struct lu_env *env, - struct ptlrpc_request *req, void *unused, + struct ptlrpc_request *req, void *args, int status) { - struct obd_capa *oc = req->rq_async_args.pointer_arg[0]; - renew_capa_cb_t cb = req->rq_async_args.pointer_arg[1]; + struct mdc_renew_capa_args *ra = args; struct mdt_body *body = NULL; struct lustre_capa *capa; ENTRY; @@ -2189,7 +2193,7 @@ static int mdc_interpret_renew_capa(const struct lu_env *env, GOTO(out, capa = ERR_PTR(-EFAULT)); EXIT; out: - cb(oc, capa); + ra->ra_cb(ra->ra_oc, capa); return 0; } @@ -2197,6 +2201,7 @@ static int mdc_renew_capa(struct obd_export *exp, struct obd_capa *oc, renew_capa_cb_t cb) { struct ptlrpc_request *req; + struct mdc_renew_capa_args *ra; ENTRY; req = ptlrpc_request_alloc_pack(class_exp2cliimp(exp), &RQF_MDS_GETATTR, @@ -2210,8 +2215,10 @@ static int mdc_renew_capa(struct obd_export *exp, struct obd_capa *oc, mdc_pack_body(req, &oc->c_capa.lc_fid, oc, OBD_MD_FLOSSCAPA, 0, -1, 0); ptlrpc_request_set_replen(req); - req->rq_async_args.pointer_arg[0] = oc; - req->rq_async_args.pointer_arg[1] = cb; + CLASSERT(sizeof(*ra) <= sizeof(req->rq_async_args)); + ra = ptlrpc_req_async_args(req); + ra->ra_oc = oc; + ra->ra_cb = cb; req->rq_interpret_reply = mdc_interpret_renew_capa; ptlrpcd_add_req(req, PSCOPE_OTHER); RETURN(0); diff --git a/lustre/osc/osc_create.c b/lustre/osc/osc_create.c index 584a730..c18183e 100644 --- a/lustre/osc/osc_create.c +++ b/lustre/osc/osc_create.c @@ -68,6 +68,7 @@ struct osc_create_async_args { struct osc_creator *rq_oscc; struct lov_stripe_md *rq_lsm; struct obd_info *rq_oinfo; + int rq_grow_count; }; static int oscc_internal_create(struct osc_creator *oscc); @@ -76,7 +77,8 @@ static int handle_async_create(struct ptlrpc_request *req, int rc); static int osc_interpret_create(const struct lu_env *env, struct ptlrpc_request *req, void *data, int rc) { - struct osc_creator *oscc; + struct osc_create_async_args *args = ptlrpc_req_async_args(req); + struct osc_creator *oscc = args->rq_oscc; struct ost_body *body = NULL; struct ptlrpc_request *fake_req, *pos; ENTRY; @@ -87,7 +89,6 @@ static int osc_interpret_create(const struct lu_env *env, rc = -EPROTO; } - oscc = req->rq_async_args.pointer_arg[0]; LASSERT(oscc && (oscc->oscc_obd != LP_POISON)); cfs_spin_lock(&oscc->oscc_lock); @@ -98,13 +99,13 @@ static int osc_interpret_create(const struct lu_env *env, int diff =ostid_id(&body->oa.o_oi)- oscc->oscc_last_id; /* oscc_internal_create() stores the original value of - * grow_count in rq_async_args.space[0]. + * grow_count in osc_create_async_args::rq_grow_count. * We can't compare against oscc_grow_count directly, * because it may have been increased while the RPC * is in flight, so we would always find ourselves * having created fewer objects and decreasing the * precreate request size. b=18577 */ - if (diff < (int) req->rq_async_args.space[0]) { + if (diff < args->rq_grow_count) { /* the OST has not managed to create all the * objects we asked for */ oscc->oscc_grow_count = max(diff, @@ -190,6 +191,7 @@ exit_wakeup: static int oscc_internal_create(struct osc_creator *oscc) { + struct osc_create_async_args *args; struct ptlrpc_request *request; struct ost_body *body; ENTRY; @@ -236,8 +238,11 @@ static int oscc_internal_create(struct osc_creator *oscc) request->rq_request_portal = OST_CREATE_PORTAL; ptlrpc_at_set_req_timeout(request); body = req_capsule_client_get(&request->rq_pill, &RMF_OST_BODY); + args = ptlrpc_req_async_args(request); + args->rq_oscc = oscc; cfs_spin_lock(&oscc->oscc_lock); + args->rq_grow_count = oscc->oscc_grow_count; if (likely(fid_seq_is_mdt(oscc->oscc_oa.o_seq))) { body->oa.o_oi.oi_seq = oscc->oscc_oa.o_seq; @@ -249,10 +254,9 @@ static int oscc_internal_create(struct osc_creator *oscc) CWARN("o_seq: "LPU64" is not indicate any MDTs.\n", oscc->oscc_oa.o_seq); } + cfs_spin_unlock(&oscc->oscc_lock); body->oa.o_valid |= OBD_MD_FLID | OBD_MD_FLGROUP; - request->rq_async_args.space[0] = oscc->oscc_grow_count; - cfs_spin_unlock(&oscc->oscc_lock); CDEBUG(D_RPCTRACE, "prealloc through id "LPU64" (last seen "LPU64")\n", body->oa.o_id, oscc->oscc_last_id); @@ -261,7 +265,6 @@ static int oscc_internal_create(struct osc_creator *oscc) request->rq_no_delay = request->rq_no_resend = 1; ptlrpc_request_set_replen(request); - request->rq_async_args.pointer_arg[0] = oscc; request->rq_interpret_reply = osc_interpret_create; ptlrpcd_add_req(request, PSCOPE_OTHER); diff --git a/lustre/ptlrpc/recov_thread.c b/lustre/ptlrpc/recov_thread.c index 3aac145..38c4ad7 100644 --- a/lustre/ptlrpc/recov_thread.c +++ b/lustre/ptlrpc/recov_thread.c @@ -76,6 +76,10 @@ enum { LLOG_LCM_FL_EXIT = 1 << 1 }; +struct llcd_async_args { + struct llog_canceld_ctxt *la_ctxt; +}; + static void llcd_print(struct llog_canceld_ctxt *llcd, const char *func, int line) { @@ -186,9 +190,11 @@ llcd_copy(struct llog_canceld_ctxt *llcd, struct llog_cookie *cookies) */ static int llcd_interpret(const struct lu_env *env, - struct ptlrpc_request *req, void *noused, int rc) + struct ptlrpc_request *req, void *args, int rc) { - struct llog_canceld_ctxt *llcd = req->rq_async_args.pointer_arg[0]; + struct llcd_async_args *la = args; + struct llog_canceld_ctxt *llcd = la->la_ctxt; + CDEBUG(D_RPCTRACE, "Sent llcd %p (%d) - killing it\n", llcd, rc); llcd_free(llcd); return 0; @@ -204,6 +210,7 @@ static int llcd_send(struct llog_canceld_ctxt *llcd) char *bufs[2] = { NULL, (char *)llcd->llcd_cookies }; struct obd_import *import = NULL; struct llog_commit_master *lcm; + struct llcd_async_args *la; struct ptlrpc_request *req; struct llog_ctxt *ctxt; int rc; @@ -268,8 +275,12 @@ static int llcd_send(struct llog_canceld_ctxt *llcd) /* bug 5515 */ req->rq_request_portal = LDLM_CANCEL_REQUEST_PORTAL; req->rq_reply_portal = LDLM_CANCEL_REPLY_PORTAL; + req->rq_interpret_reply = (ptlrpc_interpterer_t)llcd_interpret; - req->rq_async_args.pointer_arg[0] = llcd; + + CLASSERT(sizeof(*la) <= sizeof(req->rq_async_args)); + la = ptlrpc_req_async_args(req); + la->la_ctxt = llcd; /* llog cancels will be replayed after reconnect so this will do twice * first from replay llog, second for resended rpc */ -- 1.8.3.1