On 3/18/16 5:25 PM, Jeff Mahoney wrote:
> Commit d3ce65e05d1 (autofs-5.0.3 - allow directory create on NFS root),
> introduced a bug where contained_in_local_fs would *always* return true.
>
> Since the goal of contained_in_local_fs is only to determine if the
> path would be created on a local file system, it's enough to check a
> few things directly without parsing the mount table at all.
>
> We can check via stat calls:
> 1) If parent is the root directory
> 2) If the parent is located on the root file system
> 3) If the parent is located within a file system mounted
> via a block device
This test will actually fail for btrfs, but I suppose that could be
special cased.
-Jeff
> Large numbers of direct mounts when using /proc/self/mounts instead
> of /etc/mtab result in very slow startup time. In testing, we observed
> ~8k mounts taking over 6 hours to complete.
>
> This is due to reading /proc/self/mounts for every path component of
> every mount. For my test case, that turns out to be about 119k times
> for a total of just under 400 GB read. If it were a flat file, it
> would be also be slow, but /proc/self/mounts is dynamically generated
> every time it's read.
>
> By avoiding reading /proc/self/mounts many times, startup time with 8k
> direct mounts drops from ~6 hours to about 25 seconds.
>
> Signed-off-by: Jeff Mahoney <jeffm@xxxxxxxx>
> ---
> daemon/automount.c | 47 ++++++++++++++++++++++++++++++++++++++++++++---
> include/mounts.h | 1 -
> lib/mounts.c | 45 ---------------------------------------------
> 3 files changed, 44 insertions(+), 49 deletions(-)
>
> --- a/daemon/automount.c
> +++ b/daemon/automount.c
> @@ -98,6 +98,46 @@ static int umount_all(struct autofs_poin
>
> extern struct master *master_list;
>
> +#define SYSFS_PATTERN "/sys/dev/block/%d:%d"
> +static int contained_in_local_fs(const char *parent)
> +{
> + char buf[128];
> + struct stat st, root;
> + int ret;
> +
> + /* The root directory is always considered local, even if it's not. */
> + if (!*parent)
> + return 1;
> +
> + ret = stat("/", &root);
> + if (ret != 0) {
> + logerr("error: stat failed on /: %s", strerror(errno));
> + return 0;
> + }
> +
> + ret = stat(parent, &st);
> + if (ret)
> + return 0;
> + /*
> + * If the parent is on the root file system, it's local.
> + */
> + if (root.st_dev == st.st_dev)
> + return 1;
> +
> + /*
> + * If the sysfs node for the block device exists, it's local.
> + */
> + ret = snprintf(buf, sizeof(buf), SYSFS_PATTERN,
> + major(st.st_dev), minor(st.st_dev));
> + if (ret != 0) {
> + logerr("error: not enough space for sysfs path");
> + return 0;
> + }
> +
> + ret = stat(buf, &st);
> + return (ret == 0);
> +}
> +
> static int do_mkdir(const char *parent, const char *path, mode_t mode)
> {
> int status;
> @@ -114,14 +154,15 @@ static int do_mkdir(const char *parent,
> }
>
> /*
> - * If we're trying to create a directory within an autofs fs
> - * or the path is contained in a localy mounted fs go ahead.
> + * If we're trying to create a directory within an autofs fs,
> + * the parent is the root directory, or the path is contained
> + * in a locally mounted fs go ahead.
> */
> status = -1;
> if (*parent)
> status = statfs(parent, &fs);
> if ((status != -1 && fs.f_type == (__SWORD_TYPE) AUTOFS_SUPER_MAGIC) ||
> - contained_in_local_fs(path)) {
> + contained_in_local_fs(parent)) {
> mode_t mask = umask(0022);
> int ret = mkdir(path, mode);
> (void) umask(mask);
> --- a/include/mounts.h
> +++ b/include/mounts.h
> @@ -96,7 +96,6 @@ char *make_mnt_name_string(char *path);
> struct mnt_list *get_mnt_list(const char *table, const char *path, int include);
> struct mnt_list *reverse_mnt_list(struct mnt_list *list);
> void free_mnt_list(struct mnt_list *list);
> -int contained_in_local_fs(const char *path);
> int is_mounted(const char *table, const char *path, unsigned int type);
> int has_fstab_option(const char *opt);
> char *get_offset(const char *prefix, char *offset,
> --- a/lib/mounts.c
> +++ b/lib/mounts.c
> @@ -636,51 +636,6 @@ void free_mnt_list(struct mnt_list *list
> }
> }
>
> -int contained_in_local_fs(const char *path)
> -{
> - struct mnt_list *mnts, *this;
> - size_t pathlen = strlen(path);
> - int ret;
> -
> - if (!path || !pathlen || pathlen > PATH_MAX)
> - return 0;
> -
> - mnts = get_mnt_list(_PATH_MOUNTED, "/", 1);
> - if (!mnts)
> - return 0;
> -
> - ret = 0;
> -
> - for (this = mnts; this != NULL; this = this->next) {
> - size_t len = strlen(this->path);
> -
> - if (!strncmp(path, this->path, len)) {
> - if (len > 1 && pathlen > len && path[len] != '/')
> - continue;
> - else if (len == 1 && this->path[0] == '/') {
> - /*
> - * always return true on rootfs, we don't
> - * want to break diskless clients.
> - */
> - ret = 1;
> - } else if (this->fs_name[0] == '/') {
> - if (strlen(this->fs_name) > 1) {
> - if (this->fs_name[1] != '/')
> - ret = 1;
> - } else
> - ret = 1;
> - } else if (!strncmp("LABEL=", this->fs_name, 6) ||
> - !strncmp("UUID=", this->fs_name, 5))
> - ret = 1;
> - break;
> - }
> - }
> -
> - free_mnt_list(mnts);
> -
> - return ret;
> -}
> -
> static int table_is_mounted(const char *table, const char *path, unsigned int type)
> {
> struct mntent *mnt;
>
--
Jeff Mahoney
SUSE Labs
Attachment:
signature.asc
Description: OpenPGP digital signature
