reduce sysfs path lookup overhead
authorNathan Lynch <ntl@pobox.com>
Wed, 22 Oct 2008 00:26:49 +0000 (19:26 -0500)
committerNathan Lynch <ntl@pobox.com>
Wed, 22 Oct 2008 00:26:49 +0000 (19:26 -0500)
Rely more heavily on relative paths and openat(2) to avoid the lookup
overhead involved in opening paths such as
"/sys/devices/system/cpu/cpu42/topology/thread_siblings" in the
context initialization code.  This can hurt on large systems.  With
these changes, elapsed time for context initialization is reduced by
20 to 25% from v0.3 (measured on a mocked-up 1024-cpu sysfs
hierarchy).

lib/topology.c

index d8217054cf5089ed4e73dd68e2aca473098421f5..f91216444da8c390cd7be426de5ba606d5790219 100644 (file)
 /***************************/
 /* Misc. utility functions */
 /***************************/
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+
 static void *zmalloc(size_t size)
 {
        return calloc(1, size);
 }
 
+static int util_openat_ro(int dir_fd, const char *path)
+{
+       return openat(dir_fd, path, O_RDONLY | O_CLOEXEC);
+}
+
+static DIR *util_opendirat(int dir_fd, const char *path)
+{
+       int fd;
+
+       fd = util_openat_ro(dir_fd, path);
+       if (fd == -1)
+               return NULL;
+
+       return fdopendir(fd);
+}
+
 /*******************/
 /* cpumask parsing */
 /*******************/
@@ -229,7 +248,6 @@ struct topo_device {
  */
 struct topo_procent {
        int level;
-       int id;
        char *hash_key; /* for cores and packages */
        struct topo_context *context;
        struct topo_procent *parent;
@@ -531,7 +549,7 @@ static void topo_device_copy_cpumask(const struct topo_device *dev,
 /*****************************************************/
 
 static struct topo_procent *
-alloc_init_topo_procent(struct topo_context *ctx, struct topo_procent *parent, int id)
+alloc_init_topo_procent(struct topo_context *ctx, struct topo_procent *parent)
 {
        struct topo_procent *ent;
        enum topology_level level = TOPOLOGY_SYSTEM;
@@ -554,7 +572,6 @@ alloc_init_topo_procent(struct topo_context *ctx, struct topo_procent *parent, i
 
        ent->context = ctx;
        ent->level = level;
-       ent->id = id;
 
        /* Insert in global list */
        ent->next = ctx->list;
@@ -654,7 +671,7 @@ static int sysfs_open_vargs(struct topo_context *ctx, const char *fmt, va_list v
 
        vsnprintf(path, sizeof(path), fmt, vargs);
 
-       path_fd = openat(sysfs_root_fd, path, O_RDONLY | O_CLOEXEC);
+       path_fd = util_openat_ro(sysfs_root_fd, path);
 
        return path_fd;
 }
@@ -681,60 +698,6 @@ static DIR *sysfs_opendir(struct topo_context *ctx, const char *fmt, ...)
        return dir;
 }
 
-static char *sysfs_file_getline(struct topo_context *ctx, const char *fmt, ...)
-       __attribute__((format(printf, 2, 3)));
-static char *sysfs_file_getline(struct topo_context *ctx, const char *fmt, ...)
-{
-       char *buf = NULL;
-       va_list vargs;
-       size_t size;
-       FILE *file;
-       int fd;
-
-       va_start(vargs, fmt);
-       fd = sysfs_open_vargs(ctx, fmt, vargs);
-       va_end(vargs);
-
-       if (fd == -1)
-               return NULL;
-
-       file = fdopen(fd, "r");
-       if (!file)
-               goto out;
-       fd = -1;
-
-       getline(&buf, &size, file);
-       buf_remove_newline(buf);
-
-       fclose(file);
-out:
-       if (fd != -1)
-               close(fd);
-       return buf;
-}
-
-/* Return true if:
- * - /sys/devices/system/cpu/cpu$cpu/online exists and has value 1, or
- * - /sys/devices/system/cpu/cpu$cpu/online does not exist (e.g. x86 boot cpu,
- *   or kernel does not support cpu hotplug)
- * Return false if /sys/devices/system/cpu/cpu$cpu/online has value 0
- */
-static bool sysfs_cpu_is_online(struct topo_context *ctx, unsigned int cpu)
-{
-       int online = true;
-       char *buf;
-
-       buf = sysfs_file_getline(ctx, "devices/system/cpu/cpu%d/online", cpu);
-       if (!buf)
-               goto out;
-
-       if (!strcmp("0", buf))
-               online = false;
-out:
-       free(buf);
-       return online;
-}
-
 static size_t sysfs_probe_cpumask_size(struct topo_context *ctx)
 {
        DIR *dir;
@@ -844,33 +807,6 @@ err:
        return 0;
 }
 
-static int sysfs_count_caches(struct topo_context *ctx, int cpu)
-{
-       struct dirent64 *dirent;
-       int count = 0;
-       DIR *dir;
-
-       dir = sysfs_opendir(ctx, "devices/system/cpu/cpu%d/cache", cpu);
-       if (!dir)
-               goto out;
-
-       while ((dirent = readdir64(dir))) {
-               int index;
-
-               if (dirent->d_type != DT_DIR && dirent->d_type != DT_UNKNOWN)
-                       continue;
-
-               if (sscanf(dirent->d_name, "index%d", &index) != 1)
-                       continue;
-
-               count++;
-       }
-
-       closedir(dir);
-out:
-       return count;
-}
-
 static char *sysfs_get_attr_value(int dirfd, const char *attr_path)
 {
        char *line = NULL;
@@ -879,7 +815,7 @@ static char *sysfs_get_attr_value(int dirfd, const char *attr_path)
        int fd;
        int rc;
 
-       fd = openat(dirfd, attr_path, O_CLOEXEC | O_RDONLY);
+       fd = util_openat_ro(dirfd, attr_path);
        if (fd == -1)
                goto err;
 
@@ -928,62 +864,34 @@ err:
        return NULL;
 }
 
-static int __get_cache_info(struct topo_device *cache, int cpu, int index)
+static int cache_collect_attrs(struct topo_device *cache, DIR *cachedir)
 {
+       static const char *required_attrs[] = {
+               "level", "size", "type", "shared_cpu_map",
+       };
        size_t cpumask_size;
        struct attr *attr;
-       DIR *dir;
        int fd;
+       int i;
 
        cpumask_size = cache->context->cpu_set_t_size;
 
-       dir = sysfs_opendir(cache->context,
-                           "devices/system/cpu/cpu%d/cache/index%d",
-                           cpu, index);
-       if (!dir)
-               goto err;
-
-       fd = dirfd(dir);
+       fd = dirfd(cachedir);
        if (fd == -1)
                goto err;
 
-       attr = sysfs_get_attr(fd, "size");
-       if (!attr)
-               goto err;
-
-       topo_device_attach_attr(cache, attr);
-
-       attr = sysfs_get_attr(fd, "type");
-       if (!attr)
-               goto err;
-
-       topo_device_attach_attr(cache, attr);
-
-       attr = sysfs_get_attr(fd, "level");
-       if (!attr)
-               goto err;
-
-       topo_device_attach_attr(cache, attr);
-
-       attr = sysfs_get_attr(fd, "shared_cpu_map");
-       if (!attr)
-               goto err;
-
-       topo_device_attach_attr(cache, attr);
-
-       if (cpumask_parse(cache->cpumask, cpumask_size, attr->value))
-               goto err;
+       for (i = 0; i < ARRAY_SIZE(required_attrs); i++) {
+               const char *name;
 
-       /* Ensure this cpu is set in the shared map */
-       if (!CPU_ISSET_S(cpu, cpumask_size, cache->cpumask))
-               goto err;
-
-       closedir(dir);
+               name = required_attrs[i];
+               attr = sysfs_get_attr(fd, name);
+               if (!attr)
+                       goto err;
+               topo_device_attach_attr(cache, attr);
+       }
 
        return 0;
 err:
-       if (dir)
-               closedir(dir);
        return 1;
 }
 
@@ -1014,62 +922,90 @@ err:
        return 1;
 }
 
-static int get_cache_info(struct topo_procent *thread)
+static void do_one_cache(struct topo_context *ctx, DIR *cachedir)
 {
-       struct topo_context *ctx;
        struct topo_device *cache;
-       int nr_caches;
-       int index;
+       const char *cpumask_buf;
+
+       cache = alloc_topo_device(ctx, "cache");
+       if (!cache)
+               return;
 
-       ctx = thread->context;
+       /* get attributes */
+       if (cache_collect_attrs(cache, cachedir))
+               goto err;
+
+       /* parse the shared_cpu_map attr into this cache's cpumask */
+       cpumask_buf = topo_device_get_attr_value(cache, "shared_cpu_map");
+       if (!cpumask_buf)
+               goto err;
 
-       nr_caches = sysfs_count_caches(ctx, thread->id);
-       if (nr_caches == 0)
+       if (cpumask_parse(cache->cpumask, ctx->cpu_set_t_size, cpumask_buf))
+               goto err;
+
+       /* construct hash key */
+       if (cache_build_hash(cache))
+               goto err;
+
+       /* have we seen this cache before? */
+       if (htable_lookup(&ctx->device_htable, cache->hash_key))
+               goto err;
+
+       /* add to hash table */
+       if (hash_device(ctx, cache))
+               goto err;
+
+       /* register in context device list */
+       topo_device_register(ctx, cache);
+
+       return;
+err:
+       topo_device_release(cache);
+       return;
+}
+
+static int get_cache_info(struct topo_context *ctx, DIR *cpudir)
+{
+       struct dirent64 *dirent;
+       DIR *cachedir;
+
+       cachedir = util_opendirat(dirfd(cpudir), "cache");
+       if (!cachedir)
                return 0;
 
-       /* Need to keep track of which caches have been seen.  We can
-        * uniquely identify a cache by its level, type, and
-        * shared_cpu_map, so hash that combination.
-        */
+       while ((dirent = readdir64(cachedir))) {
+               DIR *indexdir;
+               int index;
 
-       for (index = 0; index < nr_caches; index++) {
-               cache = alloc_topo_device(ctx, "cache");
-               if (!cache)
-                       goto err;
-               if (__get_cache_info(cache, thread->id, index))
-                       goto err;
-               if (cache_build_hash(cache))
-                       goto err;
+               if (sscanf(dirent->d_name, "index%d", &index) != 1)
+                       continue;
 
-               /* If we've seen this cache already, release it */
-               if (htable_lookup(&ctx->device_htable, cache->hash_key)) {
-                       topo_device_release(cache);
+               indexdir = util_opendirat(dirfd(cachedir), dirent->d_name);
+               if (!cachedir)
                        continue;
-               }
 
-               if (hash_device(ctx, cache))
-                       goto err;
+               do_one_cache(ctx, indexdir);
 
-               topo_device_register(ctx, cache);
+               closedir(indexdir);
        }
 
-       return 0;
+       closedir(cachedir);
 
-err:
-       topo_device_release(cache);
-       return 1;
+       return 0;
 }
 
-static char *sysfs_cpu_thread_siblings(struct topo_context *ctx, int cpu_id)
+static char *sysfs_cpu_thread_siblings(struct topo_context *ctx,
+                                      DIR *cpudir, int cpu_id)
 {
-       char *fmt = "devices/system/cpu/cpu%d/topology/thread_siblings";
        char *buf;
        int rc;
 
-       buf = sysfs_file_getline(ctx, fmt, cpu_id);
+       buf = sysfs_get_attr_value(dirfd(cpudir), "topology/thread_siblings");
        if (buf)
                return buf;
 
+       /* cpu_id must be a subset of thread_siblings so assume one
+        * thread per core */
        rc = asprintf(&buf, "%d", cpu_id);
        if (rc == -1)
                buf = NULL;
@@ -1077,21 +1013,43 @@ static char *sysfs_cpu_thread_siblings(struct topo_context *ctx, int cpu_id)
        return buf;
 }
 
-static char *sysfs_cpu_core_siblings(struct topo_context *ctx, int cpu_id)
+static char *sysfs_cpu_core_siblings(struct topo_context *ctx,
+                                    DIR *cpudir, int cpu_id)
 {
-       char *fmt = "devices/system/cpu/cpu%d/topology/core_siblings";
        char *buf;
 
-       buf = sysfs_file_getline(ctx, fmt, cpu_id);
+       buf = sysfs_get_attr_value(dirfd(cpudir), "topology/core_siblings");
        if (buf)
                return buf;
 
        /* thread_siblings must be a subset of core_siblings so assume
         * one core per package */
-       return sysfs_cpu_thread_siblings(ctx, cpu_id);
+       return sysfs_cpu_thread_siblings(ctx, cpudir, cpu_id);
 }
 
-static int do_one_cpu(struct topo_procent *node, int cpu_id)
+/* Return true if:
+ * - /sys/devices/system/cpu/cpu$cpu/online exists and has value 1, or
+ * - /sys/devices/system/cpu/cpu$cpu/online does not exist (e.g. x86 boot cpu,
+ *   or kernel does not support cpu hotplug)
+ * Return false if /sys/devices/system/cpu/cpu$cpu/online has value 0
+ */
+static bool sysfs_cpu_is_online(struct topo_context *ctx, DIR *cpudir)
+{
+       int online = true;
+       char *buf;
+
+       buf = sysfs_get_attr_value(dirfd(cpudir), "online");
+       if (!buf)
+               goto out;
+
+       if (!strcmp("0", buf))
+               online = false;
+out:
+       free(buf);
+       return online;
+}
+
+static int do_one_cpu(struct topo_procent *node, DIR *dir, int cpu_id)
 {
        struct topo_context *ctx;
        struct topo_procent *pkg;
@@ -1101,8 +1059,11 @@ static int do_one_cpu(struct topo_procent *node, int cpu_id)
 
        ctx = node->context;
 
+       if (!sysfs_cpu_is_online(ctx, dir))
+               return 0;
+
        /* package */
-       siblings = sysfs_cpu_core_siblings(ctx, cpu_id);
+       siblings = sysfs_cpu_core_siblings(ctx, dir, cpu_id);
        if (!siblings)
                goto err;
 
@@ -1110,7 +1071,7 @@ static int do_one_cpu(struct topo_procent *node, int cpu_id)
        if (!pkg) {
                int rc;
 
-               pkg = alloc_init_topo_procent(ctx, node, cpu_id);
+               pkg = alloc_init_topo_procent(ctx, node);
                if (!pkg)
                        goto err;
 
@@ -1126,7 +1087,7 @@ static int do_one_cpu(struct topo_procent *node, int cpu_id)
        free(siblings);
 
        /* core */
-       siblings = sysfs_cpu_thread_siblings(ctx, cpu_id);
+       siblings = sysfs_cpu_thread_siblings(ctx, dir, cpu_id);
        if (!siblings)
                goto err;
 
@@ -1134,7 +1095,7 @@ static int do_one_cpu(struct topo_procent *node, int cpu_id)
        if (!core) {
                int rc;
 
-               core = alloc_init_topo_procent(ctx, pkg, cpu_id);
+               core = alloc_init_topo_procent(ctx, pkg);
                if (!core)
                        goto err;
 
@@ -1150,14 +1111,14 @@ static int do_one_cpu(struct topo_procent *node, int cpu_id)
        free(siblings);
        siblings = NULL;
 
-       thread = alloc_init_topo_procent(ctx, core, cpu_id);
+       thread = alloc_init_topo_procent(ctx, core);
        if (!thread)
                goto err;
 
        topo_procent_cpumask_set(thread, cpu_id);
 
        /* Collecting cache info is best-effort */
-       get_cache_info(thread);
+       get_cache_info(ctx, dir);
 
        return 0;
 err:
@@ -1165,54 +1126,57 @@ err:
        return 1;
 }
 
-static int do_node_cpus(struct topo_procent *node)
+static int do_node_cpus(struct topo_procent *node, DIR *dir)
 {
        struct topo_context *ctx;
        struct dirent64 *dirent;
-       DIR *dir;
+       DIR *cpus_dir;
 
        ctx = node->context;
 
-       /* If we're "faking" node 0, use the cpu sysfs hierarchy */
-       dir = sysfs_opendir(ctx, "devices/system/node/node%d", node->id);
-       if (!dir && node->id != 0) {
-               goto err;
-       } else if (!dir) {
-               dir = sysfs_opendir(ctx, "devices/system/cpu");
-               if (!dir)
-                       goto err;
-       }
+       cpus_dir = sysfs_opendir(ctx, "devices/system/cpu");
+       if (!cpus_dir)
+               return 1;
 
        while ((dirent = readdir64(dir))) {
+               DIR *cpu_dir;
                int cpu_id;
+               int rc;
 
                if (sscanf(dirent->d_name, "cpu%d", &cpu_id) != 1)
                        continue;
 
-               if (!sysfs_cpu_is_online(ctx, cpu_id))
-                       continue;
+               /* On NUMA systems dirent is a symlink; don't follow it:
+                * 1) It's against the rules (sysfs-rules.txt).
+                * 2) It's slower than opening the canonical cpu directory.
+                */
+               cpu_dir = util_opendirat(dirfd(cpus_dir), dirent->d_name);
+               if (!cpu_dir)
+                       goto err;
 
-               if (do_one_cpu(node, cpu_id))
+               rc = do_one_cpu(node, cpu_dir, cpu_id);
+               closedir(cpu_dir);
+
+               if (rc)
                        goto err;
        }
 
-       closedir(dir);
+       closedir(cpus_dir);
        return 0;
 err:
-       if (dir)
-               closedir(dir);
+       closedir(cpus_dir);
        return 1;
 }
 
-static int do_one_node(struct topo_procent *sys, int nid)
+static int do_one_node(struct topo_procent *sys, DIR *dir)
 {
        struct topo_procent *node;
 
-       node = alloc_init_topo_procent(sys->context, sys, nid);
+       node = alloc_init_topo_procent(sys->context, sys);
        if (!node)
                goto err;
 
-       if (do_node_cpus(node))
+       if (do_node_cpus(node, dir))
                goto err;
 
        return 0;
@@ -1220,41 +1184,63 @@ err:
        return 1;
 }
 
+static int build_context_nonnuma(struct topo_context *ctx,
+                                struct topo_procent *system)
+{
+       DIR *cpus_dir;
+       int rc = 1;
+
+       cpus_dir = sysfs_opendir(ctx, "devices/system/cpu");
+       if (!cpus_dir)
+               goto out;
+
+       rc = do_one_node(system, cpus_dir);
+       closedir(cpus_dir);
+out:
+       return rc;
+}
+
 static int build_context(struct topo_context *ctx)
 {
        struct topo_procent *system;
        struct dirent64 *dirent;
-       DIR *dir = NULL;
+       DIR *nodes_dir = NULL;
 
-       system = alloc_init_topo_procent(ctx, NULL, 0);
+       system = alloc_init_topo_procent(ctx, NULL);
        if (!system)
                goto err;
 
        ctx->system = system;
 
-       dir = sysfs_opendir(ctx, "devices/system/node");
-       if (!dir) /* Non-numa system, treat as single node */
-               return do_one_node(system, 0);
+       nodes_dir = sysfs_opendir(ctx, "devices/system/node");
+       if (!nodes_dir)
+               return build_context_nonnuma(ctx, system);
 
-       while ((dirent = readdir64(dir))) {
+       while ((dirent = readdir64(nodes_dir))) {
+               DIR *node_dir;
                int node_id;
-
-               if (dirent->d_type != DT_DIR && dirent->d_type != DT_UNKNOWN)
-                       continue;
+               int rc;
 
                if (sscanf(dirent->d_name, "node%d", &node_id) != 1)
                        continue;
 
-               if (do_one_node(system, node_id))
+               node_dir = util_opendirat(dirfd(nodes_dir), dirent->d_name);
+               if (!node_dir)
+                       goto err;
+
+               rc = do_one_node(system, node_dir);
+               closedir(node_dir);
+
+               if (rc)
                        goto err;
        }
 
-       closedir(dir);
+       closedir(nodes_dir);
        return 0;
 
 err:
-       if (dir)
-               closedir(dir);
+       if (nodes_dir)
+               closedir(nodes_dir);
        return 1;
 }