From a20476ca22862d4efa185d0563e552c271d2e82a Mon Sep 17 00:00:00 2001 From: James Simmons Date: Wed, 26 Feb 2025 11:47:19 -0700 Subject: [PATCH] LU-11850 obd: support the rest of "stats" with Netlink Migrate the remaining "stats" files to the debugfs Netlink API except for the exports stats. Its is possible that we lack a kobject and a debugfs dentry so we can end up in a case that we can't derive a name. So change the API to supply the stat source name instead. Update the stats packet size calculate based on the new debugging info in the function lnet_genl_parse_list(). Test-Parameters: trivial Change-Id: If52dfb2807cbdcd9a24e9334edfa2101a8483fdd Signed-off-by: James Simmons Reviewed-on: https://review.whamcloud.com/c/fs/lustre-release/+/57305 Tested-by: jenkins Tested-by: Maloo Reviewed-by: Arshad Hussain Reviewed-by: Shaun Tancheff Reviewed-by: Oleg Drokin --- lnet/lnet/api-ni.c | 4 ++- lustre/include/lprocfs_status.h | 3 +-- lustre/ldlm/ldlm_pool.c | 9 ++++--- lustre/llite/lproc_llite.c | 5 ++-- lustre/obdclass/kernelcomm.c | 26 +++++++++++------- lustre/obdclass/lprocfs_status.c | 48 ++++++++++++++++----------------- lustre/obdclass/lprocfs_status_server.c | 9 ++++--- lustre/ofd/lproc_ofd.c | 2 +- lustre/osd-ldiskfs/osd_lproc.c | 6 +++-- lustre/osd-zfs/osd_lproc.c | 6 +++-- lustre/osp/lproc_osp.c | 4 +-- lustre/ptlrpc/lproc_ptlrpc.c | 19 +++++++------ lustre/ptlrpc/ptlrpc_internal.h | 7 +---- lustre/ptlrpc/service.c | 18 ++++++++++--- 14 files changed, 94 insertions(+), 72 deletions(-) diff --git a/lnet/lnet/api-ni.c b/lnet/lnet/api-ni.c index 8985fa3..951aa64 100644 --- a/lnet/lnet/api-ni.c +++ b/lnet/lnet/api-ni.c @@ -2843,6 +2843,7 @@ static int lnet_genl_parse_list(struct sk_buff *msg, for (count = 1; count <= list->lkl_maxattr; count++) { struct nlattr *key = nla_nest_start(msg, count); + int end, start = msg->len; if (!key) return -EMSGSIZE; @@ -2869,7 +2870,8 @@ static int lnet_genl_parse_list(struct sk_buff *msg, idx = rc; } - nla_nest_end(msg, key); + end = nla_nest_end(msg, key); + CDEBUG(D_INFO, "nest attr[%d] length = %d\n", count, end - start); } nla_nest_end(msg, node); diff --git a/lustre/include/lprocfs_status.h b/lustre/include/lprocfs_status.h index 7303242..cb7c602 100644 --- a/lustre/include/lprocfs_status.h +++ b/lustre/include/lprocfs_status.h @@ -180,7 +180,7 @@ enum lprocfs_fields_flags { struct lprocfs_stats { /* source for the stats */ - char *ls_source; + char ls_source[MAX_OBD_NAME * 4]; /* index in Xarray */ unsigned int ls_index; /* # of counters */ @@ -515,7 +515,6 @@ extern void lprocfs_stats_free(struct lprocfs_stats **stats); extern void lprocfs_init_ldlm_stats(struct lprocfs_stats *ldlm_stats); struct lprocfs_stats *ldebugfs_stats_alloc(int num, char *name, struct dentry *entry, - struct kobject *kobj, enum lprocfs_stats_flags flags); extern int ldebugfs_alloc_obd_stats(struct obd_device *obd, unsigned int num_stats); diff --git a/lustre/ldlm/ldlm_pool.c b/lustre/ldlm/ldlm_pool.c index e1bac43..ae6b50a 100644 --- a/lustre/ldlm/ldlm_pool.c +++ b/lustre/ldlm/ldlm_pool.c @@ -803,10 +803,10 @@ static int ldlm_pool_debugfs_init(struct ldlm_pool *pl) struct ldlm_namespace *ns = ldlm_pl2ns(pl); struct dentry *debugfs_ns_parent; struct ldebugfs_vars pool_vars[2]; + char param[MAX_OBD_NAME * 4]; int rc = 0; ENTRY; - debugfs_ns_parent = ns->ns_debugfs_entry; if (IS_ERR_OR_NULL(debugfs_ns_parent)) { CERROR("%s: debugfs entry is not initialized\n", @@ -820,10 +820,11 @@ static int ldlm_pool_debugfs_init(struct ldlm_pool *pl) ldlm_add_var(&pool_vars[0], pl->pl_debugfs_entry, "state", pl, &lprocfs_pool_state_fops); + scnprintf(param, sizeof(param), "ldlm.namespaces.%s.pool.stats", + ldlm_ns_name(ns)); pl->pl_stats = ldebugfs_stats_alloc(LDLM_POOL_LAST_STAT - - LDLM_POOL_FIRST_STAT, "stats", - pl->pl_debugfs_entry, - &pl->pl_kobj, 0); + LDLM_POOL_FIRST_STAT, param, + pl->pl_debugfs_entry, 0); if (!pl->pl_stats) GOTO(out, rc = -ENOMEM); diff --git a/lustre/llite/lproc_llite.c b/lustre/llite/lproc_llite.c index b55fc5c..4979091 100644 --- a/lustre/llite/lproc_llite.c +++ b/lustre/llite/lproc_llite.c @@ -2627,6 +2627,7 @@ static const char *const ra_stat_string[] = { int ll_debugfs_register_super(struct super_block *sb, const char *name) { struct ll_sb_info *sbi = ll_s2sbi(sb); + char param[MAX_OBD_NAME * 4]; int err, id; ENTRY; @@ -2664,9 +2665,9 @@ int ll_debugfs_register_super(struct super_block *sb, const char *name) &ll_rw_offset_stats_fops); /* File operations stats */ - sbi->ll_stats = ldebugfs_stats_alloc(LPROC_LL_FILE_OPCODES, "stats", + scnprintf(param, sizeof(param), "llite.%s.stats", name); + sbi->ll_stats = ldebugfs_stats_alloc(LPROC_LL_FILE_OPCODES, param, sbi->ll_debugfs_entry, - &sbi->ll_kset.kobj, LPROCFS_STATS_FLAG_NONE); if (!sbi->ll_stats) GOTO(out_debugfs, err = -ENOMEM); diff --git a/lustre/obdclass/kernelcomm.c b/lustre/obdclass/kernelcomm.c index 03b7527..40c0a2da 100644 --- a/lustre/obdclass/kernelcomm.c +++ b/lustre/obdclass/kernelcomm.c @@ -374,16 +374,20 @@ int lustre_stats_done(struct netlink_callback *cb) } EXPORT_SYMBOL(lustre_stats_done); -/* Min size for key table and its matching values: +/* Min size for key table and its matching values. Key value + * measurements are collected from lnet_genl_parse_list: + * * header strlen("stats") - * source strlen("source") + MAX_OBD_NAME * 2 + * source strlen("source") + MAX_OBD_NAME * 4 * timestamp strlen("snapshot_time") + s64 * start time strlen("start time") + s64 * elapsed_time strlen("elapse time") + s64 */ -#define STATS_MSG_MIN_SIZE (267 + 58) +#define STATS_MSG_MIN_SIZE (44 + 28 + 36 + 32 + 36 + 536) -/* key table + values for each dataset entry: +/* key table + values for each dataset entry. Key value + * measurements are collected from lnet_genl_parse_list: + * * dataset name 25 * dataset count strlen("samples") + u64 * dataset units strlen("units") + 5 @@ -392,7 +396,7 @@ EXPORT_SYMBOL(lustre_stats_done); * dataset sum strlen("sum") + u64 * dataset stdev strlen("stddev") + u64 */ -#define STATS_MSG_DATASET_SIZE (97) +#define STATS_MSG_DATASET_SIZE (236 + 25 + 5 + 8 * 5) static int lustre_stats_start(struct netlink_callback *cb) { @@ -436,14 +440,15 @@ static int lustre_stats_start(struct netlink_callback *cb) int rem2; nla_for_each_nested(item, dev, rem2) { - char filter[MAX_OBD_NAME * 2]; + char filter[MAX_OBD_NAME * 4]; if (nla_type(item) != LN_SCALAR_ATTR_VALUE || nla_strcmp(item, "source") != 0) continue; item = nla_next(item, &rem2); - if (nla_type(item) != LN_SCALAR_ATTR_VALUE) { + if (!nla_ok(item, rem2) || + nla_type(item) != LN_SCALAR_ATTR_VALUE) { NL_SET_ERR_MSG(extack, "source has invalid value"); GOTO(report_err, rc = -EINVAL); @@ -453,7 +458,7 @@ static int lustre_stats_start(struct netlink_callback *cb) rc = nla_strscpy(filter, item, sizeof(filter)); if (rc < 0) { NL_SET_ERR_MSG(extack, - "source key string is invalud"); + "source key string is invalid"); GOTO(report_err, rc); } @@ -755,14 +760,15 @@ static int lustre_stats_cmd(struct sk_buff *skb, struct genl_info *info) continue; nla_for_each_nested(prop, attr, rem2) { - char source[MAX_OBD_NAME * 2]; + char source[MAX_OBD_NAME * 4]; if (nla_type(prop) != LN_SCALAR_ATTR_VALUE || nla_strcmp(prop, "source") != 0) continue; prop = nla_next(prop, &rem2); - if (nla_type(prop) != LN_SCALAR_ATTR_VALUE) + if (!nla_ok(prop, rem2) || + nla_type(prop) != LN_SCALAR_ATTR_VALUE) GOTO(report_err, rc = -EINVAL); memset(source, 0, sizeof(source)); diff --git a/lustre/obdclass/lprocfs_status.c b/lustre/obdclass/lprocfs_status.c index b44d6c6..b27008d 100644 --- a/lustre/obdclass/lprocfs_status.c +++ b/lustre/obdclass/lprocfs_status.c @@ -1234,7 +1234,6 @@ struct lprocfs_stats *lprocfs_stats_alloc(unsigned int num, stats->ls_init = ktime_get_real(); spin_lock_init(&stats->ls_lock); kref_init(&stats->ls_refcount); - stats->ls_source = NULL; stats->ls_index = -1; /* alloc num of counter headers */ @@ -1268,10 +1267,10 @@ static DEFINE_XARRAY_ALLOC(lstats_list); struct lprocfs_stats *ldebugfs_stats_alloc(int num, char *name, struct dentry *debugfs_entry, - struct kobject *kobj, enum lprocfs_stats_flags flags) { struct lprocfs_stats *stats = lprocfs_stats_alloc(num, flags); + size_t len = strlen(name); char *param; int rc; @@ -1290,17 +1289,16 @@ struct lprocfs_stats *ldebugfs_stats_alloc(int num, char *name, atomic_inc(&lstats_count); xa_unlock(&lstats_list); - stats->ls_source = kobject_get_path(kobj, GFP_KERNEL); - if (!stats->ls_source) { - lprocfs_stats_free(&stats); - return NULL; + param = strrchr(name, '.'); + if (param) { + len -= strlen(param); + param++; + } else { + param = name; } - param = stats->ls_source; - while ((param = strchr(param, '/')) != NULL) - *param = '.'; - - debugfs_create_file(name, 0644, debugfs_entry, stats, + strscpy(stats->ls_source, name, len + 1); + debugfs_create_file(param, 0644, debugfs_entry, stats, &ldebugfs_stats_seq_fops); return stats; } @@ -1341,8 +1339,6 @@ static void stats_free(struct kref *kref) xa_unlock(&lstats_list); } - kfree(stats->ls_source); /* allocated by kobject_get_path */ - LIBCFS_FREE(stats, offsetof(typeof(*stats), ls_percpu[num_entry])); } @@ -1361,12 +1357,15 @@ EXPORT_SYMBOL(lprocfs_stats_free); unsigned int lustre_stats_scan(struct lustre_stats_list *slist, const char *source) { struct lprocfs_stats *item, **stats; - unsigned int cnt = 0, snum; - const char *tmp = source; + unsigned int cnt = 0, snum = 0, i; unsigned long idx = 0; - if (source) - for (snum = 0; tmp[snum]; tmp[snum] == '.' ? snum++ : *tmp++); + if (source) { + for (i = 0; source[i]; i++) { + if (source[i] == '.') + snum++; + } + } xa_for_each(&lstats_list, idx, item) { if (!kref_get_unless_zero(&item->ls_refcount)) @@ -1378,19 +1377,19 @@ unsigned int lustre_stats_scan(struct lustre_stats_list *slist, const char *sour } if (source) { - char filter[PATH_MAX / 8], *src = item->ls_source; - unsigned int num; - - if (strstarts(src, ".fs.lustre.")) - src += strlen(".fs.lustre."); + char filter[MAX_OBD_NAME * 4], *src = item->ls_source; + unsigned int num = 0; /* glob_match() has a hard time telling *.* from *.*.* * from *.*.* so we need to compare the number of '.' * and filter on that as well. This actually avoids * the overhead of calling glob_match() every time. */ - tmp = src; - for (num = 0; tmp[num]; tmp[num] == '.' ? num++ : *tmp++); + for (i = 0; src[i]; i++) { + if (src[i] == '.') + num++; + } + if (snum != num) { lprocfs_stats_free(&item); continue; @@ -1421,6 +1420,7 @@ unsigned int lustre_stats_scan(struct lustre_stats_list *slist, const char *sour } else { strscpy(filter, source, strlen(source) + 1); } + if (!glob_match(filter, src)) { lprocfs_stats_free(&item); continue; diff --git a/lustre/obdclass/lprocfs_status_server.c b/lustre/obdclass/lprocfs_status_server.c index 1957013..a75b101 100644 --- a/lustre/obdclass/lprocfs_status_server.c +++ b/lustre/obdclass/lprocfs_status_server.c @@ -687,10 +687,13 @@ int lprocfs_exp_cleanup(struct obd_export *exp) int ldebugfs_alloc_obd_stats(struct obd_device *obd, unsigned int num_stats) { + char param[MAX_OBD_NAME * 4]; + LASSERT(!obd->obd_stats); - obd->obd_stats = ldebugfs_stats_alloc(num_stats, "stats", - obd->obd_debugfs_entry, - &obd->obd_type->typ_kobj, 0); + scnprintf(param, sizeof(param), "%s.%s.stats", obd->obd_type->typ_name, + obd->obd_name); + obd->obd_stats = ldebugfs_stats_alloc(num_stats, param, + obd->obd_debugfs_entry, 0); return obd->obd_stats ? 0 : -ENOMEM; } EXPORT_SYMBOL(ldebugfs_alloc_obd_stats); diff --git a/lustre/ofd/lproc_ofd.c b/lustre/ofd/lproc_ofd.c index e59785d..bd1fea4 100644 --- a/lustre/ofd/lproc_ofd.c +++ b/lustre/ofd/lproc_ofd.c @@ -1095,6 +1095,7 @@ int ofd_tunables_init(struct ofd_device *ofd) * to /proc incrementally as the ofd is setup */ obd->obd_ktype.default_groups = KOBJ_ATTR_GROUPS(ofd); + obd->obd_debugfs_vars = ldebugfs_ofd_obd_vars; obd->obd_vars = lprocfs_ofd_obd_vars; rc = lprocfs_obd_setup(obd, false); if (rc) { @@ -1102,7 +1103,6 @@ int ofd_tunables_init(struct ofd_device *ofd) obd->obd_name, rc); RETURN(rc); } - ldebugfs_add_vars(obd->obd_debugfs_entry, ldebugfs_ofd_obd_vars, obd); rc = tgt_tunables_init(&ofd->ofd_lut); if (rc) { diff --git a/lustre/osd-ldiskfs/osd_lproc.c b/lustre/osd-ldiskfs/osd_lproc.c index b789e87..c78bf27 100644 --- a/lustre/osd-ldiskfs/osd_lproc.c +++ b/lustre/osd-ldiskfs/osd_lproc.c @@ -85,12 +85,14 @@ out: static int osd_stats_init(struct osd_device *osd) { + char param[MAX_OBD_NAME * 4]; int result = -ENOMEM; ENTRY; - osd->od_stats = ldebugfs_stats_alloc(LPROC_OSD_LAST, "stats", + scnprintf(param, sizeof(param), "osd-zfs.%s.stats", osd_name(osd)); + osd->od_stats = ldebugfs_stats_alloc(LPROC_OSD_LAST, param, osd->od_dt_dev.dd_debugfs_entry, - &osd->od_dt_dev.dd_kobj, 0); + 0); if (osd->od_stats) { lprocfs_counter_init(osd->od_stats, LPROC_OSD_GET_PAGE, LPROCFS_TYPE_LATENCY, "get_page"); diff --git a/lustre/osd-zfs/osd_lproc.c b/lustre/osd-zfs/osd_lproc.c index d6b6945..8c6ef03 100644 --- a/lustre/osd-zfs/osd_lproc.c +++ b/lustre/osd-zfs/osd_lproc.c @@ -73,12 +73,14 @@ out: static int osd_stats_init(struct osd_device *osd) { + char param[MAX_OBD_NAME * 4]; int result = -ENOMEM; ENTRY; - osd->od_stats = ldebugfs_stats_alloc(LPROC_OSD_LAST, "stats", + scnprintf(param, sizeof(param), "osd-zfs.%s.stats", osd_name(osd)); + osd->od_stats = ldebugfs_stats_alloc(LPROC_OSD_LAST, param, osd->od_dt_dev.dd_debugfs_entry, - &osd->od_dt_dev.dd_kobj, 0); + 0); if (osd->od_stats) { lprocfs_counter_init(osd->od_stats, LPROC_OSD_GET_PAGE, LPROCFS_CNTR_AVGMINMAX | LPROCFS_CNTR_STDDEV | diff --git a/lustre/osp/lproc_osp.c b/lustre/osp/lproc_osp.c index 74e092a..6b03245 100644 --- a/lustre/osp/lproc_osp.c +++ b/lustre/osp/lproc_osp.c @@ -1288,8 +1288,8 @@ void osp_tunables_init(struct osp_device *osp) /* Since we register the obd device with ptlrpc / sptlrpc we * have to register debugfs with obd_device */ - obd->obd_debugfs_entry = debugfs_create_dir( - obd->obd_name, obd->obd_type->typ_debugfs_entry); + obd->obd_debugfs_entry = debugfs_create_dir(obd->obd_name, + obd->obd_type->typ_debugfs_entry); ldebugfs_add_vars(obd->obd_debugfs_entry, obd->obd_debugfs_vars, obd); sptlrpc_lprocfs_cliobd_attach(obd); diff --git a/lustre/ptlrpc/lproc_ptlrpc.c b/lustre/ptlrpc/lproc_ptlrpc.c index 37ed8bb..d603e5e 100644 --- a/lustre/ptlrpc/lproc_ptlrpc.c +++ b/lustre/ptlrpc/lproc_ptlrpc.c @@ -188,7 +188,6 @@ static const char *ll_eopcode2str(__u32 opcode) static void ptlrpc_ldebugfs_register(struct dentry *root, char *dir, char *name, struct dentry **debugfs_root_ret, - struct kobject *kobj, struct lprocfs_stats **stats_ret) { struct dentry *svc_debugfs_entry; @@ -197,16 +196,14 @@ ptlrpc_ldebugfs_register(struct dentry *root, char *dir, char *name, LPROCFS_CNTR_STDDEV; int i; - LASSERT(!*debugfs_root_ret); LASSERT(!*stats_ret); - if (dir) svc_debugfs_entry = debugfs_create_dir(dir, root); else svc_debugfs_entry = root; svc_stats = ldebugfs_stats_alloc(EXTRA_MAX_OPCODES + LUSTRE_MAX_OPCODES, - name, svc_debugfs_entry, kobj, 0); + name, svc_debugfs_entry, 0); if (!svc_stats) return; @@ -1194,7 +1191,7 @@ int ptlrpc_sysfs_register_service(struct kset *parent, &parent->kobj, "%s", svc->srv_name); } -void ptlrpc_ldebugfs_register_service(struct dentry *entry, +void ptlrpc_ldebugfs_register_service(struct dentry *entry, char *param, struct ptlrpc_service *svc) { struct ldebugfs_vars ldebugfs_vars[] = { @@ -1223,9 +1220,8 @@ void ptlrpc_ldebugfs_register_service(struct dentry *entry, .release = lprocfs_seq_release, }; - ptlrpc_ldebugfs_register(entry, svc->srv_name, "stats", - &svc->srv_debugfs_entry, - &svc->srv_kobj, &svc->srv_stats); + ptlrpc_ldebugfs_register(entry, svc->srv_name, param, + &svc->srv_debugfs_entry, &svc->srv_stats); if (!svc->srv_debugfs_entry) return; @@ -1237,9 +1233,12 @@ void ptlrpc_ldebugfs_register_service(struct dentry *entry, void ptlrpc_lprocfs_register_obd(struct obd_device *obd) { - ptlrpc_ldebugfs_register(obd->obd_debugfs_entry, NULL, "stats", + char param[MAX_OBD_NAME * 4]; + + scnprintf(param, sizeof(param), "%s.%s.stats", + kobject_name(&obd->obd_type->typ_kobj), obd->obd_name); + ptlrpc_ldebugfs_register(obd->obd_debugfs_entry, NULL, param, &obd->obd_svc_debugfs_entry, - &obd->obd_kset.kobj, &obd->obd_svc_stats); } EXPORT_SYMBOL(ptlrpc_lprocfs_register_obd); diff --git a/lustre/ptlrpc/ptlrpc_internal.h b/lustre/ptlrpc/ptlrpc_internal.h index 779b422..bac3ff2 100644 --- a/lustre/ptlrpc/ptlrpc_internal.h +++ b/lustre/ptlrpc/ptlrpc_internal.h @@ -96,17 +96,12 @@ int ptlrpc_sysfs_register_service(struct kset *parent, void ptlrpc_sysfs_unregister_service(struct ptlrpc_service *svc); void ptlrpc_ldebugfs_register_service(struct dentry *debugfs_entry, + char *param, struct ptlrpc_service *svc); -#ifdef CONFIG_PROC_FS void ptlrpc_lprocfs_unregister_service(struct ptlrpc_service *svc); void ptlrpc_lprocfs_rpc_sent(struct ptlrpc_request *req, long amount); void ptlrpc_lprocfs_do_request_stat (struct ptlrpc_request *req, long q_usec, long work_usec); -#else -#define ptlrpc_lprocfs_unregister_service(params...) do{}while(0) -#define ptlrpc_lprocfs_rpc_sent(params...) do{}while(0) -#define ptlrpc_lprocfs_do_request_stat(params...) do{}while(0) -#endif /* CONFIG_PROC_FS */ /* NRS */ diff --git a/lustre/ptlrpc/service.c b/lustre/ptlrpc/service.c index cf89bfe..db55471 100644 --- a/lustre/ptlrpc/service.c +++ b/lustre/ptlrpc/service.c @@ -690,7 +690,8 @@ struct ptlrpc_service *ptlrpc_register_service(struct ptlrpc_service_conf *conf, struct ptlrpc_service *service; struct ptlrpc_service_part *svcpt; struct cfs_cpt_table *cptable; - __u32 *cpts = NULL; + char param[MAX_OBD_NAME * 4]; + u32 *cpts = NULL; int ncpts; int cpt; int rc; @@ -818,13 +819,24 @@ struct ptlrpc_service *ptlrpc_register_service(struct ptlrpc_service_conf *conf, mutex_unlock(&ptlrpc_all_services_mutex); if (parent) { + char *path, *tmp; + rc = ptlrpc_sysfs_register_service(parent, service); if (rc) GOTO(failed, rc); + + path = kobject_get_path(&parent->kobj, GFP_KERNEL); + if (path) { + tmp = path + strlen("/fs/lustre/"); + scnprintf(param, sizeof(param), "%s.%s.stats", + tmp, service->srv_name); + tmp = param; + while ((tmp = strchr(tmp, '/')) != NULL) + *tmp = '.'; + } } - if (debugfs_entry != NULL) - ptlrpc_ldebugfs_register_service(debugfs_entry, service); + ptlrpc_ldebugfs_register_service(debugfs_entry, param, service); rc = ptlrpc_service_nrs_setup(service); if (rc != 0) -- 1.8.3.1