Messages in this thread | | | Subject | Re: [PATCH 3/4] perf tool: Fix build when sysconf doesn't support cache line size | From | Chris Phlipot <> | Date | Mon, 4 Jul 2016 17:47:12 -0700 |
| |
On 07/04/2016 05:26 PM, Arnaldo Carvalho de Melo wrote: > Em Mon, Jul 04, 2016 at 05:19:20PM -0700, Chris Phlipot escreveu: >> >> >> On 07/04/2016 03:48 PM, Arnaldo Carvalho de Melo wrote: >>> Em Thu, Jun 30, 2016 at 10:12:34PM -0700, Chris Phlipot escreveu: >>>> Enable perf to build on libc implementations where sysconf() doesn't >>>> support _SC_LEVEL1_DCACHE_LINESIZE as a parameter. >>>> >>>> For example, the Bionic implementation does not support this as a >>>> paremter. Older versions of Bionic will throw an error when this is passed >>>> in as a parameter, and more recent versions will just return 0 as the >>>> cache line size. >>>> >>>> Signed-off-by: Chris Phlipot <cphlipot0@gmail.com> >>>> --- >>>> tools/perf/perf.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/tools/perf/perf.c b/tools/perf/perf.c >>>> index 8f21922..113ca5b 100644 >>>> --- a/tools/perf/perf.c >>>> +++ b/tools/perf/perf.c >>>> @@ -509,7 +509,11 @@ int main(int argc, const char **argv) >>>> >>>> /* The page_size is placed in util object. */ >>>> page_size = sysconf(_SC_PAGE_SIZE); >>>> +#ifdef _SC_LEVEL1_DCACHE_LINESIZE >>>> cacheline_size = sysconf(_SC_LEVEL1_DCACHE_LINESIZE); >>>> +#else >>>> + cacheline_size = 0; >>>> +#endif >>> >>> Couldn't we instead fallback to: >>> >>> sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size", &cacheline_size) >>> >>> ? >> >> I agree that in general this would be a better fallback, but in all Android >> images I have tested so far, "devices/system/cpu/cpu0/cache" does not exist. >> I know not know of a good way to retrieve cache line size in this case. >> >> I would be ok with attempting to get cacheline size using using the >> following methods, unless you have other ideas: >> >> 1. attempt to use sysconf(_SC_LEVEL1_DCACHE_LINESIZE) >> 2. attempt to use >> sysfs__read_int("devices/system/cpu/cpu0/cache/index0/coherency_line_size", >> &cacheline_size) >> 3. set to zero if both of the above fail. > > Ok, but perhaps we should have some sort of warning in places using > this? > > - Arnaldo
Such as printing a warning when cacheline_size is set to zero, or simply adding comments to the code in areas where cacheline_size is used?
-Chris
> >> >>> >>>> if (sysctl__read_int("kernel/perf_event_max_stack", &value) == 0) >>>> sysctl_perf_event_max_stack = value; >>>> -- >>>> 2.7.4
| |