From: Jeff Mahoney <jeffm@suse.com>
To: autofs@vger.kernel.org
Cc: Ian Kent <raven@themaw.net>
Subject: [PATCH] autofs: fix contained_in_local_fs and improve scalability
Date: Fri, 18 Mar 2016 17:25:59 -0400 [thread overview]
Message-ID: <56EC7267.6000103@suse.com> (raw)
In-Reply-To: <56EC6897.7040409@suse.com>
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
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@suse.com>
---
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;
--
To unsubscribe from this list: send the line "unsubscribe autofs" in
next prev parent reply other threads:[~2016-03-18 21:25 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-18 20:44 [BUG] contained_in_local_fs will *always* return true Jeff Mahoney
2016-03-18 21:25 ` Jeff Mahoney [this message]
2016-03-19 20:58 ` [PATCH] autofs: fix contained_in_local_fs and improve scalability Jeff Mahoney
2016-03-21 3:44 ` Ian Kent
2016-03-21 22:08 ` Jeff Mahoney
2016-03-22 0:24 ` Ian Kent
2016-03-22 3:50 ` Jeff Mahoney
2016-03-22 14:13 ` [PATCH] autofs: improve scalability of direct mount path component creation Jeff Mahoney
2016-03-22 14:38 ` Jeff Mahoney
2016-03-24 1:23 ` Ian Kent
2016-03-22 1:24 ` [PATCH] autofs: fix contained_in_local_fs and improve scalability Marion, Mike
2016-03-20 13:05 ` [BUG] contained_in_local_fs will *always* return true Ian Kent
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56EC7267.6000103@suse.com \
--to=jeffm@suse.com \
--cc=autofs@vger.kernel.org \
--cc=raven@themaw.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.