Messages in this thread | | | Date | Fri, 1 Sep 2006 09:09:25 -0700 | From | Stephane Eranian <> | Subject | Re: [PATCH 6/18] 2.6.17.9 perfmon2 patch for review: sampling format support |
| |
Andrew,
On Wed, Aug 23, 2006 at 03:35:37PM -0700, Andrew Morton wrote: > On Wed, 23 Aug 2006 01:05:57 -0700 > Stephane Eranian <eranian@frankl.hpl.hp.com> wrote: > > > This files contains the sampling format support. > > > > Perfmon2 supports an in-kernel sampling buffer for performance > > reasons. Yet to ensure maximum flexibility to applications, > > the formats is which infmration is recorded into the kernel > > buffer is not specified by the interface. Instead it is > > delegated to a kernel plug-in modules called sampling formats. > > > > Each formats controls: > > - what is recorded in the the sampling buffer > > - how the information is recorded > > - when to notify the application to extract the information > > - how the buffer is exported to user level > > - hoe the buffer is allocated > > > > Each format is identified via a 128-bit UUID which can be requested > > when the context is created with pfm_create_context(). > > > > The interface comes with a simple default sampling format. It records > > information sequentially in the buffer. Each entry, called sample, > > is composed of a fixed size header and a variable size body where > > the values of PMDS can be recorded based upon the user's request. > > > > Sampling formats can be dynamically registered with perfmon. The management > > of sampling formats is implemented in perfmon_fmt.c: > > > > pfm_register_smpl_fmt(struct pfm_smpl_fmt *fmt): > > - register a new sampling format > > > > pfm_unregister_smpl_fmt(pfm_uuid_t uuid): > > - unregister a sampling format > > > > It is possible to list the available formats by looking at /sys/kernel/perfmon/formats. > > > > Why identify a format with a UUID rather than via a nice human-readable name? >
Although a UUID is slightly more difficult to manipulate than a clear text string, it offers several advantages: - is guaranteed unique - generation is fully distributed - easy generation with uuidgen - fixed size - very easy to pass to the kernel, there is not char * in a struct pass to kernel - not to worry about '\0'
We use UUID to idenitfy a format + a version number. The version number can be useful to identify backward compatible versions of a format.
> > +/* > > + * find a buffer format based on its uuid > > + */ > > +struct pfm_smpl_fmt *pfm_smpl_fmt_get(pfm_uuid_t uuid) > > +{ > > + struct pfm_smpl_fmt * fmt; > > + > > + spin_lock(&pfm_smpl_fmt_lock); > > + > > + fmt = __pfm_find_smpl_fmt(uuid); > > + > > + /* > > + * increase module refcount > > + */ > > + if (fmt && fmt_is_mod(fmt) && !try_module_get(fmt->owner)) > > + fmt = NULL; > > + > > + spin_unlock(&pfm_smpl_fmt_lock); > > + > > + return fmt; > > +} > > Is pfm_smpl_fmt_lock really needed? The module API _should_ be unracy wrt > lookup and removal. If the name of the module was equal to the name of the > format (sensible) then perhaps the module system's > refcounting/atomicity/lookup mechanisms are sufficient? >
I believe the module API + refcount make it safe against removal. When perfmon does a lookup in the linked list of registered formats, it is not clear to me what would protect it. I believe it would be safe if I used the rcu list instead of basic list.
> > + pfm_sysfs_add_fmt(fmt); > > Please check for and handle all sysfs-related errors. All errors, indeed. > > Yes, a lot of the kernel blithely assumes that sysfs operations never fail. > We need to fix that badness rather than copy it. > I fixed that now.
thanks. -- -Stephane - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |