From: Jeff Mahoney <jeffm@suse.com>
To: autofs@vger.kernel.org
Cc: Ian Kent <raven@themaw.net>
Subject: [BUG] contained_in_local_fs will *always* return true
Date: Fri, 18 Mar 2016 16:44:07 -0400 [thread overview]
Message-ID: <56EC6897.7040409@suse.com> (raw)
[-- Attachment #1.1: Type: text/plain, Size: 2963 bytes --]
Hi autofs maintainers -
I'm investigating a bug in which the user has > 8k direct mounts set up.
As documented in the README, this performs very, very badly and I'm
looking at ways to improve that situation.
In my research, I discovered commit d3ce65e05d1 (autofs-5.0.3 - allow
directory create on NFS root), which was intended to return true for
rootfs. It would pass that test. It also returns true for every other
case as long as there is *any* root file system in the mount table. So,
ultimately, we read the mount table for every path component of every
direct mount for no benefit whatsoever.
====8<====
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] == '/') {
/*
* jeffm -> This will cause the entire
* function to always return true. It
* turns the loop into a test to see if
* anything is mounted on /.
*/
/*
* 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;
}
}
====>8====
This commit landed in 2008 and, since it hasn't changed since then, I
assume there haven't been any bugs reported. I had trouble trying to
locate a mailing list archive for the autofs list, so I can't confirm
that. Googling for contained_in_local_fs really only showed the thread
that proposed the patch initially.
So, I suppose my question is this: Since this bug has existed for
nearly 8 years already, would fixing it at this point be considered a
regression?
If so, we can skip all the checking in do_mkdir entirely, which also
removes the performance impact of large numbers of direct mounts while
using /proc/self/mounts.
If not, how do we move forward ensuring minimal breakage? I have a
patch worked up that skips the mount table parsing entirely and just
tests to see if the parent is either on the root file system, regardless
of file system type, or contained in a file system mounted on a block
device (grab st_dev and see if it exists in /sys/dev/block). Is that
enough or is there a corner case I'm missing?
Thanks,
-Jeff
--
Jeff Mahoney
SUSE Labs
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 881 bytes --]
next reply other threads:[~2016-03-18 20:44 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-18 20:44 Jeff Mahoney [this message]
2016-03-18 21:25 ` [PATCH] autofs: fix contained_in_local_fs and improve scalability Jeff Mahoney
2016-03-19 20:58 ` 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=56EC6897.7040409@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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).