lkml.org 
[lkml]   [2008]   [Nov]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
DateWed, 26 Nov 2008 15:08:35 +0100
FromIngo Molnar <>
SubjectRe: [patch 20/24] perfmon: system calls interface

* eranian@googlemail.com <eranian@googlemail.com> wrote:

> +/*
> + * unlike the other perfmon system calls, this one returns a file descriptor
> + * or a value < 0 in case of error, very much like open() or socket()
> + */
> +asmlinkage long sys_pfm_create(int flags, struct pfarg_sinfo __user *ureq)
> +{
> + struct pfm_context *new_ctx;
> + struct pfarg_sinfo sif;
> + int ret;
> +
> + PFM_DBG("flags=0x%x sif=%p", flags, ureq);
> +
> + if (perfmon_disabled)
> + return -ENOSYS;

uhm. So we have a dynamic 'we dont support perfmon' flag. Which is
global and defined as:

+int perfmon_disabled; /* >0 if perfmon is disabled */
(sidenote: that should be __read_mostly)

then we go:

> + if (flags) {
> + PFM_DBG("no flags accepted yet");
> + return -EINVAL;
> + }

that should be if (unlikely())
then:

> + ret = __pfm_create_context(flags, &sif, &new_ctx);

where we get:

+int __pfm_create_context(__u32 ctx_flags,
+ struct pfarg_sinfo *sif,
+ struct pfm_context **new_ctx)
+{
+ struct pfm_context *ctx;
+ struct file *filp = NULL;
+ int fd = 0, ret = -EINVAL;
+
+ if (!pfm_pmu_conf)
+ return -ENOSYS;
+
_ANOTHER_ global dynamic flag to tell us that ... in essence 'we dont
support perfmon'. Which flag is again:

+struct pfm_pmu_config *pfm_pmu_conf;

... which should be __read_mostly at minimum.

+EXPORT_SYMBOL(pfm_pmu_conf);

and _MUST_ be exported as EXPORT_SYMBOL_GPL. If exported at all. Why
are any symbols exported here? perfmom does core kernel system calls
and is non-modular:

+config PERFMON
+ bool "Perfmon2 performance monitoring interface"

it needs _zero_ exports.

Ingo


\
 
 \ /
  Last update: 2008-11-26 15:11    [from the cache]
©2003-2010