Skip to content

Commit 15007b4

Browse files
committed
linux: use mntent.h instead of manually parsing /proc/mounts
setmntent() doesn't support root_fd, but manual parsing of /proc/mounts is fragile, and actually buggy for very long mount lines (see open-mpi/hwloc#142 (comment)). Since we only openat("/proc/mounts") there, just manually concatenate the fsroot_path and use setmntent(). Thanks to Nathan Hjelm for the report. (Cherry-picked from open-mpi/hwloc@d2d07b9) Signed-off-by: Nathan Hjelm <hjelmn@lanl.gov>
1 parent 1384559 commit 15007b4

File tree

3 files changed

+47
-85
lines changed

3 files changed

+47
-85
lines changed

opal/mca/hwloc/hwloc1112/hwloc/NEWS

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
Copyright © 2009 CNRS
2-
Copyright © 2009-2015 Inria. All rights reserved.
2+
Copyright © 2009-2016 Inria. All rights reserved.
33
Copyright © 2009-2013 Université Bordeaux
44
Copyright © 2009-2011 Cisco Systems, Inc. All rights reserved.
55

@@ -17,6 +17,12 @@ bug fixes (and other actions) for each version of hwloc since version
1717
in v0.9.1).
1818

1919

20+
Version 1.11.3
21+
--------------
22+
* Fix /proc/mounts parsing on Linux by using mntent.h.
23+
Thanks to Nathan Hjelm for reporting the issue.
24+
25+
2026
Version 1.11.2
2127
--------------
2228
* Improve support for Intel Knights Landing Xeon Phi on Linux:
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Cherry-picked commits after 1.11.2:
2+
3+
open-mpi/hwloc@d2d07b9a2268699e13e1644b4f2ef7a53ef7396c

opal/mca/hwloc/hwloc1112/hwloc/src/topology-linux.c

Lines changed: 37 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/*
22
* Copyright © 2009 CNRS
3-
* Copyright © 2009-2015 Inria. All rights reserved.
3+
* Copyright © 2009-2016 Inria. All rights reserved.
44
* Copyright © 2009-2013, 2015 Université Bordeaux
55
* Copyright © 2009-2014 Cisco Systems, Inc. All rights reserved.
66
* Copyright © 2015 Intel, Inc. All rights reserved.
@@ -36,12 +36,14 @@
3636
#include <pthread.h>
3737
#include <sys/mman.h>
3838
#include <sys/syscall.h>
39+
#include <mntent.h>
3940
#if defined HWLOC_HAVE_SET_MEMPOLICY || defined HWLOC_HAVE_MBIND
4041
#define migratepages migrate_pages /* workaround broken migratepages prototype in numaif.h before libnuma 2.0.2 */
4142
#include <numaif.h>
4243
#endif
4344

4445
struct hwloc_linux_backend_data_s {
46+
char *root_path; /* NULL if unused */
4547
int root_fd; /* The file descriptor for the file system root, used when browsing, e.g., Linux' sysfs and procfs. */
4648
int is_real_fsroot; /* Boolean saying whether root_fd points to the real filesystem root of the system */
4749
#ifdef HAVE_LIBUDEV_H
@@ -1652,96 +1654,40 @@ hwloc_parse_cpumap(const char *mappath, int fsroot_fd)
16521654
return set;
16531655
}
16541656

1655-
static char *
1656-
hwloc_strdup_mntpath(const char *escapedpath, size_t length)
1657-
{
1658-
char *path = malloc(length+1);
1659-
const char *src = escapedpath, *tmp;
1660-
char *dst = path;
1661-
1662-
while ((tmp = strchr(src, '\\')) != NULL) {
1663-
strncpy(dst, src, tmp-src);
1664-
dst += tmp-src;
1665-
if (!strncmp(tmp+1, "040", 3))
1666-
*dst = ' ';
1667-
else if (!strncmp(tmp+1, "011", 3))
1668-
*dst = ' ';
1669-
else if (!strncmp(tmp+1, "012", 3))
1670-
*dst = '\n';
1671-
else
1672-
*dst = '\\';
1673-
dst++;
1674-
src = tmp+4;
1675-
}
1676-
1677-
strcpy(dst, src);
1678-
1679-
return path;
1680-
}
1681-
16821657
static void
1683-
hwloc_find_linux_cpuset_mntpnt(char **cgroup_mntpnt, char **cpuset_mntpnt, int fsroot_fd)
1658+
hwloc_find_linux_cpuset_mntpnt(char **cgroup_mntpnt, char **cpuset_mntpnt, const char *root_path)
16841659
{
1685-
#define PROC_MOUNT_LINE_LEN 512
1686-
char line[PROC_MOUNT_LINE_LEN];
1660+
char *mount_path;
1661+
struct mntent *mntent;
16871662
FILE *fd;
1663+
int err;
16881664

16891665
*cgroup_mntpnt = NULL;
16901666
*cpuset_mntpnt = NULL;
16911667

1692-
/* ideally we should use setmntent, getmntent, hasmntopt and endmntent,
1693-
* but they do not support fsroot_fd.
1694-
*/
1695-
1696-
fd = hwloc_fopen("/proc/mounts", "r", fsroot_fd);
1668+
if (root_path) {
1669+
/* setmntent() doesn't support openat(), so use the root_path directly */
1670+
err = asprintf(&mount_path, "%s/proc/mounts", root_path);
1671+
if (err < 0)
1672+
return;
1673+
fd = setmntent(mount_path, "r");
1674+
free(mount_path);
1675+
} else {
1676+
fd = setmntent("/proc/mounts", "r");
1677+
}
16971678
if (!fd)
16981679
return;
16991680

1700-
while (fgets(line, sizeof(line), fd)) {
1701-
char *path;
1702-
char *type;
1703-
char *tmp;
1704-
1705-
/* remove the ending " 0 0\n" that the kernel always adds */
1706-
tmp = line + strlen(line) - 5;
1707-
if (tmp < line || strcmp(tmp, " 0 0\n"))
1708-
fprintf(stderr, "Unexpected end of /proc/mounts line `%s'\n", line);
1709-
else
1710-
*tmp = '\0';
1711-
1712-
/* path is after first field and a space */
1713-
tmp = strchr(line, ' ');
1714-
if (!tmp)
1715-
continue;
1716-
path = tmp+1;
1717-
1718-
/* type is after path, which may not contain spaces since the kernel escaped them to \040
1719-
* (see the manpage of getmntent) */
1720-
tmp = strchr(path, ' ');
1721-
if (!tmp)
1722-
continue;
1723-
type = tmp+1;
1724-
/* mark the end of path to ease upcoming strdup */
1725-
*tmp = '\0';
1726-
1727-
if (!strncmp(type, "cpuset ", 7)) {
1728-
/* found a cpuset mntpnt */
1729-
hwloc_debug("Found cpuset mount point on %s\n", path);
1730-
*cpuset_mntpnt = hwloc_strdup_mntpath(path, type-path);
1681+
while ((mntent = getmntent(fd)) != NULL) {
1682+
if (!strcmp(mntent->mnt_type, "cpuset")) {
1683+
hwloc_debug("Found cpuset mount point on %s\n", mntent->mnt_dir);
1684+
*cpuset_mntpnt = strdup(mntent->mnt_dir);
17311685
break;
1732-
1733-
} else if (!strncmp(type, "cgroup ", 7)) {
1686+
} else if (!strcmp(mntent->mnt_type, "cgroup")) {
17341687
/* found a cgroup mntpnt */
1735-
char *opt, *opts;
1688+
char *opt, *opts = mntent->mnt_opts;
17361689
int cpuset_opt = 0;
17371690
int noprefix_opt = 0;
1738-
1739-
/* find options */
1740-
tmp = strchr(type, ' ');
1741-
if (!tmp)
1742-
continue;
1743-
opts = tmp+1;
1744-
17451691
/* look at options */
17461692
while ((opt = strsep(&opts, ",")) != NULL) {
17471693
if (!strcmp(opt, "cpuset"))
@@ -1751,19 +1697,18 @@ hwloc_find_linux_cpuset_mntpnt(char **cgroup_mntpnt, char **cpuset_mntpnt, int f
17511697
}
17521698
if (!cpuset_opt)
17531699
continue;
1754-
17551700
if (noprefix_opt) {
1756-
hwloc_debug("Found cgroup emulating a cpuset mount point on %s\n", path);
1757-
*cpuset_mntpnt = hwloc_strdup_mntpath(path, type-path);
1701+
hwloc_debug("Found cgroup emulating a cpuset mount point on %s\n", mntent->mnt_dir);
1702+
*cpuset_mntpnt = strdup(mntent->mnt_dir);
17581703
} else {
1759-
hwloc_debug("Found cgroup/cpuset mount point on %s\n", path);
1760-
*cgroup_mntpnt = hwloc_strdup_mntpath(path, type-path);
1704+
hwloc_debug("Found cgroup/cpuset mount point on %s\n", mntent->mnt_dir);
1705+
*cgroup_mntpnt = strdup(mntent->mnt_dir);
17611706
}
17621707
break;
17631708
}
17641709
}
17651710

1766-
fclose(fd);
1711+
endmntent(fd);
17671712
}
17681713

17691714
/*
@@ -4130,7 +4075,7 @@ hwloc_look_linuxfs(struct hwloc_backend *backend)
41304075
/**********************
41314076
* Gather the list of admin-disabled cpus and mems
41324077
*/
4133-
hwloc_find_linux_cpuset_mntpnt(&cgroup_mntpnt, &cpuset_mntpnt, data->root_fd);
4078+
hwloc_find_linux_cpuset_mntpnt(&cgroup_mntpnt, &cpuset_mntpnt, data->root_path);
41344079
if (cgroup_mntpnt || cpuset_mntpnt) {
41354080
cpuset_name = hwloc_read_linux_cpuset_name(data->root_fd, topology->pid);
41364081
if (cpuset_name) {
@@ -5157,6 +5102,8 @@ hwloc_linux_backend_disable(struct hwloc_backend *backend)
51575102
{
51585103
struct hwloc_linux_backend_data_s *data = backend->private_data;
51595104
#ifdef HAVE_OPENAT
5105+
if (data->root_path)
5106+
free(data->root_path);
51605107
close(data->root_fd);
51615108
#endif
51625109
#ifdef HAVE_LIBUDEV_H
@@ -5197,6 +5144,7 @@ hwloc_linux_component_instantiate(struct hwloc_disc_component *component,
51975144
/* default values */
51985145
data->is_knl = 0;
51995146
data->is_real_fsroot = 1;
5147+
data->root_path = NULL;
52005148
if (!fsroot_path)
52015149
fsroot_path = "/";
52025150

@@ -5208,6 +5156,7 @@ hwloc_linux_component_instantiate(struct hwloc_disc_component *component,
52085156
if (strcmp(fsroot_path, "/")) {
52095157
backend->is_thissystem = 0;
52105158
data->is_real_fsroot = 0;
5159+
data->root_path = strdup(fsroot_path);
52115160
}
52125161

52135162
/* Since this fd stays open after hwloc returns, mark it as
@@ -5246,6 +5195,10 @@ hwloc_linux_component_instantiate(struct hwloc_disc_component *component,
52465195
return backend;
52475196

52485197
out_with_data:
5198+
#ifdef HAVE_OPENAT
5199+
if (data->root_path)
5200+
free(data->root_path);
5201+
#endif
52495202
free(data);
52505203
out_with_backend:
52515204
free(backend);

0 commit comments

Comments
 (0)