diff --git a/arch/powerpc/perf/hv-24x7.c b/arch/powerpc/perf/hv-24x7.c index 36b29fd9295d..59012e750abe 100644 --- a/arch/powerpc/perf/hv-24x7.c +++ b/arch/powerpc/perf/hv-24x7.c @@ -27,20 +27,6 @@ #include "hv-24x7-catalog.h" #include "hv-common.h" -static const char *event_domain_suffix(unsigned domain) -{ - switch (domain) { -#define DOMAIN(n, v, x, c) \ - case HV_PERF_DOMAIN_##n: \ - return "__" #n; -#include "hv-24x7-domains.h" -#undef DOMAIN - default: - WARN(1, "unknown domain %d\n", domain); - return "__UNKNOWN_DOMAIN_SUFFIX"; - } -} - static bool domain_is_valid(unsigned domain) { switch (domain) { @@ -294,38 +280,70 @@ static unsigned long h_get_24x7_catalog_page(char page[], version, index); } -static unsigned core_domains[] = { - HV_PERF_DOMAIN_PHYS_CORE, - HV_PERF_DOMAIN_VCPU_HOME_CORE, - HV_PERF_DOMAIN_VCPU_HOME_CHIP, - HV_PERF_DOMAIN_VCPU_HOME_NODE, - HV_PERF_DOMAIN_VCPU_REMOTE_NODE, -}; -/* chip event data always yeilds a single event, core yeilds multiple */ -#define MAX_EVENTS_PER_EVENT_DATA ARRAY_SIZE(core_domains) - +/* + * Each event we find in the catalog, will have a sysfs entry. Format the + * data for this sysfs entry based on the event's domain. + * + * Events belonging to the Chip domain can only be monitored in that domain. + * i.e the domain for these events is a fixed/knwon value. + * + * Events belonging to the Core domain can be monitored either in the physical + * core or in one of the virtual CPU domains. So the domain value for these + * events must be specified by the user (i.e is a required parameter). Format + * the Core events with 'domain=?' so the perf-tool can error check required + * parameters. + * + * NOTE: For the Core domain events, rather than making domain a required + * parameter we could default it to PHYS_CORE and allowe users to + * override the domain to one of the VCPU domains. + * + * However, this can make the interface a little inconsistent. + * + * If we set domain=2 (PHYS_CHIP) and allow user to override this field + * the user may be tempted to also modify the "offset=x" field in which + * can lead to confusing usage. Consider the HPM_PCYC (offset=0x18) and + * HPM_INST (offset=0x20) events. With: + * + * perf stat -e hv_24x7/HPM_PCYC,offset=0x20/ + * + * we end up monitoring HPM_INST, while the command line has HPM_PCYC. + * + * By not assigning a default value to the domain for the Core events, + * we can have simple guidelines: + * + * - Specifying values for parameters with "=?" is required. + * + * - Specifying (i.e overriding) values for other parameters + * is undefined. + */ static char *event_fmt(struct hv_24x7_event_data *event, unsigned domain) { const char *sindex; const char *lpar; + const char *domain_str; + char buf[8]; switch (domain) { case HV_PERF_DOMAIN_PHYS_CHIP: + snprintf(buf, sizeof(buf), "%d", domain); + domain_str = buf; lpar = "0x0"; sindex = "chip"; break; case HV_PERF_DOMAIN_PHYS_CORE: + domain_str = "?"; lpar = "0x0"; sindex = "core"; break; default: + domain_str = "?"; lpar = "?"; sindex = "vcpu"; } return kasprintf(GFP_KERNEL, - "domain=0x%x,offset=0x%x,%s=?,lpar=%s", - domain, + "domain=%s,offset=0x%x,%s=?,lpar=%s", + domain_str, be16_to_cpu(event->event_counter_offs) + be16_to_cpu(event->event_group_record_offs), sindex, @@ -365,6 +383,15 @@ static struct attribute *device_str_attr_create_(char *name, char *str) return &attr->attr.attr; } +/* + * Allocate and initialize strings representing event attributes. + * + * NOTE: The strings allocated here are never destroyed and continue to + * exist till shutdown. This is to allow us to create as many events + * from the catalog as possible, even if we encounter errors with some. + * In case of changes to error paths in future, these may need to be + * freed by the caller. + */ static struct attribute *device_str_attr_create(char *name, int name_max, int name_nonce, char *str, size_t str_max) @@ -396,16 +423,6 @@ out_s: return NULL; } -static void device_str_attr_destroy(struct attribute *attr) -{ - struct dev_ext_attribute *d; - - d = container_of(attr, struct dev_ext_attribute, attr.attr); - kfree(d->var); - kfree(d->attr.attr.name); - kfree(d); -} - static struct attribute *event_to_attr(unsigned ix, struct hv_24x7_event_data *event, unsigned domain, @@ -413,7 +430,6 @@ static struct attribute *event_to_attr(unsigned ix, { int event_name_len; char *ev_name, *a_ev_name, *val; - const char *ev_suffix; struct attribute *attr; if (!domain_is_valid(domain)) { @@ -426,14 +442,13 @@ static struct attribute *event_to_attr(unsigned ix, if (!val) return NULL; - ev_suffix = event_domain_suffix(domain); ev_name = event_name(event, &event_name_len); if (!nonce) - a_ev_name = kasprintf(GFP_KERNEL, "%.*s%s", - (int)event_name_len, ev_name, ev_suffix); + a_ev_name = kasprintf(GFP_KERNEL, "%.*s", + (int)event_name_len, ev_name); else - a_ev_name = kasprintf(GFP_KERNEL, "%.*s%s__%d", - (int)event_name_len, ev_name, ev_suffix, nonce); + a_ev_name = kasprintf(GFP_KERNEL, "%.*s__%d", + (int)event_name_len, ev_name, nonce); if (!a_ev_name) goto out_val; @@ -478,45 +493,14 @@ event_to_long_desc_attr(struct hv_24x7_event_data *event, int nonce) return device_str_attr_create(name, nl, nonce, desc, dl); } -static ssize_t event_data_to_attrs(unsigned ix, struct attribute **attrs, +static int event_data_to_attrs(unsigned ix, struct attribute **attrs, struct hv_24x7_event_data *event, int nonce) { - unsigned i; - - switch (event->domain) { - case HV_PERF_DOMAIN_PHYS_CHIP: - *attrs = event_to_attr(ix, event, event->domain, nonce); - return 1; - case HV_PERF_DOMAIN_PHYS_CORE: - for (i = 0; i < ARRAY_SIZE(core_domains); i++) { - attrs[i] = event_to_attr(ix, event, core_domains[i], - nonce); - if (!attrs[i]) { - pr_warn("catalog event %u: individual attr %u " - "creation failure\n", ix, i); - for (; i; i--) - device_str_attr_destroy(attrs[i - 1]); - return -1; - } - } - return i; - default: - pr_warn("catalog event %u: domain %u is not allowed in the " - "catalog\n", ix, event->domain); + *attrs = event_to_attr(ix, event, event->domain, nonce); + if (!*attrs) return -1; - } -} -static size_t event_to_attr_ct(struct hv_24x7_event_data *event) -{ - switch (event->domain) { - case HV_PERF_DOMAIN_PHYS_CHIP: - return 1; - case HV_PERF_DOMAIN_PHYS_CORE: - return ARRAY_SIZE(core_domains); - default: - return 0; - } + return 0; } static unsigned long vmalloc_to_phys(void *v) @@ -752,9 +736,8 @@ static int create_events_from_catalog(struct attribute ***events_, goto e_free; } - if (SIZE_MAX / MAX_EVENTS_PER_EVENT_DATA - 1 < event_entry_count) { - pr_err("event_entry_count %zu is invalid\n", - event_entry_count); + if (SIZE_MAX - 1 < event_entry_count) { + pr_err("event_entry_count %zu is invalid\n", event_entry_count); ret = -EIO; goto e_free; } @@ -827,7 +810,7 @@ static int create_events_from_catalog(struct attribute ***events_, continue; } - attr_max += event_to_attr_ct(event); + attr_max++; } event_idx_last = event_idx; @@ -877,12 +860,12 @@ static int create_events_from_catalog(struct attribute ***events_, nonce = event_uniq_add(&ev_uniq, name, nl, event->domain); ct = event_data_to_attrs(event_idx, events + event_attr_ct, event, nonce); - if (ct <= 0) { + if (ct < 0) { pr_warn("event %zu (%.*s) creation failure, skipping\n", event_idx, nl, name); junk_events++; } else { - event_attr_ct += ct; + event_attr_ct++; event_descs[desc_ct] = event_to_desc_attr(event, nonce); if (event_descs[desc_ct]) desc_ct++;