From 8f24a0e29b5f4b18193742c9aecd960c6b6102ff Mon Sep 17 00:00:00 2001 From: James Simmons Date: Sun, 9 Feb 2014 09:37:44 -0500 Subject: [PATCH 05/13] LU-3319 procfs: fix symlink handling While working on symlink handling for seq files I noticed a long outstanding bug. Code was developed to link osc obds to target_obds of the lov layer. The target_obds directory was never created for the symlinks. This patches enables this long forgotten feature. Also addressed is the race condition experinced with server side code ported to seq_files that used symlinks. To avoid the race the handle obd_proc_private was moved from struct obd_device to struct obd_type which now allows earlier registeration that only happens once. Change-Id: Ib158ec4444ed7abc0f3c3e820ee4a333631a58d1 Signed-off-by: James Simmons --- lustre/include/obd.h | 17 +++++---- lustre/lmv/lmv_obd.c | 48 ++++++++--------------- lustre/lov/lov_obd.c | 99 +++++++++++++++++++++++++++--------------------- lustre/obdclass/genops.c | 17 +++++---- 4 files changed, 91 insertions(+), 90 deletions(-) diff --git a/lustre/include/obd.h b/lustre/include/obd.h index c18052b..8fd2ce7 100644 --- a/lustre/include/obd.h +++ b/lustre/include/obd.h @@ -183,13 +183,15 @@ struct obd_info { }; struct obd_type { - cfs_list_t typ_chain; - struct obd_ops *typ_dt_ops; - struct md_ops *typ_md_ops; - cfs_proc_dir_entry_t *typ_procroot; - char *typ_name; - int typ_refcnt; - struct lu_device_type *typ_lu; + struct list_head typ_chain; + struct obd_ops *typ_dt_ops; + struct md_ops *typ_md_ops; + cfs_proc_dir_entry_t *typ_procroot; + cfs_proc_dir_entry_t *typ_procsym; + __u32 typ_sym_filter; + char *typ_name; + int typ_refcnt; + struct lu_device_type *typ_lu; spinlock_t obd_type_lock; }; @@ -825,7 +827,6 @@ struct obd_device { struct proc_dir_entry *obd_proc_entry; struct proc_dir_entry *obd_proc_exports_entry; - void *obd_proc_private; /* type private PDEs */ struct proc_dir_entry *obd_svc_procroot; struct lprocfs_stats *obd_svc_stats; struct lprocfs_seq_vars *obd_vars; diff --git a/lustre/lmv/lmv_obd.c b/lustre/lmv/lmv_obd.c index e0941c9..93db311 100644 --- a/lustre/lmv/lmv_obd.c +++ b/lustre/lmv/lmv_obd.c @@ -242,9 +242,6 @@ static int lmv_connect(const struct lu_env *env, struct obd_uuid *cluuid, struct obd_connect_data *data, void *localdata) { -#ifdef __KERNEL__ - struct proc_dir_entry *lmv_proc_dir; -#endif struct lmv_obd *lmv = &obd->u.lmv; struct lustre_handle conn = { 0 }; int rc = 0; @@ -277,18 +274,15 @@ static int lmv_connect(const struct lu_env *env, lmv->conn_data = *data; #ifdef __KERNEL__ - if (obd->obd_proc_private != NULL) { - lmv_proc_dir = obd->obd_proc_private; - } else { - lmv_proc_dir = lprocfs_seq_register("target_obds", - obd->obd_proc_entry, - NULL, NULL); - if (IS_ERR(lmv_proc_dir)) { + if (obd->obd_type->typ_procsym == NULL) { + obd->obd_type->typ_procsym = lprocfs_seq_register("target_obds", + obd->obd_proc_entry, + NULL, NULL); + if (IS_ERR(obd->obd_type->typ_procsym)) { CERROR("could not register /proc/fs/lustre/%s/%s/target_obds.", obd->obd_type->typ_name, obd->obd_name); - lmv_proc_dir = NULL; + obd->obd_type->typ_procsym = NULL; } - obd->obd_proc_private = lmv_proc_dir; } #endif @@ -302,10 +296,8 @@ static int lmv_connect(const struct lu_env *env, rc = lmv_check_connect(obd); #ifdef __KERNEL__ - if (rc && lmv_proc_dir) { - lprocfs_remove(&lmv_proc_dir); - obd->obd_proc_private = NULL; - } + if (rc && obd->obd_type->typ_procsym != NULL) + lprocfs_remove(&obd->obd_type->typ_procsym); #endif RETURN(rc); } @@ -384,9 +376,6 @@ static int lmv_init_ea_size(struct obd_export *exp, int easize, int lmv_connect_mdc(struct obd_device *obd, struct lmv_tgt_desc *tgt) { -#ifdef __KERNEL__ - struct proc_dir_entry *lmv_proc_dir; -#endif struct lmv_obd *lmv = &obd->u.lmv; struct obd_uuid *cluuid = &lmv->cluuid; struct obd_uuid lmv_mdc_uuid = { "LMV_MDC_UUID" }; @@ -466,14 +455,13 @@ int lmv_connect_mdc(struct obd_device *obd, struct lmv_tgt_desc *tgt) cfs_atomic_read(&obd->obd_refcount)); #ifdef __KERNEL__ - lmv_proc_dir = obd->obd_proc_private; - if (lmv_proc_dir) { + if (obd->obd_type->typ_procsym != NULL) { struct proc_dir_entry *mdc_symlink; LASSERT(mdc_obd->obd_type != NULL); LASSERT(mdc_obd->obd_type->typ_name != NULL); mdc_symlink = lprocfs_add_symlink(mdc_obd->obd_name, - lmv_proc_dir, + obd->obd_type->typ_procsym, "../../../%s/%s", mdc_obd->obd_type->typ_name, mdc_obd->obd_name); @@ -482,8 +470,7 @@ int lmv_connect_mdc(struct obd_device *obd, struct lmv_tgt_desc *tgt) "/proc/fs/lustre/%s/%s/target_obds/%s.", obd->obd_type->typ_name, obd->obd_name, mdc_obd->obd_name); - lprocfs_remove(&lmv_proc_dir); - obd->obd_proc_private = NULL; + lprocfs_remove(&obd->obd_type->typ_procsym); } } #endif @@ -675,9 +662,6 @@ int lmv_check_connect(struct obd_device *obd) static int lmv_disconnect_mdc(struct obd_device *obd, struct lmv_tgt_desc *tgt) { -#ifdef __KERNEL__ - struct proc_dir_entry *lmv_proc_dir; -#endif struct lmv_obd *lmv = &obd->u.lmv; struct obd_device *mdc_obd; int rc; @@ -695,9 +679,9 @@ static int lmv_disconnect_mdc(struct obd_device *obd, struct lmv_tgt_desc *tgt) } #ifdef __KERNEL__ - lmv_proc_dir = obd->obd_proc_private; - if (lmv_proc_dir) - lprocfs_remove_proc_entry(mdc_obd->obd_name, lmv_proc_dir); + if (obd->obd_type->typ_procsym != NULL) + lprocfs_remove_proc_entry(mdc_obd->obd_name, + obd->obd_type->typ_procsym); #endif rc = obd_fid_fini(tgt->ltd_exp->exp_obd); if (rc) @@ -747,8 +731,8 @@ static int lmv_disconnect(struct obd_export *exp) } #ifdef __KERNEL__ - if (obd->obd_proc_private) - lprocfs_remove((struct proc_dir_entry **)&obd->obd_proc_private); + if (obd->obd_type->typ_procsym != NULL) + lprocfs_remove(&obd->obd_type->typ_procsym); else CERROR("/proc/fs/lustre/%s/%s/target_obds missing\n", obd->obd_type->typ_name, obd->obd_name); diff --git a/lustre/lov/lov_obd.c b/lustre/lov/lov_obd.c index 286cd15..a3310fd 100644 --- a/lustre/lov/lov_obd.c +++ b/lustre/lov/lov_obd.c @@ -127,19 +127,16 @@ static int lov_notify(struct obd_device *obd, struct obd_device *watched, int lov_connect_obd(struct obd_device *obd, __u32 index, int activate, struct obd_connect_data *data) { - struct lov_obd *lov = &obd->u.lov; - struct obd_uuid *tgt_uuid; - struct obd_device *tgt_obd; - static struct obd_uuid lov_osc_uuid = { "LOV_OSC_UUID" }; - struct obd_import *imp; -#ifdef __KERNEL__ - struct proc_dir_entry *lov_proc_dir; -#endif - int rc; - ENTRY; + struct lov_obd *lov = &obd->u.lov; + struct obd_uuid *tgt_uuid; + struct obd_device *tgt_obd; + static struct obd_uuid lov_osc_uuid = { "LOV_OSC_UUID" }; + struct obd_import *imp; + int rc; + ENTRY; - if (!lov->lov_tgts[index]) - RETURN(-EINVAL); + if (lov->lov_tgts[index] == NULL) + RETURN(-EINVAL); tgt_uuid = &lov->lov_tgts[index]->ltd_uuid; tgt_obd = lov->lov_tgts[index]->ltd_obd; @@ -195,27 +192,25 @@ int lov_connect_obd(struct obd_device *obd, __u32 index, int activate, obd_uuid2str(tgt_uuid), tgt_obd->obd_name, activate ? "":"in"); #ifdef __KERNEL__ - lov_proc_dir = obd->obd_proc_private; - if (lov_proc_dir) { - struct obd_device *osc_obd = lov->lov_tgts[index]->ltd_exp->exp_obd; + if (obd->obd_type->typ_procsym != NULL) { + struct obd_device *osc_obd = lov->lov_tgts[index]->ltd_exp->exp_obd; struct proc_dir_entry *osc_symlink; - LASSERT(osc_obd != NULL); - LASSERT(osc_obd->obd_magic == OBD_DEVICE_MAGIC); - LASSERT(osc_obd->obd_type->typ_name != NULL); - - osc_symlink = lprocfs_add_symlink(osc_obd->obd_name, - lov_proc_dir, - "../../../%s/%s", - osc_obd->obd_type->typ_name, - osc_obd->obd_name); - if (osc_symlink == NULL) { - CERROR("could not register LOV target " - "/proc/fs/lustre/%s/%s/target_obds/%s.", - obd->obd_type->typ_name, obd->obd_name, - osc_obd->obd_name); - lprocfs_remove(&lov_proc_dir); - obd->obd_proc_private = NULL; + LASSERT(osc_obd != NULL); + LASSERT(osc_obd->obd_magic == OBD_DEVICE_MAGIC); + LASSERT(osc_obd->obd_type->typ_name != NULL); + + osc_symlink = lprocfs_add_symlink(osc_obd->obd_name, + obd->obd_type->typ_procsym, + "../../../%s/%s", + osc_obd->obd_type->typ_name, + osc_obd->obd_name); + if (osc_symlink == NULL) { + CERROR("could not register LOV target " + "/proc/fs/lustre/%s/%s/target_obds/%s.", + obd->obd_type->typ_name, obd->obd_name, + osc_obd->obd_name); + lprocfs_remove(&obd->obd_type->typ_procsym); } } #endif @@ -250,6 +245,17 @@ static int lov_connect(const struct lu_env *env, if (data) lov->lov_ocd = *data; +#ifdef __KERNEL__ + obd->obd_type->typ_procsym = lprocfs_seq_register("target_obds", + obd->obd_proc_entry, + NULL, NULL); + if (IS_ERR(obd->obd_type->typ_procsym)) { + CERROR("could not register /proc/fs/lustre/%s/%s/target_obds.", + obd->obd_type->typ_name, obd->obd_name); + obd->obd_type->typ_procsym = NULL; + } +#endif + obd_getref(obd); for (i = 0; i < lov->desc.ld_tgt_count; i++) { tgt = lov->lov_tgts[i]; @@ -280,7 +286,6 @@ static int lov_connect(const struct lu_env *env, static int lov_disconnect_obd(struct obd_device *obd, struct lov_tgt_desc *tgt) { - struct proc_dir_entry *lov_proc_dir; struct lov_obd *lov = &obd->u.lov; struct obd_device *osc_obd; int rc; @@ -296,18 +301,18 @@ static int lov_disconnect_obd(struct obd_device *obd, struct lov_tgt_desc *tgt) tgt->ltd_exp->exp_obd->obd_inactive = 1; } - lov_proc_dir = obd->obd_proc_private; - if (lov_proc_dir) - lprocfs_remove_proc_entry(osc_obd->obd_name, lov_proc_dir); + if (obd->obd_type->typ_procsym) + lprocfs_remove_proc_entry(osc_obd->obd_name, + obd->obd_type->typ_procsym); - if (osc_obd) { - /* Pass it on to our clients. - * XXX This should be an argument to disconnect, - * XXX not a back-door flag on the OBD. Ah well. - */ - osc_obd->obd_force = obd->obd_force; - osc_obd->obd_fail = obd->obd_fail; - osc_obd->obd_no_recov = obd->obd_no_recov; + if (osc_obd) { + /* Pass it on to our clients. + * XXX This should be an argument to disconnect, + * XXX not a back-door flag on the OBD. Ah well. + */ + osc_obd->obd_force = obd->obd_force; + osc_obd->obd_fail = obd->obd_fail; + osc_obd->obd_no_recov = obd->obd_no_recov; } obd_register_observer(osc_obd, NULL); @@ -353,6 +358,14 @@ static int lov_disconnect(struct obd_export *exp) } obd_putref(obd); +#ifdef __KERNEL__ + if (obd->obd_type->typ_procsym) + lprocfs_remove(&obd->obd_type->typ_procsym); + else + CERROR("/proc/fs/lustre/%s/%s/target_obds missing\n", + obd->obd_type->typ_name, obd->obd_name); +#endif + out: rc = class_disconnect(exp); /* bz 9811 */ RETURN(rc); diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index c9d8a4e..a4a981c 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -181,14 +181,15 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops, RETURN(-EEXIST); } - rc = -ENOMEM; - OBD_ALLOC(type, sizeof(*type)); - if (type == NULL) - RETURN(rc); + rc = -ENOMEM; + OBD_ALLOC(type, sizeof(*type)); + if (type == NULL) + RETURN(rc); + memset(type, 0, sizeof(*type)); - OBD_ALLOC_PTR(type->typ_dt_ops); - OBD_ALLOC_PTR(type->typ_md_ops); - OBD_ALLOC(type->typ_name, strlen(name) + 1); + OBD_ALLOC_PTR(type->typ_dt_ops); + OBD_ALLOC_PTR(type->typ_md_ops); + OBD_ALLOC(type->typ_name, strlen(name) + 1); if (type->typ_dt_ops == NULL || type->typ_md_ops == NULL || @@ -242,6 +243,8 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops, if (type->typ_dt_ops != NULL) OBD_FREE_PTR(type->typ_dt_ops); #ifdef LPROCFS + if (type->typ_procsym != NULL) + lprocfs_remove(&type->typ_procsym); #ifndef HAVE_ONLY_PROCFS_SEQ lprocfs_try_remove_proc_entry(type->typ_name, proc_lustre_root); #else -- 1.8.5.3