Re: [PATCH 05/59] tools api fs: Move in the fncache from perf

[Jiri Olsa <jolsa@redhat.com>]

On Mon, Nov 08, 2021 at 09:46:43AM -0800, Ian Rogers wrote:
> On Mon, Nov 8, 2021 at 5:37 AM Jiri Olsa <jolsa@redhat.com> wrote:
> >
> > Moving fncache from perf under libapi.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > ---
> >  tools/lib/api/fs/Build             |  1 +
> >  tools/lib/api/fs/fncache.c         | 63 ++++++++++++++++++++++++++++++
> >  tools/lib/api/fs/fs.h              |  2 +
> >  tools/perf/arch/x86/util/pmu.c     |  1 -
> >  tools/perf/tests/parse-events.c    |  1 -
> >  tools/perf/util/Build              |  1 -
> >  tools/perf/util/fncache.c          | 63 ------------------------------
> >  tools/perf/util/fncache.h          |  7 ----
> 
> This change looks like it could be independent. Could this code be
> simplified to just using a hashmap now that we have one?

true, Sohaib, perhaps you might be interest in this?

jirka

> 
> Thanks,
> Ian
> 
> >  tools/perf/util/pmu-hybrid.c       |  1 -
> >  tools/perf/util/pmu.c              |  1 -
> >  tools/perf/util/python-ext-sources |  1 -
> >  tools/perf/util/srccode.c          | 10 ++++-
> >  12 files changed, 75 insertions(+), 77 deletions(-)
> >  create mode 100644 tools/lib/api/fs/fncache.c
> >  delete mode 100644 tools/perf/util/fncache.c
> >  delete mode 100644 tools/perf/util/fncache.h
> >
> > diff --git a/tools/lib/api/fs/Build b/tools/lib/api/fs/Build
> > index 0f75b28654de..3af4b5265a5f 100644
> > --- a/tools/lib/api/fs/Build
> > +++ b/tools/lib/api/fs/Build
> > @@ -1,3 +1,4 @@
> >  libapi-y += fs.o
> >  libapi-y += tracing_path.o
> >  libapi-y += cgroup.o
> > +libapi-y += fncache.o
> > diff --git a/tools/lib/api/fs/fncache.c b/tools/lib/api/fs/fncache.c
> > new file mode 100644
> > index 000000000000..7fb4586c341a
> > --- /dev/null
> > +++ b/tools/lib/api/fs/fncache.c
> > @@ -0,0 +1,63 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/* Manage a cache of file names' existence */
> > +#include <stdlib.h>
> > +#include <unistd.h>
> > +#include <string.h>
> > +#include <linux/list.h>
> > +#include "fs.h"
> > +
> > +struct fncache {
> > +       struct hlist_node nd;
> > +       bool res;
> > +       char name[];
> > +};
> > +
> > +#define FNHSIZE 61
> > +
> > +static struct hlist_head fncache_hash[FNHSIZE];
> > +
> > +static unsigned shash(const unsigned char *s)
> > +{
> > +       unsigned h = 0;
> > +       while (*s)
> > +               h = 65599 * h + *s++;
> > +       return h ^ (h >> 16);
> > +}
> > +
> > +static bool lookup_fncache(const char *name, bool *res)
> > +{
> > +       int h = shash((const unsigned char *)name) % FNHSIZE;
> > +       struct fncache *n;
> > +
> > +       hlist_for_each_entry(n, &fncache_hash[h], nd) {
> > +               if (!strcmp(n->name, name)) {
> > +                       *res = n->res;
> > +                       return true;
> > +               }
> > +       }
> > +       return false;
> > +}
> > +
> > +static void update_fncache(const char *name, bool res)
> > +{
> > +       struct fncache *n = malloc(sizeof(struct fncache) + strlen(name) + 1);
> > +       int h = shash((const unsigned char *)name) % FNHSIZE;
> > +
> > +       if (!n)
> > +               return;
> > +       strcpy(n->name, name);
> > +       n->res = res;
> > +       hlist_add_head(&n->nd, &fncache_hash[h]);
> > +}
> > +
> > +/* No LRU, only use when bounded in some other way. */
> > +bool file_available(const char *name)
> > +{
> > +       bool res;
> > +
> > +       if (lookup_fncache(name, &res))
> > +               return res;
> > +       res = access(name, R_OK) == 0;
> > +       update_fncache(name, res);
> > +       return res;
> > +}
> > diff --git a/tools/lib/api/fs/fs.h b/tools/lib/api/fs/fs.h
> > index aa222ca30311..c42d4ff30ca7 100644
> > --- a/tools/lib/api/fs/fs.h
> > +++ b/tools/lib/api/fs/fs.h
> > @@ -59,4 +59,6 @@ int sysfs__read_str(const char *entry, char **buf, size_t *sizep);
> >  int sysfs__read_bool(const char *entry, bool *value);
> >
> >  int sysfs__write_int(const char *entry, int value);
> > +
> > +bool file_available(const char *name);
> >  #endif /* __API_FS__ */
> > diff --git a/tools/perf/arch/x86/util/pmu.c b/tools/perf/arch/x86/util/pmu.c
> > index 74d69db1ea99..c875dded65bb 100644
> > --- a/tools/perf/arch/x86/util/pmu.c
> > +++ b/tools/perf/arch/x86/util/pmu.c
> > @@ -13,7 +13,6 @@
> >  #include "../../../util/intel-pt.h"
> >  #include "../../../util/intel-bts.h"
> >  #include "../../../util/pmu.h"
> > -#include "../../../util/fncache.h"
> >
> >  #define TEMPLATE_ALIAS "%s/bus/event_source/devices/%s/alias"
> >
> > diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> > index 8875e388563e..69381fe1655d 100644
> > --- a/tools/perf/tests/parse-events.c
> > +++ b/tools/perf/tests/parse-events.c
> > @@ -9,7 +9,6 @@
> >  #include "pmu-hybrid.h"
> >  #include <dirent.h>
> >  #include <errno.h>
> > -#include "fncache.h"
> >  #include <sys/types.h>
> >  #include <sys/stat.h>
> >  #include <unistd.h>
> > diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> > index 03d5d6ed7fe4..b93828aacc27 100644
> > --- a/tools/perf/util/Build
> > +++ b/tools/perf/util/Build
> > @@ -52,7 +52,6 @@ perf-y += header.o
> >  perf-y += callchain.o
> >  perf-y += values.o
> >  perf-y += debug.o
> > -perf-y += fncache.o
> >  perf-y += machine.o
> >  perf-y += map.o
> >  perf-y += pstack.o
> > diff --git a/tools/perf/util/fncache.c b/tools/perf/util/fncache.c
> > deleted file mode 100644
> > index 6225cbc52310..000000000000
> > --- a/tools/perf/util/fncache.c
> > +++ /dev/null
> > @@ -1,63 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0-only
> > -/* Manage a cache of file names' existence */
> > -#include <stdlib.h>
> > -#include <unistd.h>
> > -#include <string.h>
> > -#include <linux/list.h>
> > -#include "fncache.h"
> > -
> > -struct fncache {
> > -       struct hlist_node nd;
> > -       bool res;
> > -       char name[];
> > -};
> > -
> > -#define FNHSIZE 61
> > -
> > -static struct hlist_head fncache_hash[FNHSIZE];
> > -
> > -unsigned shash(const unsigned char *s)
> > -{
> > -       unsigned h = 0;
> > -       while (*s)
> > -               h = 65599 * h + *s++;
> > -       return h ^ (h >> 16);
> > -}
> > -
> > -static bool lookup_fncache(const char *name, bool *res)
> > -{
> > -       int h = shash((const unsigned char *)name) % FNHSIZE;
> > -       struct fncache *n;
> > -
> > -       hlist_for_each_entry(n, &fncache_hash[h], nd) {
> > -               if (!strcmp(n->name, name)) {
> > -                       *res = n->res;
> > -                       return true;
> > -               }
> > -       }
> > -       return false;
> > -}
> > -
> > -static void update_fncache(const char *name, bool res)
> > -{
> > -       struct fncache *n = malloc(sizeof(struct fncache) + strlen(name) + 1);
> > -       int h = shash((const unsigned char *)name) % FNHSIZE;
> > -
> > -       if (!n)
> > -               return;
> > -       strcpy(n->name, name);
> > -       n->res = res;
> > -       hlist_add_head(&n->nd, &fncache_hash[h]);
> > -}
> > -
> > -/* No LRU, only use when bounded in some other way. */
> > -bool file_available(const char *name)
> > -{
> > -       bool res;
> > -
> > -       if (lookup_fncache(name, &res))
> > -               return res;
> > -       res = access(name, R_OK) == 0;
> > -       update_fncache(name, res);
> > -       return res;
> > -}
> > diff --git a/tools/perf/util/fncache.h b/tools/perf/util/fncache.h
> > deleted file mode 100644
> > index fe020beaefb1..000000000000
> > --- a/tools/perf/util/fncache.h
> > +++ /dev/null
> > @@ -1,7 +0,0 @@
> > -#ifndef _FCACHE_H
> > -#define _FCACHE_H 1
> > -
> > -unsigned shash(const unsigned char *s);
> > -bool file_available(const char *name);
> > -
> > -#endif
> > diff --git a/tools/perf/util/pmu-hybrid.c b/tools/perf/util/pmu-hybrid.c
> > index f51ccaac60ee..65fdce81a384 100644
> > --- a/tools/perf/util/pmu-hybrid.c
> > +++ b/tools/perf/util/pmu-hybrid.c
> > @@ -13,7 +13,6 @@
> >  #include <stdarg.h>
> >  #include <locale.h>
> >  #include <api/fs/fs.h>
> > -#include "fncache.h"
> >  #include "pmu-hybrid.h"
> >
> >  LIST_HEAD(perf_pmu__hybrid_pmus);
> > diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> > index 55d834160428..13e1835955e0 100644
> > --- a/tools/perf/util/pmu.c
> > +++ b/tools/perf/util/pmu.c
> > @@ -26,7 +26,6 @@
> >  #include "header.h"
> >  #include "string2.h"
> >  #include "strbuf.h"
> > -#include "fncache.h"
> >  #include "pmu-hybrid.h"
> >
> >  struct perf_pmu perf_pmu__fake;
> > diff --git a/tools/perf/util/python-ext-sources b/tools/perf/util/python-ext-sources
> > index a685d20165f7..992a76c2af01 100644
> > --- a/tools/perf/util/python-ext-sources
> > +++ b/tools/perf/util/python-ext-sources
> > @@ -39,4 +39,3 @@ util/affinity.c
> >  util/rwsem.c
> >  util/hashmap.c
> >  util/pmu-hybrid.c
> > -util/fncache.c
> > diff --git a/tools/perf/util/srccode.c b/tools/perf/util/srccode.c
> > index 476e99896d5e..5c1ff87eb98c 100644
> > --- a/tools/perf/util/srccode.c
> > +++ b/tools/perf/util/srccode.c
> > @@ -16,7 +16,7 @@
> >  #include "srccode.h"
> >  #include "debug.h"
> >  #include <internal/lib.h> // page_size
> > -#include "fncache.h"
> > +#include <api/fs/fs.h>
> >
> >  #define MAXSRCCACHE (32*1024*1024)
> >  #define MAXSRCFILES     64
> > @@ -86,6 +86,14 @@ static void free_srcfile(struct srcfile *sf)
> >         num_srcfiles--;
> >  }
> >
> > +static unsigned shash(const unsigned char *s)
> > +{
> > +       unsigned h = 0;
> > +       while (*s)
> > +               h = 65599 * h + *s++;
> > +       return h ^ (h >> 16);
> > +}
> > +
> >  static struct srcfile *find_srcfile(char *fn)
> >  {
> >         struct stat st;
> > --
> > 2.31.1
> >
>