2015-02-19 18:22:48

by David Ahern

[permalink] [raw]
Subject: [PATCH] perf: Only include tsc file for x86

perf_time_to_tsc and tsc_to_perf_time are only used for x86. Make
inclusion of tsc.c dependent on x86 as well.

Signed-off-by: David Ahern <[email protected]>
Cc: Adrian Hunter <[email protected]>
Cc: Jiri Olsa <[email protected]>
---
tools/perf/util/Build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 32f9327b1a97..4c7095785ba0 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -71,7 +71,7 @@ libperf-y += stat.o
libperf-y += record.o
libperf-y += srcline.o
libperf-y += data.o
-libperf-y += tsc.o
+libperf-$(CONFIG_X86) += tsc.o
libperf-y += cloexec.o
libperf-y += thread-stack.o

--
1.9.3


2015-02-19 20:18:21

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf: Only include tsc file for x86

On Thu, Feb 19, 2015 at 01:22:33PM -0500, David Ahern wrote:
> perf_time_to_tsc and tsc_to_perf_time are only used for x86. Make
> inclusion of tsc.c dependent on x86 as well.

hum, should we move it to arch/x86/util/tsc.c and remove util/tsc.c?

looks like it's used only by test tsc code, which is enabled
for x86 only anyway..

jirka

>
> Signed-off-by: David Ahern <[email protected]>
> Cc: Adrian Hunter <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> ---
> tools/perf/util/Build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index 32f9327b1a97..4c7095785ba0 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -71,7 +71,7 @@ libperf-y += stat.o
> libperf-y += record.o
> libperf-y += srcline.o
> libperf-y += data.o
> -libperf-y += tsc.o
> +libperf-$(CONFIG_X86) += tsc.o
> libperf-y += cloexec.o
> libperf-y += thread-stack.o
>
> --
> 1.9.3
>

2015-02-20 08:38:13

by Adrian Hunter

[permalink] [raw]
Subject: Re: [PATCH] perf: Only include tsc file for x86

On 19/02/15 21:42, Jiri Olsa wrote:
> On Thu, Feb 19, 2015 at 01:22:33PM -0500, David Ahern wrote:
>> perf_time_to_tsc and tsc_to_perf_time are only used for x86. Make
>> inclusion of tsc.c dependent on x86 as well.
>
> hum, should we move it to arch/x86/util/tsc.c and remove util/tsc.c?

It is not arch-specific because you can read a perf.data file
from one architecture on another architecture.

The TSC stuff was planned for Intel PT although it might now
be done differently.

Probably you should have CONFIG_X86_TSC and then select that
with CONFIG_X86. Later it can be selected with CONFIG_INTEL_PT
as well.

>
> looks like it's used only by test tsc code, which is enabled
> for x86 only anyway..
>
> jirka
>
>>
>> Signed-off-by: David Ahern <[email protected]>
>> Cc: Adrian Hunter <[email protected]>
>> Cc: Jiri Olsa <[email protected]>
>> ---
>> tools/perf/util/Build | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
>> index 32f9327b1a97..4c7095785ba0 100644
>> --- a/tools/perf/util/Build
>> +++ b/tools/perf/util/Build
>> @@ -71,7 +71,7 @@ libperf-y += stat.o
>> libperf-y += record.o
>> libperf-y += srcline.o
>> libperf-y += data.o
>> -libperf-y += tsc.o
>> +libperf-$(CONFIG_X86) += tsc.o
>> libperf-y += cloexec.o
>> libperf-y += thread-stack.o
>>
>> --
>> 1.9.3
>>
>
>

2015-02-20 16:41:12

by David Ahern

[permalink] [raw]
Subject: Re: [PATCH] perf: Only include tsc file for x86

On 2/20/15 1:36 AM, Adrian Hunter wrote:
> On 19/02/15 21:42, Jiri Olsa wrote:
>> On Thu, Feb 19, 2015 at 01:22:33PM -0500, David Ahern wrote:
>>> perf_time_to_tsc and tsc_to_perf_time are only used for x86. Make
>>> inclusion of tsc.c dependent on x86 as well.
>>
>> hum, should we move it to arch/x86/util/tsc.c and remove util/tsc.c?
>
> It is not arch-specific because you can read a perf.data file
> from one architecture on another architecture.
>
> The TSC stuff was planned for Intel PT although it might now
> be done differently.
>
> Probably you should have CONFIG_X86_TSC and then select that
> with CONFIG_X86. Later it can be selected with CONFIG_INTEL_PT
> as well.

For now let's leave it as CONFIG_X86. Right now only X86 code needs the
functions it provides and there is no need taking any other steps until
something is going to use it.

ie., patch is good as is. Agreed?

David

>
>>
>> looks like it's used only by test tsc code, which is enabled
>> for x86 only anyway..
>>
>> jirka
>>
>>>
>>> Signed-off-by: David Ahern <[email protected]>
>>> Cc: Adrian Hunter <[email protected]>
>>> Cc: Jiri Olsa <[email protected]>
>>> ---
>>> tools/perf/util/Build | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
>>> index 32f9327b1a97..4c7095785ba0 100644
>>> --- a/tools/perf/util/Build
>>> +++ b/tools/perf/util/Build
>>> @@ -71,7 +71,7 @@ libperf-y += stat.o
>>> libperf-y += record.o
>>> libperf-y += srcline.o
>>> libperf-y += data.o
>>> -libperf-y += tsc.o
>>> +libperf-$(CONFIG_X86) += tsc.o
>>> libperf-y += cloexec.o
>>> libperf-y += thread-stack.o
>>>
>>> --
>>> 1.9.3
>>>
>>
>>
>

2015-02-20 18:53:35

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH] perf: Only include tsc file for x86

On Fri, Feb 20, 2015 at 09:40:57AM -0700, David Ahern wrote:
> On 2/20/15 1:36 AM, Adrian Hunter wrote:
> >On 19/02/15 21:42, Jiri Olsa wrote:
> >>On Thu, Feb 19, 2015 at 01:22:33PM -0500, David Ahern wrote:
> >>>perf_time_to_tsc and tsc_to_perf_time are only used for x86. Make
> >>>inclusion of tsc.c dependent on x86 as well.
> >>
> >>hum, should we move it to arch/x86/util/tsc.c and remove util/tsc.c?
> >
> >It is not arch-specific because you can read a perf.data file
> >from one architecture on another architecture.
> >
> >The TSC stuff was planned for Intel PT although it might now
> >be done differently.
> >
> >Probably you should have CONFIG_X86_TSC and then select that
> >with CONFIG_X86. Later it can be selected with CONFIG_INTEL_PT
> >as well.
>
> For now let's leave it as CONFIG_X86. Right now only X86 code needs the
> functions it provides and there is no need taking any other steps until
> something is going to use it.
>
> ie., patch is good as is. Agreed?

ook, jirka