perfmon2 minimal: sysfs interface
PATCH 12/21 perfmon2 minimal: 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.
--
perfmon/perfmon_init.c +3 -0
@@ -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 ();
perfmon/perfmon_sysfs.c new file +422 -0
422 lines not on this page · raw message
perfmon/perfmon_priv.h +2 -0
@@ -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 );
perfmon/perfmon_res.c +33 -0
@@ -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); +}
perfmon/Makefile +2 -1
@@ -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
perfmon/perfmon_pmu.c +4 -0
@@ -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);
Review
Quoted 2 lines · On Mon, Jun 09, 2008 at 03:34:25PM -0700, [email protected] wrote:
(quoted diff) +2
-0+ +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
Greg,
Quoted 7 lines · Greg KH
Review
Quoted 2 lines · On Mon, Jun 09, 2008 at 03:34:25PM -0700, [email protected] wrote:
(quoted diff) +2
-0+ +static struct kobject pfm_kernel_kobj;Eeek, no, please look at
Documentation/kobject.txtthat says to not ever have static kobjects.
Will do.
Quoted 4 lines
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.
Quoted 5 lines
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.
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.
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.
--
perfmon/perfmon_init.c +3 -0
@@ -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 ();
perfmon/perfmon_sysfs.c new file +348 -0
348 lines not on this page · raw message
perfmon/perfmon_priv.h +2 -0
@@ -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 );
perfmon/perfmon_res.c +33 -0
@@ -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); +}
perfmon/Makefile +2 -1
@@ -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
perfmon/perfmon_pmu.c +4 -0
@@ -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);
Documentation/ABI/testing/sysfs-perfmon new file +38 -0
@@ -0,0 +1,38 @@ +What: /sys/kernel/perfmon +Date: June 2008 +KernelVersion: 2.6.26 +Contact: [email protected] + +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
Documentation/ABI/testing/sysfs-perfmon-pmu new file +46 -0
46 lines not on this page · raw message
Quoted 6 lines · stephane eranian
Greg,
I looked at the documentation and the examples.
Find attached an updated version of the perfmon-sysfs.diff patch.
Quoted 7 lines
I have grouped attribute functions into common show/store functions with a strcmp on
the attribute name. I have also used thekobject_init_and_add()function.Once thing still confusing to me is the business with
kobject_put()after akobject_init_and_add().
Is that necessary to drop the ref count to zero?
Yes.
Quoted 4 lines
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
Quoted 3 lines · stephane eranian
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
Greg,
Quoted 9 lines · Greg KH
Quoted 3 lines · stephane eranian
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 theto_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.
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.
Quoted 45 lines · stephane eranian
Greg,
Quoted 9 lines · Greg KH
Quoted 3 lines · stephane eranian
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_tpfm_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 thestrcmp()) 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.