linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch 12/21] perfmon2 minimal:  sysfs interface
@ 2008-06-09 22:34 eranian
  2008-06-11 14:49 ` Greg KH
  0 siblings, 1 reply; 8+ messages in thread
From: eranian @ 2008-06-09 22:34 UTC (permalink / raw)
  To: linux-kernel

This patch adds the sysfs interface to the perfmon2 subsystem. It is
used for configuration of the interface. It exposes the PMU register
mappings and various attributes of the subsystem.

Signed-off-by: Stephane Eranian <eranian@gmail.com>
--

Index: o/perfmon/perfmon_init.c
===================================================================
--- o.orig/perfmon/perfmon_init.c	2008-06-04 22:46:24.000000000 +0200
+++ o/perfmon/perfmon_init.c	2008-06-04 22:46:26.000000000 +0200
@@ -62,6 +62,9 @@
 	if (pfm_init_fs())
 		goto error_disable;
 
+	if (pfm_init_sysfs())
+		goto error_disable;
+
 	/* not critical, so no error checking */
 	pfm_init_debugfs();
 
Index: o/perfmon/perfmon_sysfs.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ o/perfmon/perfmon_sysfs.c	2008-06-04 22:46:26.000000000 +0200
@@ -0,0 +1,422 @@
+/*
+ * perfmon_sysfs.c: perfmon2 sysfs interface
+ *
+ * This file implements the perfmon2 interface which
+ * provides access to the hardware performance counters
+ * of the host processor.
+ *
+ * The initial version of perfmon.c was written by
+ * Ganesh Venkitachalam, IBM Corp.
+ *
+ * Then it was modified for perfmon-1.x by Stephane Eranian and
+ * David Mosberger, Hewlett Packard Co.
+ *
+ * Version Perfmon-2.x is a complete rewrite of perfmon-1.x
+ * by Stephane Eranian, Hewlett Packard Co.
+ *
+ * Copyright (c) 1999-2006 Hewlett-Packard Development Company, L.P.
+ * Contributed by Stephane Eranian <eranian@hpl.hp.com>
+ *                David Mosberger-Tang <davidm@hpl.hp.com>
+ *
+ * More information about perfmon available at:
+ * 	http://perfmon2.sf.net
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ * 02111-1307 USA
+ */
+#include <linux/kernel.h>
+#include <linux/module.h> /* for EXPORT_SYMBOL */
+#include <linux/perfmon_kern.h>
+#include "perfmon_priv.h"
+
+struct pfm_attribute {
+	struct attribute attr;
+	ssize_t (*show)(void *, char *);
+	ssize_t (*store)(void *, const char *, size_t);
+};
+#define to_attr(n) container_of(n, struct pfm_attribute, attr);
+
+#define PFM_RO_ATTR(_name) \
+struct pfm_attribute attr_##_name = __ATTR_RO(_name)
+
+#define PFM_RW_ATTR(_name, _mode, _show,_store) 			\
+struct pfm_attribute attr_##_name = __ATTR(_name, _mode, _show, _store);
+
+int pfm_sysfs_add_pmu(struct pfm_pmu_config *pmu);
+
+static struct kobject pfm_kernel_kobj;
+
+static ssize_t pfm_pmu_attr_show(struct kobject *kobj,
+		struct attribute *attr, char *buf)
+{
+	struct pfm_pmu_config *pmu = to_pmu(kobj);
+	struct pfm_attribute *attribute = to_attr(attr);
+	return attribute->show ? attribute->show(pmu, buf) : -EIO;
+}
+
+static ssize_t pfm_regs_attr_show(struct kobject *kobj,
+		struct attribute *attr, char *buf)
+{
+	struct pfm_regmap_desc *reg = to_reg(kobj);
+	struct pfm_attribute *attribute = to_attr(attr);
+	return attribute->show ? attribute->show(reg, buf) : -EIO;
+}
+
+static struct sysfs_ops pfm_pmu_sysfs_ops = {
+	.show = pfm_pmu_attr_show
+};
+
+static struct sysfs_ops pfm_regs_sysfs_ops = {
+	.show  = pfm_regs_attr_show
+};
+
+
+static struct kobj_type pfm_pmu_ktype = {
+	.sysfs_ops = &pfm_pmu_sysfs_ops,
+};
+
+static struct kobj_type pfm_regs_ktype = {
+	.sysfs_ops = &pfm_regs_sysfs_ops,
+};
+
+static ssize_t version_show(void *info, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u.%u\n",  PFM_VERSION_MAJ, PFM_VERSION_MIN);
+}
+
+static ssize_t task_sessions_count_show(void *info, char *buf)
+{
+	return pfm_sysfs_res_show(buf, PAGE_SIZE, 0);
+}
+
+
+static ssize_t debug_show(void *info, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", pfm_controls.debug);
+}
+
+static ssize_t debug_store(void *info, const char *buf, size_t sz)
+{
+	int d;
+
+	if (sscanf(buf, "%d", &d) != 1)
+		return -EINVAL;
+
+	pfm_controls.debug = d;
+	return sz;
+}
+
+static ssize_t task_group_show(void *info, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", pfm_controls.task_group);
+}
+
+static ssize_t task_group_store(void *info, const char *buf, size_t sz)
+{
+	int d;
+
+	if (sscanf(buf, "%d", &d) != 1)
+		return -EINVAL;
+
+	pfm_controls.task_group = d;
+
+	return strnlen(buf, PAGE_SIZE);
+}
+
+static ssize_t arg_size_show(void *info, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%zu\n", pfm_controls.arg_mem_max);
+}
+
+static ssize_t arg_size_store(void *info, const char *buf, size_t sz)
+{
+	size_t d;
+
+	if (sscanf(buf, "%zu", &d) != 1)
+		return -EINVAL;
+
+	/*
+	 * we impose a page as the minimum.
+	 *
+	 * This limit may be smaller than the stack buffer
+	 * available and that is fine.
+	 */
+	if (d < PAGE_SIZE)
+		return -EINVAL;
+
+	pfm_controls.arg_mem_max = d;
+
+	return strnlen(buf, PAGE_SIZE);
+}
+
+static int __init enable_debug(char *str)
+{
+	pfm_controls.debug = 1;
+	PFM_INFO("debug output enabled\n");
+	return 1;
+}
+__setup("perfmon_debug", enable_debug);
+
+/*
+ * /sys/kernel/perfmon attributes
+ */
+static PFM_RO_ATTR(version);
+static PFM_RO_ATTR(task_sessions_count);
+
+static PFM_RW_ATTR(debug, 0644, debug_show, debug_store);
+static PFM_RW_ATTR(task_group, 0644, task_group_show, task_group_store);
+static PFM_RW_ATTR(arg_mem_max, 0644, arg_size_show, arg_size_store);
+
+static struct attribute *pfm_kernel_attrs[] = {
+	&attr_version.attr,
+	&attr_task_sessions_count.attr,
+	&attr_debug.attr,
+	&attr_task_group.attr,
+	&attr_arg_mem_max.attr,
+	NULL
+};
+
+static ssize_t pfm_kernel_show(struct kobject *kobj,
+			       struct attribute *attr, char *buf)
+{
+	int ret;
+	struct pfm_attribute *fattr = to_attr(attr);
+
+	if (fattr->show)
+		ret = fattr->show(NULL, buf);
+	else
+		ret = -EIO;
+
+	return ret;
+}
+
+static ssize_t pfm_kernel_store(struct kobject *kobj,
+				struct attribute *attr,
+				const char *buf, size_t count)
+{
+	int ret;
+	struct pfm_attribute *fattr = to_attr(attr);
+
+	if (fattr->store)
+		ret = fattr->store(NULL, buf, count);
+	else
+		ret = -EIO;
+	return ret;
+}
+
+static struct sysfs_ops pfm_kernel_sysfs_ops = {
+	.show	= pfm_kernel_show,
+	.store	= pfm_kernel_store,
+};
+
+static struct kobj_type ktype_kernel = {
+	.sysfs_ops	= &pfm_kernel_sysfs_ops,
+	.default_attrs	= pfm_kernel_attrs,
+};
+
+int __init pfm_init_sysfs(void)
+{
+	int ret;
+
+	ret = kobject_init_and_add(&pfm_kernel_kobj, &ktype_kernel,
+				   kobject_get(kernel_kobj), "perfmon");
+	if (ret) {
+		PFM_ERR("cannot add kernel object: %d", ret);
+		return ret;
+	}
+
+	if (pfm_pmu_conf)
+		pfm_sysfs_add_pmu(pfm_pmu_conf);
+
+	return 0;
+}
+
+/*
+ * per-cpu perfmon stats attributes
+ */
+#define PFM_DECL_STATS_ATTR(name) \
+static ssize_t name##_show(void *info, char *buf) \
+{ \
+	struct pfm_stats *st = info;\
+	return snprintf(buf, PAGE_SIZE, "%llu\n", \
+			(unsigned long long)st->name); \
+} \
+static PFM_RO_ATTR(name)
+
+/*
+ * per-reg attributes
+ */
+static ssize_t name_show(void *info, char *buf)
+{
+	struct pfm_regmap_desc *reg = info;
+	return snprintf(buf, PAGE_SIZE, "%s\n", reg->desc);
+}
+static PFM_RO_ATTR(name);
+
+static ssize_t dfl_val_show(void *info, char *buf)
+{
+	struct pfm_regmap_desc *reg = info;
+	return snprintf(buf, PAGE_SIZE, "0x%llx\n",
+			(unsigned long long)reg->dfl_val);
+}
+static PFM_RO_ATTR(dfl_val);
+
+static ssize_t rsvd_msk_show(void *info, char *buf)
+{
+	struct pfm_regmap_desc *reg = info;
+	return snprintf(buf, PAGE_SIZE, "0x%llx\n",
+			(unsigned long long)reg->rsvd_msk);
+}
+static PFM_RO_ATTR(rsvd_msk);
+
+static ssize_t width_show(void *info, char *buf)
+{
+	struct pfm_regmap_desc *reg = info;
+	int w;
+
+	w = (reg->type & PFM_REG_C64) ? pfm_pmu_conf->counter_width : 64;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", w);
+}
+static PFM_RO_ATTR(width);
+
+
+static ssize_t addr_show(void *info, char *buf)
+{
+	struct pfm_regmap_desc *reg = info;
+	return snprintf(buf, PAGE_SIZE, "0x%lx\n", reg->hw_addr);
+}
+static PFM_RO_ATTR(addr);
+
+
+static struct attribute *pfm_reg_attrs[] = {
+	&attr_name.attr,
+	&attr_dfl_val.attr,
+	&attr_rsvd_msk.attr,
+	&attr_width.attr,
+	&attr_addr.attr,
+	NULL
+};
+
+static struct attribute_group pfm_reg_attr_group = {
+	.attrs = pfm_reg_attrs,
+};
+
+static ssize_t model_show(void *info, char *buf)
+{
+	struct pfm_pmu_config *p = info;
+	return snprintf(buf, PAGE_SIZE, "%s\n", p->pmu_name);
+}
+static PFM_RO_ATTR(model);
+
+static struct attribute *pfm_pmu_desc_attrs[] = {
+	&attr_model.attr,
+	NULL
+};
+
+static struct attribute_group pfm_pmu_desc_attr_group = {
+	.attrs = pfm_pmu_desc_attrs,
+};
+
+static int pfm_sysfs_add_pmu_regs(struct pfm_pmu_config *pmu)
+{
+	struct pfm_regmap_desc *reg;
+	unsigned int i, k;
+	int ret;
+
+	reg = pmu->pmc_desc;
+	for (i = 0; i < pmu->num_pmc_entries; i++, reg++) {
+
+		if (!(reg->type & PFM_REG_I))
+			continue;
+
+		kobject_init(&reg->kobj, &pfm_regs_ktype);
+
+		ret = kobject_add(&reg->kobj, &pmu->kobj, "pmc%u", i);
+		if (ret)
+			goto undo_pmcs;
+
+		ret = sysfs_create_group(&reg->kobj, &pfm_reg_attr_group);
+		if (ret) {
+			kobject_del(&reg->kobj);
+			goto undo_pmcs;
+		}
+	}
+
+	reg = pmu->pmd_desc;
+	for (i = 0; i < pmu->num_pmd_entries; i++, reg++) {
+
+		if (!(reg->type & PFM_REG_I))
+			continue;
+
+		kobject_init(&reg->kobj, &pfm_regs_ktype);
+
+		ret = kobject_add(&reg->kobj, &pmu->kobj, "pmd%u", i);
+		if (ret)
+			goto undo_pmds;
+
+		ret = sysfs_create_group(&reg->kobj, &pfm_reg_attr_group);
+		if (ret) {
+			kobject_del(&reg->kobj);
+			goto undo_pmds;
+		}
+	}
+	return 0;
+undo_pmds:
+	reg = pmu->pmd_desc;
+	for (k = 0; k < i; k++, reg++) {
+		if (!(reg->type & PFM_REG_I))
+			continue;
+		sysfs_remove_group(&reg->kobj, &pfm_reg_attr_group);
+		kobject_del(&reg->kobj);
+	}
+	i = pmu->num_pmc_entries;
+	/* fall through */
+undo_pmcs:
+	reg = pmu->pmc_desc;
+	for (k = 0; k < i; k++, reg++) {
+		if (!(reg->type & PFM_REG_I))
+			continue;
+		sysfs_remove_group(&reg->kobj, &pfm_reg_attr_group);
+		kobject_del(&reg->kobj);
+	}
+	return ret;
+}
+
+/*
+ * when a PMU description module is inserted, we create
+ * a pmu_desc subdir in sysfs and we populate it with
+ * PMU specific information, such as register mappings
+ */
+int pfm_sysfs_add_pmu(struct pfm_pmu_config *pmu)
+{
+	int ret;
+
+	kobject_init(&pmu->kobj, &pfm_pmu_ktype);
+
+	ret = kobject_add(&pmu->kobj, &pfm_kernel_kobj, "%s", "pmu_desc");
+	if (ret)
+		return ret;
+
+	ret = sysfs_create_group(&pmu->kobj, &pfm_pmu_desc_attr_group);
+	if (ret)
+		kobject_del(&pmu->kobj);
+
+	ret = pfm_sysfs_add_pmu_regs(pmu);
+	if (ret) {
+		sysfs_remove_group(&pmu->kobj, &pfm_pmu_desc_attr_group);
+		kobject_del(&pmu->kobj);
+	}
+	return ret;
+}
Index: o/perfmon/perfmon_priv.h
===================================================================
--- o.orig/perfmon/perfmon_priv.h	2008-06-04 22:46:24.000000000 +0200
+++ o/perfmon/perfmon_priv.h	2008-06-04 22:46:26.000000000 +0200
@@ -52,6 +52,8 @@
 
 void pfm_free_context(struct pfm_context *ctx);
 
+ssize_t pfm_sysfs_res_show(char *buf, size_t sz, int what);
+
 int pfm_pmu_acquire(void);
 void pfm_pmu_release(void);
 
Index: o/perfmon/perfmon_res.c
===================================================================
--- o.orig/perfmon/perfmon_res.c	2008-06-04 22:46:24.000000000 +0200
+++ o/perfmon/perfmon_res.c	2008-06-04 22:46:26.000000000 +0200
@@ -188,3 +188,36 @@
 	spin_unlock_irqrestore(&pfm_res_lock, flags);
 }
 EXPORT_SYMBOL(pfm_session_allcpus_release);
+
+/**
+ * pfm_sysfs_res_show - return currnt resourcde usage for sysfs
+ * @buf: buffer to hold string in return
+ * @sz: size of buf
+ * @what: what to produce
+ *        what=0 : thread_sessions
+ *        what=1 : cpus_weight(sys_cpumask)
+ *        what=2 : smpl_buf_mem_cur
+ *        what=3 : pmu model name
+ *
+ * called from perfmon_sysfs.c
+ * return number of bytes written into buf (up to sz)
+ */
+ssize_t pfm_sysfs_res_show(char *buf, size_t sz, int what)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&pfm_res_lock, flags);
+
+	switch (what) {
+	case 0: snprintf(buf, sz, "%u\n", pfm_res.thread_sessions);
+		break;
+	case 1: snprintf(buf, sz, "%d\n", cpus_weight(pfm_res.sys_cpumask));
+		break;
+	case 3:
+		snprintf(buf, sz, "%s\n",
+			pfm_pmu_conf ?	pfm_pmu_conf->pmu_name
+				     :	"unknown\n");
+	}
+	spin_unlock_irqrestore(&pfm_res_lock, flags);
+	return strlen(buf);
+}
Index: o/perfmon/Makefile
===================================================================
--- o.orig/perfmon/Makefile	2008-06-04 22:46:24.000000000 +0200
+++ o/perfmon/Makefile	2008-06-04 22:46:26.000000000 +0200
@@ -5,4 +5,5 @@
 obj-$(CONFIG_PERFMON) = perfmon_ctx.o perfmon_ctxsw.o		\
 			perfmon_file.o perfmon_attach.o 	\
 			perfmon_res.o perfmon_init.o		\
-			perfmon_intr.o perfmon_pmu.o
+			perfmon_intr.o perfmon_pmu.o		\
+			perfmon_sysfs.o
Index: o/perfmon/perfmon_pmu.c
===================================================================
--- o.orig/perfmon/perfmon_pmu.c	2008-06-04 22:46:24.000000000 +0200
+++ o/perfmon/perfmon_pmu.c	2008-06-04 22:46:26.000000000 +0200
@@ -164,6 +164,10 @@
 	pfm_pmu_conf = cfg;
 	pfm_pmu_conf->ovfl_mask = (1ULL << cfg->counter_width) - 1;
 
+	ret = pfm_sysfs_add_pmu(pfm_pmu_conf);
+	if (ret)
+		pfm_pmu_conf = NULL;
+
 unlock:
 	spin_unlock(&pfm_pmu_conf_lock);
 

-- 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 12/21] perfmon2 minimal:  sysfs interface
  2008-06-09 22:34 [patch 12/21] perfmon2 minimal: sysfs interface eranian
@ 2008-06-11 14:49 ` Greg KH
  2008-06-11 15:00   ` stephane eranian
  0 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2008-06-11 14:49 UTC (permalink / raw)
  To: eranian; +Cc: linux-kernel

On Mon, Jun 09, 2008 at 03:34:25PM -0700, eranian@googlemail.com wrote:
> +
> +static struct kobject pfm_kernel_kobj;

Eeek, no, please look at Documentation/kobject.txt that says to not ever
have static kobjects.

I think a lot of this code can be cleaned up and shrunk given the way
that kobjects have changed recently.  See the examples in
samples/kobject/ and the documentation file for more details.

If you have any questions about it, please let me know.

Also, whenever you add a sysfs file to the kernel, please document it in
Documentation/ABI/ so that everyone knows what it is and how to use it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 12/21] perfmon2 minimal: sysfs interface
  2008-06-11 14:49 ` Greg KH
@ 2008-06-11 15:00   ` stephane eranian
  2008-06-12 12:54     ` stephane eranian
  0 siblings, 1 reply; 8+ messages in thread
From: stephane eranian @ 2008-06-11 15:00 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

Greg,

On Wed, Jun 11, 2008 at 4:49 PM, Greg KH <greg@kroah.com> wrote:
> On Mon, Jun 09, 2008 at 03:34:25PM -0700, eranian@googlemail.com wrote:
>> +
>> +static struct kobject pfm_kernel_kobj;
>
> Eeek, no, please look at Documentation/kobject.txt that says to not ever
> have static kobjects.
>
Will do.

> I think a lot of this code can be cleaned up and shrunk given the way
> that kobjects have changed recently.  See the examples in
> samples/kobject/ and the documentation file for more details.
>
If you say so. When building the series, I also wondered why there was
so much code in there. But it was not clear to me how it could be shrunk.
I saw some simplifications in 2.6.26 but still a lot of small functions.

> If you have any questions about it, please let me know.
>
> Also, whenever you add a sysfs file to the kernel, please document it in
> Documentation/ABI/ so that everyone knows what it is and how to use it.
>

I have that in my GIT tree. I simply forgot to include them in the quilt series.
I will fix that.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 12/21] perfmon2 minimal: sysfs interface
  2008-06-11 15:00   ` stephane eranian
@ 2008-06-12 12:54     ` stephane eranian
  2008-06-13  3:50       ` Greg KH
  2008-06-13  3:53       ` Greg KH
  0 siblings, 2 replies; 8+ messages in thread
From: stephane eranian @ 2008-06-12 12:54 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 699 bytes --]

Greg,

I looked at the documentation and the examples.

Find attached an updated version of the perfmon-sysfs.diff patch.

I have grouped attribute functions into common show/store functions
with a strcmp on
the attribute name. I have also used the kobject_init_and_add() function.

Once thing still confusing to me is the business with kobject_put()
after a kobject_init_and_add().
Is that necessary to drop the ref count to zero?

Also when you link to a parent,  like this:
	pfm_kernel_kobj = kobject_create_and_add("perfmon", kernel_kobj);

Does this increment the ref count on the parent?

In any case, let me know if there are still things that must be
changed/simplified in my file.

Thanks.

[-- Attachment #2: perfmon-sysfs.diff --]
[-- Type: application/octet-stream, Size: 16486 bytes --]

Subject: sysfs interface

This patch adds the sysfs interface to the perfmon2 subsystem. It is
used for configuration of the interface. It exposes the PMU register
mappings and various attributes of the subsystem.

Signed-off-by: Stephane Eranian <eranian@gmail.com>
--

Index: o/perfmon/perfmon_init.c
===================================================================
--- o.orig/perfmon/perfmon_init.c	2008-06-11 23:54:07.000000000 +0200
+++ o/perfmon/perfmon_init.c	2008-06-11 23:54:33.000000000 +0200
@@ -62,6 +62,9 @@
 	if (pfm_init_fs())
 		goto error_disable;
 
+	if (pfm_init_sysfs())
+		goto error_disable;
+
 	/* not critical, so no error checking */
 	pfm_init_debugfs();
 
Index: o/perfmon/perfmon_sysfs.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ o/perfmon/perfmon_sysfs.c	2008-06-12 14:42:51.000000000 +0200
@@ -0,0 +1,348 @@
+/*
+ * perfmon_sysfs.c: perfmon2 sysfs interface
+ *
+ * This file implements the perfmon2 interface which
+ * provides access to the hardware performance counters
+ * of the host processor.
+ *
+ * The initial version of perfmon.c was written by
+ * Ganesh Venkitachalam, IBM Corp.
+ *
+ * Then it was modified for perfmon-1.x by Stephane Eranian and
+ * David Mosberger, Hewlett Packard Co.
+ *
+ * Version Perfmon-2.x is a complete rewrite of perfmon-1.x
+ * by Stephane Eranian, Hewlett Packard Co.
+ *
+ * Copyright (c) 1999-2006 Hewlett-Packard Development Company, L.P.
+ * Contributed by Stephane Eranian <eranian@hpl.hp.com>
+ *                David Mosberger-Tang <davidm@hpl.hp.com>
+ *
+ * More information about perfmon available at:
+ * 	http://perfmon2.sf.net
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+ * 02111-1307 USA
+ */
+#include <linux/kernel.h>
+#include <linux/module.h> /* for EXPORT_SYMBOL */
+#include <linux/perfmon_kern.h>
+#include "perfmon_priv.h"
+
+struct pfm_attribute {
+	struct attribute attr;
+	ssize_t (*show)(void *, struct pfm_attribute *attr, char *);
+	ssize_t (*store)(void *, const char *, size_t);
+};
+#define to_attr(n) container_of(n, struct pfm_attribute, attr);
+
+#define PFM_ROS_ATTR(_name, _show) \
+	struct pfm_attribute attr_##_name = __ATTR(_name, 0444, _show, NULL)
+
+#define PFM_RO_ATTR(_name, _show) \
+	struct kobj_attribute attr_##_name = __ATTR(_name, 0444, _show, NULL)
+
+#define PFM_RW_ATTR(_name, _show, _store) 			\
+	struct kobj_attribute attr_##_name = __ATTR(_name, 0644, _show, _store)
+
+#define is_attr_name(a, n) (!strcmp((a)->attr.name, n))
+int pfm_sysfs_add_pmu(struct pfm_pmu_config *pmu);
+
+static struct kobject *pfm_kernel_kobj;
+
+static ssize_t pfm_pmu_attr_show(struct kobject *kobj,
+		struct attribute *attr, char *buf)
+{
+	struct pfm_pmu_config *pmu = to_pmu(kobj);
+	struct pfm_attribute *attribute = to_attr(attr);
+	return attribute->show ? attribute->show(pmu, attribute, buf) : -EIO;
+}
+
+static ssize_t pfm_regs_attr_show(struct kobject *kobj,
+		struct attribute *attr, char *buf)
+{
+	struct pfm_regmap_desc *reg = to_reg(kobj);
+	struct pfm_attribute *attribute = to_attr(attr);
+	return attribute->show ? attribute->show(reg, attribute, buf) : -EIO;
+}
+
+static struct sysfs_ops pfm_pmu_sysfs_ops = {
+	.show = pfm_pmu_attr_show
+};
+
+static struct sysfs_ops pfm_regs_sysfs_ops = {
+	.show  = pfm_regs_attr_show
+};
+
+
+static struct kobj_type pfm_pmu_ktype = {
+	.sysfs_ops = &pfm_pmu_sysfs_ops,
+};
+
+static struct kobj_type pfm_regs_ktype = {
+	.sysfs_ops = &pfm_regs_sysfs_ops,
+};
+
+static ssize_t pfm_controls_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+
+	if (is_attr_name(attr, "version"))
+		return snprintf(buf, PAGE_SIZE, "%u.%u\n",  PFM_VERSION_MAJ, PFM_VERSION_MIN);
+
+	if (is_attr_name(attr, "task_sessions_count"))
+		return pfm_sysfs_res_show(buf, PAGE_SIZE, 0);
+
+	if (is_attr_name(attr, "debug"))
+		return snprintf(buf, PAGE_SIZE, "%d\n", pfm_controls.debug);
+
+	if (is_attr_name(attr, "task_group"))
+		return snprintf(buf, PAGE_SIZE, "%d\n", pfm_controls.task_group);
+
+	if (is_attr_name(attr, "arg_mem_max"))
+		return snprintf(buf, PAGE_SIZE, "%zu\n", pfm_controls.arg_mem_max);
+
+	return 0;
+}
+
+static ssize_t pfm_controls_store(struct kobject *kobj, struct kobj_attribute *attr,
+			 	  const char *buf, size_t count)
+{
+	size_t d;
+
+	if (sscanf(buf, "%zu", &d) != 1)
+		goto skip;
+
+	if (is_attr_name(attr, "debug"))
+		pfm_controls.debug = d;
+
+	if (is_attr_name(attr, "task_group"))
+		pfm_controls.task_group = d;
+
+	if (is_attr_name(attr, "arg_mem_max")) {
+		/*
+		 * we impose a page as the minimum.
+		 *
+		 * This limit may be smaller than the stack buffer
+		 * available and that is fine.
+		 */
+		if (d >= PAGE_SIZE)
+			pfm_controls.arg_mem_max = d;
+	}
+
+skip:
+	return count;
+}
+
+/*
+ * /sys/kernel/perfmon attributes
+ */
+static PFM_RO_ATTR(version, pfm_controls_show);
+static PFM_RO_ATTR(task_sessions_count, pfm_controls_show);
+static PFM_RW_ATTR(debug, pfm_controls_show, pfm_controls_store);
+static PFM_RW_ATTR(task_group, pfm_controls_show, pfm_controls_store);
+static PFM_RW_ATTR(arg_mem_max, pfm_controls_show, pfm_controls_store);
+
+static struct attribute *pfm_kernel_attrs[] = {
+	&attr_version.attr,
+	&attr_task_sessions_count.attr,
+	&attr_debug.attr,
+	&attr_task_group.attr,
+	&attr_arg_mem_max.attr,
+	NULL
+};
+
+static struct attribute_group pfm_kernel_attr_group = {
+	.attrs = pfm_kernel_attrs,
+};
+
+/*
+ * per-reg attributes
+ */
+static ssize_t pfm_reg_show(void *data, struct pfm_attribute *attr, char *buf)
+{
+	struct pfm_regmap_desc *reg;
+	int w;
+
+	reg = data;
+
+	if (is_attr_name(attr, "name"))
+		return snprintf(buf, PAGE_SIZE, "%s\n", reg->desc);
+
+	if (is_attr_name(attr, "dfl_val"))
+		return snprintf(buf, PAGE_SIZE, "0x%llx\n",
+				(unsigned long long)reg->dfl_val);
+
+	if (is_attr_name(attr, "width")) {
+		w = (reg->type & PFM_REG_C64) ?
+		    pfm_pmu_conf->counter_width : 64;
+		return snprintf(buf, PAGE_SIZE, "%d\n", w);
+	}
+
+	if (is_attr_name(attr, "rsvd_msk"))
+		return snprintf(buf, PAGE_SIZE, "0x%llx\n",
+				(unsigned long long)reg->rsvd_msk);
+
+	if (is_attr_name(attr, "addr"))
+		return snprintf(buf, PAGE_SIZE, "0x%lx\n", reg->hw_addr);
+
+	return 0;
+}
+
+static PFM_ROS_ATTR(name, pfm_reg_show);
+static PFM_ROS_ATTR(dfl_val, pfm_reg_show);
+static PFM_ROS_ATTR(rsvd_msk, pfm_reg_show);
+static PFM_ROS_ATTR(width, pfm_reg_show);
+static PFM_ROS_ATTR(addr, pfm_reg_show);
+
+static struct attribute *pfm_reg_attrs[] = {
+	&attr_name.attr,
+	&attr_dfl_val.attr,
+	&attr_rsvd_msk.attr,
+	&attr_width.attr,
+	&attr_addr.attr,
+	NULL
+};
+
+static struct attribute_group pfm_reg_attr_group = {
+	.attrs = pfm_reg_attrs,
+};
+
+static ssize_t model_show(void *info, struct pfm_attribute *attr, char *buf)
+{
+	struct pfm_pmu_config *p = info;
+	return snprintf(buf, PAGE_SIZE, "%s\n", p->pmu_name);
+}
+static PFM_ROS_ATTR(model, model_show);
+
+static struct attribute *pfm_pmu_desc_attrs[] = {
+	&attr_model.attr,
+	NULL
+};
+
+static struct attribute_group pfm_pmu_desc_attr_group = {
+	.attrs = pfm_pmu_desc_attrs,
+};
+
+static int pfm_sysfs_add_pmu_regs(struct pfm_pmu_config *pmu)
+{
+	struct pfm_regmap_desc *reg;
+	unsigned int i, k;
+	int ret;
+
+	reg = pmu->pmc_desc;
+	for (i = 0; i < pmu->num_pmc_entries; i++, reg++) {
+
+		if (!(reg->type & PFM_REG_I))
+			continue;
+
+		ret = kobject_init_and_add(&reg->kobj, &pfm_regs_ktype,
+					   &pmu->kobj, "pmc%u", i);
+		if (ret)
+			goto undo_pmcs;
+
+		ret = sysfs_create_group(&reg->kobj, &pfm_reg_attr_group);
+		if (ret) {
+			kobject_del(&reg->kobj);
+			goto undo_pmcs;
+		}
+	}
+
+	reg = pmu->pmd_desc;
+	for (i = 0; i < pmu->num_pmd_entries; i++, reg++) {
+
+		if (!(reg->type & PFM_REG_I))
+			continue;
+
+		ret = kobject_init_and_add(&reg->kobj, &pfm_regs_ktype,
+					   &pmu->kobj, "pmd%u", i);
+		if (ret)
+			goto undo_pmds;
+
+		ret = sysfs_create_group(&reg->kobj, &pfm_reg_attr_group);
+		if (ret) {
+			kobject_del(&reg->kobj);
+			goto undo_pmds;
+		}
+	}
+	return 0;
+undo_pmds:
+	reg = pmu->pmd_desc;
+	for (k = 0; k < i; k++, reg++) {
+		if (!(reg->type & PFM_REG_I))
+			continue;
+		sysfs_remove_group(&reg->kobj, &pfm_reg_attr_group);
+		kobject_del(&reg->kobj);
+	}
+	i = pmu->num_pmc_entries;
+	/* fall through */
+undo_pmcs:
+	reg = pmu->pmc_desc;
+	for (k = 0; k < i; k++, reg++) {
+		if (!(reg->type & PFM_REG_I))
+			continue;
+		sysfs_remove_group(&reg->kobj, &pfm_reg_attr_group);
+		kobject_del(&reg->kobj);
+	}
+	return ret;
+}
+
+/*
+ * when a PMU description module is inserted, we create
+ * a pmu_desc subdir in sysfs and we populate it with
+ * PMU specific information, such as register mappings
+ */
+int pfm_sysfs_add_pmu(struct pfm_pmu_config *pmu)
+{
+	int ret;
+
+	ret = kobject_init_and_add(&pmu->kobj, &pfm_pmu_ktype,
+				   pfm_kernel_kobj, "pmu_desc");
+	if (ret)
+		return ret;
+
+	ret = sysfs_create_group(&pmu->kobj, &pfm_pmu_desc_attr_group);
+	if (ret)
+		kobject_del(&pmu->kobj);
+
+	ret = pfm_sysfs_add_pmu_regs(pmu);
+	if (ret) {
+		sysfs_remove_group(&pmu->kobj, &pfm_pmu_desc_attr_group);
+		kobject_del(&pmu->kobj);
+	} else
+		kobject_uevent(&pmu->kobj, KOBJ_ADD);
+
+	return ret;
+}
+
+int __init pfm_init_sysfs(void)
+{
+	int ret;
+
+	pfm_kernel_kobj = kobject_create_and_add("perfmon", kernel_kobj);
+	if (!pfm_kernel_kobj) {
+		PFM_ERR("cannot add kernel object");
+		return -ENOMEM;
+	}
+
+	ret = sysfs_create_group(pfm_kernel_kobj, &pfm_kernel_attr_group);
+	if (ret) {
+		kobject_put(pfm_kernel_kobj);
+		return ret;
+	}
+
+	if (pfm_pmu_conf)
+		pfm_sysfs_add_pmu(pfm_pmu_conf);
+
+	return 0;
+}
Index: o/perfmon/perfmon_priv.h
===================================================================
--- o.orig/perfmon/perfmon_priv.h	2008-06-11 23:54:32.000000000 +0200
+++ o/perfmon/perfmon_priv.h	2008-06-12 14:47:22.000000000 +0200
@@ -52,6 +52,8 @@
 
 void pfm_free_context(struct pfm_context *ctx);
 
+ssize_t pfm_sysfs_res_show(char *buf, size_t sz, int what);
+
 int pfm_pmu_acquire(void);
 void pfm_pmu_release(void);
 
Index: o/perfmon/perfmon_res.c
===================================================================
--- o.orig/perfmon/perfmon_res.c	2008-06-11 23:54:30.000000000 +0200
+++ o/perfmon/perfmon_res.c	2008-06-11 23:54:33.000000000 +0200
@@ -188,3 +188,36 @@
 	spin_unlock_irqrestore(&pfm_res_lock, flags);
 }
 EXPORT_SYMBOL(pfm_session_allcpus_release);
+
+/**
+ * pfm_sysfs_res_show - return currnt resourcde usage for sysfs
+ * @buf: buffer to hold string in return
+ * @sz: size of buf
+ * @what: what to produce
+ *        what=0 : thread_sessions
+ *        what=1 : cpus_weight(sys_cpumask)
+ *        what=2 : smpl_buf_mem_cur
+ *        what=3 : pmu model name
+ *
+ * called from perfmon_sysfs.c
+ * return number of bytes written into buf (up to sz)
+ */
+ssize_t pfm_sysfs_res_show(char *buf, size_t sz, int what)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&pfm_res_lock, flags);
+
+	switch (what) {
+	case 0: snprintf(buf, sz, "%u\n", pfm_res.thread_sessions);
+		break;
+	case 1: snprintf(buf, sz, "%d\n", cpus_weight(pfm_res.sys_cpumask));
+		break;
+	case 3:
+		snprintf(buf, sz, "%s\n",
+			pfm_pmu_conf ?	pfm_pmu_conf->pmu_name
+				     :	"unknown\n");
+	}
+	spin_unlock_irqrestore(&pfm_res_lock, flags);
+	return strlen(buf);
+}
Index: o/perfmon/Makefile
===================================================================
--- o.orig/perfmon/Makefile	2008-06-11 23:54:32.000000000 +0200
+++ o/perfmon/Makefile	2008-06-12 14:47:22.000000000 +0200
@@ -5,4 +5,5 @@
 obj-$(CONFIG_PERFMON) = perfmon_ctx.o perfmon_ctxsw.o		\
 			perfmon_file.o perfmon_attach.o 	\
 			perfmon_res.o perfmon_init.o		\
-			perfmon_intr.o perfmon_pmu.o
+			perfmon_intr.o perfmon_pmu.o		\
+			perfmon_sysfs.o
Index: o/perfmon/perfmon_pmu.c
===================================================================
--- o.orig/perfmon/perfmon_pmu.c	2008-06-11 23:54:32.000000000 +0200
+++ o/perfmon/perfmon_pmu.c	2008-06-11 23:54:33.000000000 +0200
@@ -164,6 +164,10 @@
 	pfm_pmu_conf = cfg;
 	pfm_pmu_conf->ovfl_mask = (1ULL << cfg->counter_width) - 1;
 
+	ret = pfm_sysfs_add_pmu(pfm_pmu_conf);
+	if (ret)
+		pfm_pmu_conf = NULL;
+
 unlock:
 	spin_unlock(&pfm_pmu_conf_lock);
 
Index: o/Documentation/ABI/testing/sysfs-perfmon
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ o/Documentation/ABI/testing/sysfs-perfmon	2008-06-11 23:54:33.000000000 +0200
@@ -0,0 +1,38 @@
+What:		/sys/kernel/perfmon
+Date:		June 2008
+KernelVersion:	2.6.26
+Contact:	eranian@gmail.com
+
+Description:	provide the configuration interface for the perfmon2 subsystems.
+	        The tree contains information about the detected hardware, current
+		state of the subsystem as well as some configuration parameters.
+
+		The tree consists of the following entries:
+
+   	/sys/kernel/perfmon/version (read-only):
+
+		Perfmon2 interface revision number.
+
+	/sys/kernel/perfmon/task_sessions_count (read-only):
+
+		Number of per-thread contexts currently attached to threads.
+
+	/sys/kernel/perfmon/debug (read-write):
+
+		Enable perfmon2 debugging output via klogd. Debug messages produced during
+		PMU interrupt handling are not controlled by this entry. The traces a rate-limited
+		to avoid flooding of the console. It is possible to change the throttling
+	        via /proc/sys/kernel/printk_ratelimit. The value is interpreted as a bitmask.
+		Each bit enables a particular type of debug messages. Refer to the file
+		include/linux/perfmon_kern.h for more information
+
+	/sys/kernel/perfmon/task_group (read-write):
+
+		Users group allowed to create a per-thread context (session).
+   		-1 means any group. This control will be kept until we find a
+		package able to control capabilities via PAM.
+
+	/sys/kernel/perfmon/arg_mem_max(read-write):
+
+		Maximum size of vector arguments expressed in bytes. Can be modified
+		but must be at least a page. Default is PAGE_SIZE
Index: o/Documentation/ABI/testing/sysfs-perfmon-pmu
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ o/Documentation/ABI/testing/sysfs-perfmon-pmu	2008-06-11 23:54:33.000000000 +0200
@@ -0,0 +1,46 @@
+What:		/sys/kernel/perfmon/pmu
+Date:		June 2008
+KernelVersion:	2.6.26
+Contact:	eranian@gmail.com
+
+Description:	provide information about the currently loaded PMU description module.
+		The module contains the mapping of the actual performance counter registers
+		onto the logical PMU exposed by perfmon.  There is at most one PMU description
+		module loaded at any time.
+
+		The sysfs PMU tree provides a description of the mapping for each register.
+		There is one subdir per config and data registers along an entry for the
+		name of the PMU model.
+
+		The model entry is as follows:
+
+	/sys/kernel/perfmon/pmu_desc/model (read-only):
+
+		Name of the PMU model is clear text and zero terminated.
+
+		Then for each logical PMU register, XX, gets a subtree with the following entries:
+
+	/sys/kernel/perfmon/pmu_desc/pm*XX/addr (read-only):
+
+		The physical address or index of the actual underlying hardware register.
+		On Itanium, it corresponds to the index. But on X86 processor, this is
+		the actual MSR address.
+
+	/sys/kernel/perfmon/pmu_desc/pm*XX/dfl_val (read-only):
+
+		The default value of the register in hexadecimal.
+
+	/sys/kernel/perfmon/pmu_desc/pm*XX/name (read-only):
+
+		The name of the hardware register.
+
+	/sys/kernel/perfmon/pmu_desc/pm*XX/rsvd_msk (read-only):
+
+		The bitmask of reserved bits, i.e., bits which cannot be changed by
+		applications. When a bit is set, it means the corresponding bit in the
+		actual register is reserved.
+
+	/sys/kernel/perfmon/pmu_desc/pm*XX/width (read-only):
+
+		the width in bits of the registers. This field is only relevant for counter
+		registers.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 12/21] perfmon2 minimal: sysfs interface
  2008-06-12 12:54     ` stephane eranian
@ 2008-06-13  3:50       ` Greg KH
  2008-06-13  3:53       ` Greg KH
  1 sibling, 0 replies; 8+ messages in thread
From: Greg KH @ 2008-06-13  3:50 UTC (permalink / raw)
  To: eranian; +Cc: linux-kernel

On Thu, Jun 12, 2008 at 02:54:15PM +0200, stephane eranian wrote:
> Greg,
> 
> I looked at the documentation and the examples.
> 
> Find attached an updated version of the perfmon-sysfs.diff patch.
> 
> I have grouped attribute functions into common show/store functions
> with a strcmp on
> the attribute name. I have also used the kobject_init_and_add() function.
> 
> Once thing still confusing to me is the business with kobject_put()
> after a kobject_init_and_add().
> Is that necessary to drop the ref count to zero?

Yes.

> Also when you link to a parent,  like this:
> 	pfm_kernel_kobj = kobject_create_and_add("perfmon", kernel_kobj);
> 
> Does this increment the ref count on the parent?

Yes.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 12/21] perfmon2 minimal: sysfs interface
  2008-06-12 12:54     ` stephane eranian
  2008-06-13  3:50       ` Greg KH
@ 2008-06-13  3:53       ` Greg KH
  2008-06-13 22:31         ` stephane eranian
  1 sibling, 1 reply; 8+ messages in thread
From: Greg KH @ 2008-06-13  3:53 UTC (permalink / raw)
  To: eranian; +Cc: linux-kernel

On Thu, Jun 12, 2008 at 02:54:15PM +0200, stephane eranian wrote:
> 
> In any case, let me know if there are still things that must be
> changed/simplified in my file.

This looks much better.  I wonder if you could use the "default" kobject
attributes, which might allow you to remove some of your show/store
wrappers, but in general, it's much better than before.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 12/21] perfmon2 minimal: sysfs interface
  2008-06-13  3:53       ` Greg KH
@ 2008-06-13 22:31         ` stephane eranian
  2008-06-17  8:52           ` stephane eranian
  0 siblings, 1 reply; 8+ messages in thread
From: stephane eranian @ 2008-06-13 22:31 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

Greg,



On Fri, Jun 13, 2008 at 5:53 AM, Greg KH <greg@kroah.com> wrote:
> On Thu, Jun 12, 2008 at 02:54:15PM +0200, stephane eranian wrote:
>>
>> In any case, let me know if there are still things that must be
>> changed/simplified in my file.
>
> This looks much better.  I wonder if you could use the "default" kobject
> attributes, which might allow you to remove some of your show/store
> wrappers, but in general, it's much better than before.
>
Well, I wondered about that myself, but I think I am missing one piece.
The difference between some of the show/store functions and others
is that if you look closely, you see that some make reference to global
structures, e.g., pfm_controls, or call out to routines in others modules.
For those, I have switched to the default attribute. But the other show/store
functions need to access the particular object where the kobj is embedded.
And for that, it seems you need to glue function:

#define to_pmu(n) container_of(n, struct pfm_pmu_config, kobj)

static ssize_t pfm_pmu_attr_show(struct kobject *kobj,
                struct attribute *attr, char *buf)
{
        struct pfm_pmu_config *pmu = to_pmu(kobj);
        struct pfm_attribute *attribute = to_attr(attr);
        return attribute->show ? attribute->show(pmu, attribute, buf) : -EIO;
}

which basically calls the container_of() routine..

Yet, I get the feeling that is the show routein gets the same kobj,
then I could as well do the
to_pmu() there. And the attr (use the the strcmp()) would be the default.

If you tell me this is the way to go, I will certainly be happy to
make the change and remove even
more code!


Thanks.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 12/21] perfmon2 minimal: sysfs interface
  2008-06-13 22:31         ` stephane eranian
@ 2008-06-17  8:52           ` stephane eranian
  0 siblings, 0 replies; 8+ messages in thread
From: stephane eranian @ 2008-06-17  8:52 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel

Greg,

I looked at this some more today.
I was able to remove the need for a custom attribute structure for one
of the objects.

I think it is not possible to use the generic kobj_attribute if you
need to refer to a particular
object in the show/store, i.e, not a global data structure. In other
words, if you need to use
the kobj pointer passed to the show() routine to access your data
structure, then you need to
rely on container_of() and thus the kobject MUST be embedded into the
data structure defining
the object. It cannot just be a kobject pointer.

The kobject interface to create and add objects come in two forms, the
one which requires
a ktype and the one which does not, i.e., kobject_create() and its
derivatives. It seems like
if you want to leverage the kobj_attribute, you need to avoid all
calls which require a ktype.
But then there is no such function which would work on an already
allocated kobject.
kobject_create_and_add() does allocate the kobject. You do not want
that because it means
the kobject is not embedded into your data structure anymore.

Am I missing something here?

Thanks.



On Sat, Jun 14, 2008 at 12:31 AM, stephane eranian
<eranian@googlemail.com> wrote:
> Greg,
>
>
>
> On Fri, Jun 13, 2008 at 5:53 AM, Greg KH <greg@kroah.com> wrote:
>> On Thu, Jun 12, 2008 at 02:54:15PM +0200, stephane eranian wrote:
>>>
>>> In any case, let me know if there are still things that must be
>>> changed/simplified in my file.
>>
>> This looks much better.  I wonder if you could use the "default" kobject
>> attributes, which might allow you to remove some of your show/store
>> wrappers, but in general, it's much better than before.
>>
> Well, I wondered about that myself, but I think I am missing one piece.
> The difference between some of the show/store functions and others
> is that if you look closely, you see that some make reference to global
> structures, e.g., pfm_controls, or call out to routines in others modules.
> For those, I have switched to the default attribute. But the other show/store
> functions need to access the particular object where the kobj is embedded.
> And for that, it seems you need to glue function:
>
> #define to_pmu(n) container_of(n, struct pfm_pmu_config, kobj)
>
> static ssize_t pfm_pmu_attr_show(struct kobject *kobj,
>                struct attribute *attr, char *buf)
> {
>        struct pfm_pmu_config *pmu = to_pmu(kobj);
>        struct pfm_attribute *attribute = to_attr(attr);
>        return attribute->show ? attribute->show(pmu, attribute, buf) : -EIO;
> }
>
> which basically calls the container_of() routine..
>
> Yet, I get the feeling that is the show routein gets the same kobj,
> then I could as well do the
> to_pmu() there. And the attr (use the the strcmp()) would be the default.
>
> If you tell me this is the way to go, I will certainly be happy to
> make the change and remove even
> more code!
>
>
> Thanks.
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2008-06-17  8:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-06-09 22:34 [patch 12/21] perfmon2 minimal: sysfs interface eranian
2008-06-11 14:49 ` Greg KH
2008-06-11 15:00   ` stephane eranian
2008-06-12 12:54     ` stephane eranian
2008-06-13  3:50       ` Greg KH
2008-06-13  3:53       ` Greg KH
2008-06-13 22:31         ` stephane eranian
2008-06-17  8:52           ` stephane eranian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).