Re: [PATCH 6/6] sh: oprofile: Use perf-events oprofile backend

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27.09.10 16:01:38, Matt Fleming wrote:
> On Thu, Sep 16, 2010 at 04:32:54PM +0200, Robert Richter wrote:
> > > diff --git a/arch/sh/kernel/perf_event.c b/arch/sh/kernel/perf_event.c
> > > index 2cb9ad5..3c3fc9a 100644
> > > --- a/arch/sh/kernel/perf_event.c
> > > +++ b/arch/sh/kernel/perf_event.c
> > > @@ -59,6 +59,14 @@ static inline int sh_pmu_initialized(void)
> > >  	return !!sh_pmu;
> > >  }
> > >  
> > > +const char *sh_pmu_name(void)
> > > +{
> > > +	if (!sh_pmu)
> > > +		return NULL;
> > > +
> > > +	return sh_pmu->name;
> > > +}
> > 
> > Couldn't we make this a generic function like perf_num_counters()?
> 
> Well, ARM doesn't have names as strings for its pmus currently. What's
> more, ARM wouldn't use it; SH would be the only user of this function. I
> don't think this one makes sense to be a generic function.

I didn't catch this with my first review, the function will need an
EXPORT_SYMBOL_GPL() to allow building modules. This will mean an
interface extension what should be non-arch. So, for architectures we
need the pmu name like SH we just implement the generic function. For
ARM we don't need to provide this function.

Most of the interface is defined in linux/perf_event.h. We shouldn't
move this to asm/perf_event.h, so this is one more argument for the
non-arch implementation.

As the implementation of the function would be optional, why should we
make it architectural?

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center

--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel]     [Kernel Newbies]     [x86 Platform Driver]     [Netdev]     [Linux Wireless]     [Netfilter]     [Bugtraq]     [Linux Filesystems]     [Yosemite Discussion]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux