diff mbox series

xfsprogs: ignore autofs mount table entries

Message ID 160212194125.16851.17467120219710843339.stgit@mickey.themaw.net (mailing list archive)
State Accepted, archived
Headers show
Series xfsprogs: ignore autofs mount table entries | expand

Commit Message

Ian Kent Oct. 8, 2020, 1:52 a.m. UTC
Some of the xfsprogs utilities read the mount table via. getmntent(3).

The mount table may contain (almost always these days since /etc/mtab is
symlinked to /proc/self/mounts) autofs mount entries. During processing
of the mount table entries statfs(2) can be called on mount point paths
which will trigger an automount if those entries are direct or offset
autofs mount triggers (indirect autofs mounts aren't affected).

This can be a problem when there are a lot of autofs direct or offset
mounts because real mounts will be triggered when statfs(2) is called.
This can be particularly bad if the triggered mounts are NFS mounts and
the server is unavailable leading to lengthy boot times or worse.

Simply ignoring autofs mount entries during getmentent(3) traversals
avoids the statfs() call that triggers these mounts. If there are
automounted mounts (real mounts) at the time of reading the mount table
these will still be seen in the list so they will be included if that
actually matters to the reader.

Recent glibc getmntent(3) can ignore autofs mounts but that requires the
autofs user to configure autofs to use the "ignore" pseudo mount option
for autofs mounts. But this isn't yet the autofs default (to prevent
unexpected side effects) so that can't be used.

The autofs direct and offset automount triggers are pseudo file system
mounts and are more or less useless in terms on file system information
so excluding them doesn't sacrifice useful file system information
either.

Consequently excluding autofs mounts shouldn't have any adverse side
effects.

Changes since v1:
- drop hunk from fsr/xfs_fsr.c.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 libfrog/linux.c |    2 ++
 libfrog/paths.c |    2 ++
 2 files changed, 4 insertions(+)

Comments

Ian Kent Oct. 8, 2020, 1:54 a.m. UTC | #1
Oops!

Didn't add v2 to [PATCH] in the title, sorry about that.

On Thu, 2020-10-08 at 09:52 +0800, Ian Kent wrote:
> Some of the xfsprogs utilities read the mount table via.
> getmntent(3).
> 
> The mount table may contain (almost always these days since /etc/mtab
> is
> symlinked to /proc/self/mounts) autofs mount entries. During
> processing
> of the mount table entries statfs(2) can be called on mount point
> paths
> which will trigger an automount if those entries are direct or offset
> autofs mount triggers (indirect autofs mounts aren't affected).
> 
> This can be a problem when there are a lot of autofs direct or offset
> mounts because real mounts will be triggered when statfs(2) is
> called.
> This can be particularly bad if the triggered mounts are NFS mounts
> and
> the server is unavailable leading to lengthy boot times or worse.
> 
> Simply ignoring autofs mount entries during getmentent(3) traversals
> avoids the statfs() call that triggers these mounts. If there are
> automounted mounts (real mounts) at the time of reading the mount
> table
> these will still be seen in the list so they will be included if that
> actually matters to the reader.
> 
> Recent glibc getmntent(3) can ignore autofs mounts but that requires
> the
> autofs user to configure autofs to use the "ignore" pseudo mount
> option
> for autofs mounts. But this isn't yet the autofs default (to prevent
> unexpected side effects) so that can't be used.
> 
> The autofs direct and offset automount triggers are pseudo file
> system
> mounts and are more or less useless in terms on file system
> information
> so excluding them doesn't sacrifice useful file system information
> either.
> 
> Consequently excluding autofs mounts shouldn't have any adverse side
> effects.
> 
> Changes since v1:
> - drop hunk from fsr/xfs_fsr.c.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
>  libfrog/linux.c |    2 ++
>  libfrog/paths.c |    2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/libfrog/linux.c b/libfrog/linux.c
> index 40a839d1..a45d99ab 100644
> --- a/libfrog/linux.c
> +++ b/libfrog/linux.c
> @@ -73,6 +73,8 @@ platform_check_mount(char *name, char *block,
> struct stat *s, int flags)
>  	 * servers.  So first, a simple check: does the "dev" start
> with "/" ?
>  	 */
>  	while ((mnt = getmntent(f)) != NULL) {
> +		if (!strcmp(mnt->mnt_type, "autofs"))
> +			continue;
>  		if (mnt->mnt_fsname[0] != '/')
>  			continue;
>  		if (stat(mnt->mnt_dir, &mst) < 0)
> diff --git a/libfrog/paths.c b/libfrog/paths.c
> index 32737223..d6793764 100644
> --- a/libfrog/paths.c
> +++ b/libfrog/paths.c
> @@ -389,6 +389,8 @@ fs_table_initialise_mounts(
>  			return errno;
>  
>  	while ((mnt = getmntent(mtp)) != NULL) {
> +		if (!strcmp(mnt->mnt_type, "autofs"))
> +			continue;
>  		if (!realpath(mnt->mnt_dir, rmnt_dir))
>  			continue;
>  		if (!realpath(mnt->mnt_fsname, rmnt_fsname))
> 
>
Eric Sandeen Oct. 8, 2020, 8:03 p.m. UTC | #2
On 10/7/20 8:52 PM, Ian Kent wrote:
> Some of the xfsprogs utilities read the mount table via. getmntent(3).
> 
> The mount table may contain (almost always these days since /etc/mtab is
> symlinked to /proc/self/mounts) autofs mount entries. During processing
> of the mount table entries statfs(2) can be called on mount point paths
> which will trigger an automount if those entries are direct or offset
> autofs mount triggers (indirect autofs mounts aren't affected).
> 
> This can be a problem when there are a lot of autofs direct or offset
> mounts because real mounts will be triggered when statfs(2) is called.
> This can be particularly bad if the triggered mounts are NFS mounts and
> the server is unavailable leading to lengthy boot times or worse.
> 
> Simply ignoring autofs mount entries during getmentent(3) traversals
> avoids the statfs() call that triggers these mounts. If there are
> automounted mounts (real mounts) at the time of reading the mount table
> these will still be seen in the list so they will be included if that
> actually matters to the reader.
> 
> Recent glibc getmntent(3) can ignore autofs mounts but that requires the
> autofs user to configure autofs to use the "ignore" pseudo mount option
> for autofs mounts. But this isn't yet the autofs default (to prevent
> unexpected side effects) so that can't be used.
> 
> The autofs direct and offset automount triggers are pseudo file system
> mounts and are more or less useless in terms on file system information
> so excluding them doesn't sacrifice useful file system information
> either.
> 
> Consequently excluding autofs mounts shouldn't have any adverse side
> effects.

(usually this'd go below the "---")

> Changes since v1:
> - drop hunk from fsr/xfs_fsr.c.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
>  libfrog/linux.c |    2 ++
>  libfrog/paths.c |    2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/libfrog/linux.c b/libfrog/linux.c
> index 40a839d1..a45d99ab 100644
> --- a/libfrog/linux.c
> +++ b/libfrog/linux.c
> @@ -73,6 +73,8 @@ platform_check_mount(char *name, char *block, struct stat *s, int flags)
>  	 * servers.  So first, a simple check: does the "dev" start with "/" ?
>  	 */
>  	while ((mnt = getmntent(f)) != NULL) {
> +		if (!strcmp(mnt->mnt_type, "autofs"))
> +			continue;

I may change the order of this test and the next, just so it continues to
align with the comment above.  Shouldn't make any difference, right?

Otherwise:

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

>  		if (mnt->mnt_fsname[0] != '/')
>  			continue;
>  		if (stat(mnt->mnt_dir, &mst) < 0)
> diff --git a/libfrog/paths.c b/libfrog/paths.c
> index 32737223..d6793764 100644
> --- a/libfrog/paths.c
> +++ b/libfrog/paths.c
> @@ -389,6 +389,8 @@ fs_table_initialise_mounts(
>  			return errno;
>  
>  	while ((mnt = getmntent(mtp)) != NULL) {
> +		if (!strcmp(mnt->mnt_type, "autofs"))
> +			continue;
>  		if (!realpath(mnt->mnt_dir, rmnt_dir))
>  			continue;
>  		if (!realpath(mnt->mnt_fsname, rmnt_fsname))
> 
>
Ian Kent Oct. 9, 2020, 12:49 a.m. UTC | #3
On Thu, 2020-10-08 at 15:03 -0500, Eric Sandeen wrote:
> On 10/7/20 8:52 PM, Ian Kent wrote:
> > Some of the xfsprogs utilities read the mount table via.
> > getmntent(3).
> > 
> > The mount table may contain (almost always these days since
> > /etc/mtab is
> > symlinked to /proc/self/mounts) autofs mount entries. During
> > processing
> > of the mount table entries statfs(2) can be called on mount point
> > paths
> > which will trigger an automount if those entries are direct or
> > offset
> > autofs mount triggers (indirect autofs mounts aren't affected).
> > 
> > This can be a problem when there are a lot of autofs direct or
> > offset
> > mounts because real mounts will be triggered when statfs(2) is
> > called.
> > This can be particularly bad if the triggered mounts are NFS mounts
> > and
> > the server is unavailable leading to lengthy boot times or worse.
> > 
> > Simply ignoring autofs mount entries during getmentent(3)
> > traversals
> > avoids the statfs() call that triggers these mounts. If there are
> > automounted mounts (real mounts) at the time of reading the mount
> > table
> > these will still be seen in the list so they will be included if
> > that
> > actually matters to the reader.
> > 
> > Recent glibc getmntent(3) can ignore autofs mounts but that
> > requires the
> > autofs user to configure autofs to use the "ignore" pseudo mount
> > option
> > for autofs mounts. But this isn't yet the autofs default (to
> > prevent
> > unexpected side effects) so that can't be used.
> > 
> > The autofs direct and offset automount triggers are pseudo file
> > system
> > mounts and are more or less useless in terms on file system
> > information
> > so excluding them doesn't sacrifice useful file system information
> > either.
> > 
> > Consequently excluding autofs mounts shouldn't have any adverse
> > side
> > effects.
> 
> (usually this'd go below the "---")
> 
> > Changes since v1:
> > - drop hunk from fsr/xfs_fsr.c.
> > 
> > Signed-off-by: Ian Kent <raven@themaw.net>
> > ---
> >  libfrog/linux.c |    2 ++
> >  libfrog/paths.c |    2 ++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/libfrog/linux.c b/libfrog/linux.c
> > index 40a839d1..a45d99ab 100644
> > --- a/libfrog/linux.c
> > +++ b/libfrog/linux.c
> > @@ -73,6 +73,8 @@ platform_check_mount(char *name, char *block,
> > struct stat *s, int flags)
> >  	 * servers.  So first, a simple check: does the "dev" start
> > with "/" ?
> >  	 */
> >  	while ((mnt = getmntent(f)) != NULL) {
> > +		if (!strcmp(mnt->mnt_type, "autofs"))
> > +			continue;
> 
> I may change the order of this test and the next, just so it
> continues to
> align with the comment above.  Shouldn't make any difference, right?

Yep, no difference to resolving the problem.

The only reason for it to be first is cases where there's say, 40 or
50 mounts and a thousand or more autofs direct mounts. Then every
test you do before skipping the autofs entry adds a thousand or more
tests to the traversal.

Ian
> 
> Otherwise:
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> >  		if (mnt->mnt_fsname[0] != '/')
> >  			continue;
> >  		if (stat(mnt->mnt_dir, &mst) < 0)
> > diff --git a/libfrog/paths.c b/libfrog/paths.c
> > index 32737223..d6793764 100644
> > --- a/libfrog/paths.c
> > +++ b/libfrog/paths.c
> > @@ -389,6 +389,8 @@ fs_table_initialise_mounts(
> >  			return errno;
> >  
> >  	while ((mnt = getmntent(mtp)) != NULL) {
> > +		if (!strcmp(mnt->mnt_type, "autofs"))
> > +			continue;
> >  		if (!realpath(mnt->mnt_dir, rmnt_dir))
> >  			continue;
> >  		if (!realpath(mnt->mnt_fsname, rmnt_fsname))
> > 
> >
Hellwig, Christoph Oct. 15, 2020, 8:25 a.m. UTC | #4
Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/libfrog/linux.c b/libfrog/linux.c
index 40a839d1..a45d99ab 100644
--- a/libfrog/linux.c
+++ b/libfrog/linux.c
@@ -73,6 +73,8 @@  platform_check_mount(char *name, char *block, struct stat *s, int flags)
 	 * servers.  So first, a simple check: does the "dev" start with "/" ?
 	 */
 	while ((mnt = getmntent(f)) != NULL) {
+		if (!strcmp(mnt->mnt_type, "autofs"))
+			continue;
 		if (mnt->mnt_fsname[0] != '/')
 			continue;
 		if (stat(mnt->mnt_dir, &mst) < 0)
diff --git a/libfrog/paths.c b/libfrog/paths.c
index 32737223..d6793764 100644
--- a/libfrog/paths.c
+++ b/libfrog/paths.c
@@ -389,6 +389,8 @@  fs_table_initialise_mounts(
 			return errno;
 
 	while ((mnt = getmntent(mtp)) != NULL) {
+		if (!strcmp(mnt->mnt_type, "autofs"))
+			continue;
 		if (!realpath(mnt->mnt_dir, rmnt_dir))
 			continue;
 		if (!realpath(mnt->mnt_fsname, rmnt_fsname))