On Tue, May 28, 2019 at 03:06:16PM -0400, Liang, Kan wrote:
>
>
> On 5/28/2019 5:00 AM, Jiri Olsa wrote:
> > On Thu, May 23, 2019 at 01:41:19PM -0700, kan.li...@linux.intel.com wrote:
> >
> > SNIP
> >
> > > diff --git a/tools/perf/util/cputopo.c b/tools/perf/util/cputopo.c
> > > index ece0710..f6e7db7 100644
> > > --- a/tools/perf/util/cputopo.c
> > > +++ b/tools/perf/util/cputopo.c
> > > @@ -1,5 +1,6 @@
> > > // SPDX-License-Identifier: GPL-2.0
> > > #include <sys/param.h>
> > > +#include <sys/utsname.h>
> > > #include <inttypes.h>
> > > #include <api/fs/fs.h>
> > > @@ -8,9 +9,10 @@
> > > #include "util.h"
> > > #include "env.h"
> > > -
> > > #define CORE_SIB_FMT \
> > > "%s/devices/system/cpu/cpu%d/topology/core_siblings_list"
> > > +#define DIE_SIB_FMT \
> > > + "%s/devices/system/cpu/cpu%d/topology/die_cpus_list"
> > > #define THRD_SIB_FMT \
> > > "%s/devices/system/cpu/cpu%d/topology/thread_siblings_list"
> > > #define NODE_ONLINE_FMT \
> > > @@ -20,7 +22,26 @@
> > > #define NODE_CPULIST_FMT \
> > > "%s/devices/system/node/node%d/cpulist"
> > > -static int build_cpu_topology(struct cpu_topology *tp, int cpu)
> > > +bool check_x86_die_exists(void)
> > > +{
> > > + char filename[MAXPATHLEN];
> > > + struct utsname uts;
> > > +
> > > + if (uname(&uts) < 0)
> > > + return false;
> > > +
> > > + if (strncmp(uts.machine, "x86_64", 6))
> > > + return false;
> > > +
> > > + scnprintf(filename, MAXPATHLEN, DIE_SIB_FMT,
> > > + sysfs__mountpoint(), 0);
> > > + if (access(filename, F_OK) == -1)
> > > + return false;
> > > +
> > > + return true;
> > > +}
> >
> > we could rename this to:
> >
> > static bool has_die(void)
> > {
> > static bool has_die;
> >
> > if (initialized)
> > return has_die;
> >
> > has_die = ...
> > initialized = true;
> > }
> >
> > and got rid of all those 'has_die' arguments below
>
> Yes, we can rename the function to has_die(). It looks like all the
> 'has_die' arguments can be replaced either.
>
> But why we want a "initialized" here? to cache the value? It looks like we
> only need to call has_die() once.
right, if it's called from one place then it's ok
thanks,
jirka
