diff --git a/lustre/ChangeLog b/lustre/ChangeLog index 5adb04d039e6f2e1a06815d9be71ef20e98bdb1d..638001a5cb9bc30e7f7d7c05d288d735e7a3933e 100644 --- a/lustre/ChangeLog +++ b/lustre/ChangeLog @@ -12,6 +12,15 @@ * Recommended e2fsprogs version: 1.40.2-cfs4 * Note that reiserfs quotas are disabled on SLES 10 in this kernel. +Severity : major +Bugzilla : 14260 +Frequency : rare, at shutdown +Description: access already free / zero obd_namespace. +Details : if client_disconnect_export was called without force flag set, + and exist connect request in flight, this can produce access to + NULL pointer (or already free pointer) when connect_interpret + store ocd flags in obd_namespace. + Severity : minor Bugzilla : 14418 Frequency : only at startup diff --git a/lustre/ldlm/ldlm_internal.h b/lustre/ldlm/ldlm_internal.h index 80555806dac63e7e00aa4faa9cfc924df4fe28fe..ebfbaacbc961795e64bc1f28069fead50cd87a3a 100644 --- a/lustre/ldlm/ldlm_internal.h +++ b/lustre/ldlm/ldlm_internal.h @@ -52,7 +52,8 @@ int ldlm_cancel_lru_local(struct ldlm_namespace *ns, struct list_head *cancels, int ldlm_resource_putref_locked(struct ldlm_resource *res); void ldlm_resource_insert_lock_after(struct ldlm_lock *original, struct ldlm_lock *new); - +int ldlm_namespace_free_prior(struct ldlm_namespace *ns); +int ldlm_namespace_free_post(struct ldlm_namespace *ns, int force); /* ldlm_lock.c */ /* Number of blocking/completion callbacks that will be sent in diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c index abf719f39687c9708a4cb53ffc408b434a92c241..3303ab5a9d2a3c5b67dfe17927ad7cc2772d4820 100644 --- a/lustre/ldlm/ldlm_lib.c +++ b/lustre/ldlm/ldlm_lib.c @@ -37,6 +37,8 @@ #include <lustre_dlm.h> #include <lustre_net.h> #include <lustre_sec.h> +#include "ldlm_internal.h" + /* @priority: if non-zero, move the selected to the list head * @create: if zero, only search in existed connections @@ -369,6 +371,7 @@ int client_connect_import(const struct lu_env *env, struct obd_import *imp = cli->cl_import; struct obd_export *exp; struct obd_connect_data *ocd; + struct ldlm_namespace *to_be_freed = NULL; int rc; ENTRY; @@ -425,7 +428,8 @@ int client_connect_import(const struct lu_env *env, if (rc) { out_ldlm: - ldlm_namespace_free(obd->obd_namespace, 0); + ldlm_namespace_free_prior(obd->obd_namespace); + to_be_freed = obd->obd_namespace; obd->obd_namespace = NULL; out_disco: cli->cl_conn_count--; @@ -435,6 +439,9 @@ out_disco: } out_sem: mutex_up(&cli->cl_sem); + if (to_be_freed) + ldlm_namespace_free_post(to_be_freed, 1); + return rc; } @@ -444,6 +451,7 @@ int client_disconnect_export(struct obd_export *exp) struct client_obd *cli; struct obd_import *imp; int rc = 0, err; + struct ldlm_namespace *to_be_freed = NULL; ENTRY; if (!obd) { @@ -483,14 +491,18 @@ int client_disconnect_export(struct obd_export *exp) ldlm_cli_cancel_unused(obd->obd_namespace, NULL, obd->obd_force ? LDLM_FL_LOCAL_ONLY:0, NULL); - ldlm_namespace_free(obd->obd_namespace, obd->obd_force); - obd->obd_namespace = NULL; + ldlm_namespace_free_prior(obd->obd_namespace); + to_be_freed = obd->obd_namespace; } - if (!obd->obd_force) - rc = ptlrpc_disconnect_import(imp, 0); + rc = ptlrpc_disconnect_import(imp, 0); ptlrpc_invalidate_import(imp); + /* set obd_namespace to NULL only after invalidate, because we can have + * some connect requests in flight, and his need store a connect flags + * in obd_namespace. bug 14260 */ + obd->obd_namespace = NULL; + ptlrpc_free_rq_pool(imp->imp_rq_pool); destroy_import(imp); cli->cl_import = NULL; @@ -502,6 +514,9 @@ int client_disconnect_export(struct obd_export *exp) rc = err; out_sem: mutex_up(&cli->cl_sem); + if (to_be_freed) + ldlm_namespace_free_post(to_be_freed, obd->obd_force); + RETURN(rc); } diff --git a/lustre/ldlm/ldlm_resource.c b/lustre/ldlm/ldlm_resource.c index 16d831f97269f9183934dc79ceeaba5ad1c5b498..2f74265e74519c0e01487de75831e23d68b65017 100644 --- a/lustre/ldlm/ldlm_resource.c +++ b/lustre/ldlm/ldlm_resource.c @@ -472,7 +472,7 @@ int ldlm_namespace_cleanup(struct ldlm_namespace *ns, int flags) } /* Cleanup, but also free, the namespace */ -int ldlm_namespace_free(struct ldlm_namespace *ns, int force) +int ldlm_namespace_free_prior(struct ldlm_namespace *ns) { ENTRY; if (!ns) @@ -487,19 +487,6 @@ int ldlm_namespace_free(struct ldlm_namespace *ns, int force) /* At shutdown time, don't call the cancellation callback */ ldlm_namespace_cleanup(ns, 0); -#ifdef LPROCFS - { - struct proc_dir_entry *dir; - dir = lprocfs_srch(ldlm_ns_proc_dir, ns->ns_name); - if (dir == NULL) { - CERROR("dlm namespace %s has no procfs dir?\n", - ns->ns_name); - } else { - lprocfs_remove(&dir); - } - } -#endif - if (ns->ns_refcount > 0) { struct l_wait_info lwi = LWI_INTR(LWI_ON_SIGNAL_NOOP, NULL); int rc; @@ -520,15 +507,60 @@ int ldlm_namespace_free(struct ldlm_namespace *ns, int force) "dlm namespace %s free done waiting\n", ns->ns_name); } + RETURN(ELDLM_OK); +} + +int ldlm_namespace_free_post(struct ldlm_namespace *ns, int force) +{ + ENTRY; + if (!ns) + RETURN(ELDLM_OK); + +#ifdef LPROCFS + { + struct proc_dir_entry *dir; + dir = lprocfs_srch(ldlm_ns_proc_dir, ns->ns_name); + if (dir == NULL) { + CERROR("dlm namespace %s has no procfs dir?\n", + ns->ns_name); + } else { + lprocfs_remove(&dir); + } + } +#endif OBD_VFREE(ns->ns_hash, sizeof(*ns->ns_hash) * RES_HASH_SIZE); OBD_FREE(ns->ns_name, strlen(ns->ns_name) + 1); OBD_FREE_PTR(ns); - ldlm_put_ref(force); - RETURN(ELDLM_OK); } + +/* Cleanup the resource, and free namespace. + * bug 12864: + * Deadlock issue: + * proc1: destroy import + * class_disconnect_export(grab cl_sem) -> + * -> ldlm_namespace_free -> + * -> lprocfs_remove(grab _lprocfs_lock). + * proc2: read proc info + * lprocfs_fops_read(grab _lprocfs_lock) -> + * -> osc_rd_active, etc(grab cl_sem). + * + * So that I have to split the ldlm_namespace_free into two parts - the first + * part ldlm_namespace_free_prior is used to cleanup the resource which is + * being used; the 2nd part ldlm_namespace_free_post is used to unregister the + * lprocfs entries, and then free memory. It will be called w/o cli->cl_sem + * held. + */ +int ldlm_namespace_free(struct ldlm_namespace *ns, int force) +{ + ldlm_namespace_free_prior(ns); + ldlm_namespace_free_post(ns, force); + return ELDLM_OK; +} + + void ldlm_namespace_get_nolock(struct ldlm_namespace *ns) { LASSERT(ns->ns_refcount >= 0); diff --git a/lustre/obdclass/obd_mount.c b/lustre/obdclass/obd_mount.c index 5c24bf1665a8281c6bfb9eadb35b26c950bd2a63..a09dc3618c8ff07c0fdc087fa516babc16c61fa7 100644 --- a/lustre/obdclass/obd_mount.c +++ b/lustre/obdclass/obd_mount.c @@ -770,9 +770,8 @@ static int lustre_stop_mgc(struct super_block *sb) GOTO(out, rc = -EBUSY); } - /* MGC should disconnect nicely so MGS won't print eviction messages */ - obd->obd_force = (lsi->lsi_flags & LSI_UMOUNT_FORCE) != 0; - /* The MGC has no recoverable data in any case. */ + /* The MGC has no recoverable data in any case. + * force shotdown set in umount_begin */ obd->obd_no_recov = 1; if (obd->u.cli.cl_mgc_mgsexp) @@ -1446,12 +1445,10 @@ static void server_put_super(struct super_block *sb) obd = class_name2obd(lsi->lsi_ldd->ldd_svname); if (obd) { CDEBUG(D_MOUNT, "stopping %s\n", obd->obd_name); - if (lsi->lsi_flags & LSI_UMOUNT_FORCE) - obd->obd_force = 1; if (lsi->lsi_flags & LSI_UMOUNT_FAILOVER) obd->obd_fail = 1; /* We can't seem to give an error return code - to .put_super, so we better make sure we clean up! */ + * to .put_super, so we better make sure we clean up! */ obd->obd_force = 1; class_manual_cleanup(obd); } else { diff --git a/lustre/ptlrpc/client.c b/lustre/ptlrpc/client.c index 765df313e85c7a7af6d0e119ae65346a56d4f6fa..e85a7f8899001135abc7d18fc7fde50bc312b9ea 100644 --- a/lustre/ptlrpc/client.c +++ b/lustre/ptlrpc/client.c @@ -1026,14 +1026,13 @@ check_ctx: if (req->rq_bulk != NULL) ptlrpc_unregister_bulk (req); - req->rq_phase = RQ_PHASE_COMPLETE; - if (req->rq_interpret_reply != NULL) { int (*interpreter)(struct ptlrpc_request *,void *,int) = req->rq_interpret_reply; req->rq_status = interpreter(req, &req->rq_async_args, req->rq_status); } + req->rq_phase = RQ_PHASE_COMPLETE; CDEBUG(D_RPCTRACE, "Completed RPC pname:cluuid:pid:xid:nid:" "opc %s:%s:%d:"LPU64":%s:%d\n", cfs_curproc_comm(), diff --git a/lustre/ptlrpc/import.c b/lustre/ptlrpc/import.c index 25978724348edef4ff9ee3223ec1cb1ab9fd831a..737a86aa0105dbc181f0498a02c7160d1e220bdb 100644 --- a/lustre/ptlrpc/import.c +++ b/lustre/ptlrpc/import.c @@ -1021,8 +1021,12 @@ int ptlrpc_disconnect_import(struct obd_import *imp, int noclose) { struct ptlrpc_request *req; int rq_opc, rc = 0; + int nowait = imp->imp_obd->obd_force; ENTRY; + if (nowait) + GOTO(set_state, rc); + switch (imp->imp_connect_op) { case OST_CONNECT: rq_opc = OST_DISCONNECT; break; case MDS_CONNECT: rq_opc = MDS_DISCONNECT; break; @@ -1074,6 +1078,7 @@ int ptlrpc_disconnect_import(struct obd_import *imp, int noclose) ptlrpc_req_finished(req); } +set_state: spin_lock(&imp->imp_lock); out: if (noclose)