From 15212f7e716e1e3453ae466c73d0852c9daeaa99 Mon Sep 17 00:00:00 2001 From: shadow Date: Thu, 28 May 2009 14:23:42 +0000 Subject: [PATCH] resolve race between obd_disconnect and class_disconnect_exports Branch b_release_1_8_1 b=19528 i=tappro i=rread --- lustre/ChangeLog | 6 ++++++ lustre/ldlm/ldlm_lib.c | 11 +++++++---- lustre/obdclass/genops.c | 47 ++++++++++++++++++++++------------------------- 3 files changed, 35 insertions(+), 29 deletions(-) diff --git a/lustre/ChangeLog b/lustre/ChangeLog index 58642f8..d237721 100644 --- a/lustre/ChangeLog +++ b/lustre/ChangeLog @@ -14,6 +14,12 @@ tbd Sun Microsystems, Inc. of Lustre filesystem with 4K stack may cause a stack overflow. For more information, please refer to bugzilla 17630. +Severity : normal +Bugzilla : 19528 +Description: resolve race between obd_disconnect and class_disconnect_exports +Details : if obd_disconnect will be called to already disconnected export he + forget release one reference and osc module can't unloaded. + Severity : enhancement Bugzilla : 19293 Description: move AT tunable parameters for more consistent usage diff --git a/lustre/ldlm/ldlm_lib.c b/lustre/ldlm/ldlm_lib.c index 57f3f2e..4988a20 100644 --- a/lustre/ldlm/ldlm_lib.c +++ b/lustre/ldlm/ldlm_lib.c @@ -464,12 +464,12 @@ int client_disconnect_export(struct obd_export *exp) if (!cli->cl_conn_count) { CERROR("disconnecting disconnected device (%s)\n", obd->obd_name); - GOTO(out_sem, rc = -EINVAL); + GOTO(out_disconnect, rc = -EINVAL); } cli->cl_conn_count--; if (cli->cl_conn_count) - GOTO(out_no_disconnect, rc = 0); + GOTO(out_disconnect, rc = 0); /* Mark import deactivated now, so we don't try to reconnect if any * of the cleanup RPCs fails (e.g. ldlm cancel, etc). We don't @@ -509,11 +509,14 @@ int client_disconnect_export(struct obd_export *exp) cli->cl_import = NULL; EXIT; - out_no_disconnect: + + out_disconnect: + /* use server style - class_disconnect should be always called for + * o_disconnect */ err = class_disconnect(exp); if (!rc && err) rc = err; - out_sem: + up_write(&cli->cl_sem); if (to_be_freed) ldlm_namespace_free_post(to_be_freed); diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index fe78085..d291f40 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -940,10 +940,12 @@ int class_connect(struct lustre_handle *conn, struct obd_device *obd, } EXPORT_SYMBOL(class_connect); -/* This function removes two references from the export: one for the - * hash entry and one for the export pointer passed in. The export - * pointer passed to this function is destroyed should not be used - * again. */ +/* This function removes 1-3 references from the export: + * 1 - for export pointer passed + * and if disconnect really need + * 2 - removing from hash + * 3 - in client_unlink_export + * The export pointer passed to this function can destroyed */ int class_disconnect(struct obd_export *export) { int already_disconnected; @@ -958,24 +960,27 @@ int class_disconnect(struct obd_export *export) spin_lock(&export->exp_lock); already_disconnected = export->exp_disconnected; export->exp_disconnected = 1; - - if (!hlist_unhashed(&export->exp_nid_hash)) - lustre_hash_del(export->exp_obd->obd_nid_hash, - &export->exp_connection->c_peer.nid, - &export->exp_nid_hash); - spin_unlock(&export->exp_lock); + /* class_cleanup(), abort_recovery(), and class_fail_export() * all end up in here, and if any of them race we shouldn't * call extra class_export_puts(). */ if (already_disconnected) - RETURN(0); + GOTO(no_disconn, already_disconnected); CDEBUG(D_IOCTL, "disconnect: cookie "LPX64"\n", export->exp_handle.h_cookie); + + if (!hlist_unhashed(&export->exp_nid_hash)) + lustre_hash_del(export->exp_obd->obd_nid_hash, + &export->exp_connection->c_peer.nid, + &export->exp_nid_hash); + class_unlink_export(export); + +no_disconn: class_export_put(export); RETURN(0); } @@ -984,14 +989,14 @@ static void class_disconnect_export_list(struct list_head *list, enum obd_option flags) { int rc; - struct lustre_handle fake_conn; - struct obd_export *fake_exp, *exp; + struct obd_export *exp; ENTRY; /* It's possible that an export may disconnect itself, but * nothing else will be added to this list. */ while (!list_empty(list)) { exp = list_entry(list->next, struct obd_export, exp_obd_chain); + /* need for safe call CDEBUG after obd_disconnect */ class_export_get(exp); spin_lock(&exp->exp_lock); @@ -1010,22 +1015,14 @@ static void class_disconnect_export_list(struct list_head *list, continue; } - fake_conn.cookie = exp->exp_handle.h_cookie; - fake_exp = class_conn2export(&fake_conn); - if (!fake_exp) { - class_export_put(exp); - continue; - } - - spin_lock(&fake_exp->exp_lock); - fake_exp->exp_flags = flags; - spin_unlock(&fake_exp->exp_lock); - + class_export_get(exp); CDEBUG(D_HA, "%s: disconnecting export at %s (%p), " "last request at %ld\n", exp->exp_obd->obd_name, obd_export_nid2str(exp), exp, exp->exp_last_request_time); - rc = obd_disconnect(fake_exp); + + /* release one export reference anyway */ + rc = obd_disconnect(exp); CDEBUG(D_HA, "disconnected export at %s (%p): rc %d\n", obd_export_nid2str(exp), exp, rc); class_export_put(exp); -- 1.8.3.1