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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox