Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for autofs "strictexpire" option #18445

Closed
huww98 opened this issue Feb 3, 2021 · 20 comments
Closed

Support for autofs "strictexpire" option #18445

huww98 opened this issue Feb 3, 2021 · 20 comments
Labels
pid1 RFE 🎁 Request for Enhancement, i.e. a feature request

Comments

@huww98
Copy link

huww98 commented Feb 3, 2021

Is your feature request related to a problem? Please describe.
When using automount unit, I found the mount point is not unmounted when TimeoutIdleSec expires. I figured out that it is node-exporter filesystem collector that resets the last_used field in a statfs system call. This can be avoided by specifying "strictexpire" option.

Describe the solution you'd like
Add a StrictExpire option to [Automount] section in .automount unit file. And add strictexpire to this line

xsprintf(options, "fd=%i,pgrp="PID_FMT",minproto=5,maxproto=5,direct", p[1], getpgrp());

Describe alternatives you've considered
Add an AutofsOptions config to [Automount] section, allowing specifying arbitrary options, just like Options in [Mount] section.

For anyone who may be interested in this, Here is the call stack of the last_used field reset, gathered from perf

ffffffffc03ceab1 autofs_mount_wait+0x71 (/lib/modules/5.4.0-62-generic/kernel/fs/autofs/autofs4.ko)
ffffffffc03cfb24 autofs_d_manage+0x84 (/lib/modules/5.4.0-62-generic/kernel/fs/autofs/autofs4.ko)
ffffffff88cea62c follow_managed+0x7c ([kernel.kallsyms])
ffffffff88ceb2dd lookup_fast+0x11d ([kernel.kallsyms])
ffffffff88cebca8 walk_component+0x48 ([kernel.kallsyms])
ffffffff88cecd20 path_lookupat.isra.0+0x80 ([kernel.kallsyms])
ffffffff88cf12de filename_lookup+0xae ([kernel.kallsyms])
ffffffff88cf148a user_path_at_empty+0x3a ([kernel.kallsyms])
ffffffff88d1cf08 user_statfs+0x48 ([kernel.kallsyms])
ffffffff88d1cf98 __do_sys_statfs+0x28 ([kernel.kallsyms])
ffffffff88d1cfe6 __x64_sys_statfs+0x16 ([kernel.kallsyms])
ffffffff88a044c7 do_syscall_64+0x57 ([kernel.kallsyms])
ffffffff8960008c entry_SYSCALL_64+0x7c ([kernel.kallsyms])
          4a5c20 syscall.Syscall+0x30 (/bin/node_exporter)
          9062cd github.com/prometheus/node_exporter/collector.(*filesystemCollector).GetStats+0x73d (/bin/node_exporter)
          904a83 github.com/prometheus/node_exporter/collector.(*filesystemCollector).Update+0x43 (/bin/node_exporter)
          8fa75e github.com/prometheus/node_exporter/collector.execute+0x7e (/bin/node_exporter)
          9500b3 github.com/prometheus/node_exporter/collector.NodeCollector.Collect.func1+0x53 (/bin/node_exporter)
          45bd51 runtime.goexit+0x1 (/bin/node_exporter)
@poettering poettering added RFE 🎁 Request for Enhancement, i.e. a feature request pid1 labels Feb 3, 2021
@raven-au
Copy link

raven-au commented Feb 4, 2021

Fyi,

The questions that need to be answered are should the strictexpire option be used unconditionally and are there any cases where it could cause a problem?

The answer to the later question should be, probably no, there shouldn't be a problem expiring a mount even though it has been recently accessed. The answer to the former question isn't quite so straight forward because some users might see increased umount/mount behaviour they don't expect and don't much like.

The default behaviour is to reset the last_used counter on any access.

The strictexpire autofs mount option changes the expire checking behaviour so that the last_used counter isn't updated on any access so that only mounts that are actually in use, such as when there is an open file or a process working directory within the automounted mount.

Neither the default behaviour nor the strictexpire behaviour satisfies everyone which is why there is an option to change the behaviour.

The default behaviour ensures that mounts that are being used in some way don't expire only to be mounted again shortly afterwards. This has the effect of mounts not expiring when people think they should and can lead to an accumulation of automounted mounts that won't expire because they are being accessed more frequently than the expire timeout.

The strictexpire option avoids these accesses from keeping the automount from expiring.

@raven-au
Copy link

raven-au commented Feb 4, 2021

When using automount unit, I found the mount point is not unmounted when TimeoutIdleSec expires. I figured out that it is node-exporter filesystem collector that resets the last_used field in a statfs system call. This can be avoided by specifying "strictexpire" option.

Did you verify that the using strictexpire option actually does resolve your problem?

@huww98
Copy link
Author

huww98 commented Feb 4, 2021

@raven-au I agree that strictexpire option shouldn't be used unconditionally. That's why I propose to add an option to the [Automount] section, and leave the choice to the end user.

Did you verify that the using strictexpire option actually does resolve your problem?

Sorry, I'm not familiar with this. Since systemd does not expose the strictexpire option, how can I verify that?

@huww98 huww98 closed this as completed Feb 4, 2021
@huww98 huww98 reopened this Feb 4, 2021
@raven-au
Copy link

raven-au commented Feb 4, 2021

@raven-au I agree that strictexpire option shouldn't be used unconditionally. That's why I propose to add an option to the [Automount] section, and leave the choice to the end user.

Yes, I saw that after posting when I looked at this again.

Did you verify that the using strictexpire option actually does resolve your problem?

Sorry, I'm not familiar with this. Since systemd does not expose the strictexpire option, how can I verify that?

Ok, I was asking if you had made a change to systemd to test whether using the option actually resolved the mount not expiring.

If you're not able to do that it's fine I just wanted to know, partly because I had observed mounts not expiring a while ago and I didn't think it was because of accesses updating the last_used field.

I'm not familiar with the unit option processing so that's something I would need to work out if I was to try and propose changes to add this functionality.

I'm also curious what others think about this request.

@raven-au
Copy link

raven-au commented Feb 4, 2021

Did you verify that the using strictexpire option actually does resolve your problem?

Sorry, I'm not familiar with this. Since systemd does not expose the strictexpire option, how can I verify that?

Ok, I was asking if you had made a change to systemd to test whether using the option actually resolved the mount not expiring.

I guess an alternative to checking if the change helps would be a little more about how you concluded the cause was the node-exporter filesystem collector that is accessing the mount more frequently than the expire timeout.

@raven-au
Copy link

raven-au commented Feb 4, 2021

I guess an alternative to checking if the change helps would be a little more about how you concluded the cause was the node-exporter filesystem collector that is accessing the mount more frequently than the expire timeout.

Ok, so this node-exporter is a file system monitoring system.

The strictexpire option can be used for this purpose if it isn't possible to instruct the monitoring system not look at these mounts or if it isn't sensible to exclude them for some reason.

@huww98
Copy link
Author

huww98 commented Feb 4, 2021

I guess an alternative to checking if the change helps would be a little more about how you concluded the cause was the node-exporter filesystem collector that is accessing the mount more frequently than the expire timeout.

I use auditd to verify that no IO request is made to that file system. Then I set a trace point on this line in the kernel using perf probe -s ... -k ... -m autofs4 autofs_mount_wait:16. Then with perf record -e probe:autofs_mount_wait_2 -e probe:autofs_mount_wait_1 -e probe:autofs_mount_wait -g -aR sleep 20, I got the previously posted call stack.

@raven-au
Copy link

raven-au commented Feb 4, 2021

I'm not sure adding this option will behave the way you hope it will.

statfs(2) is one of the stat like functions (I think the only one) that triggers an automount.

There is an argument that, because it wants information about the file system there, and the autofs file system information is essentially meaningless, it should trigger an automount.

So the strictexpire option will probably allow it to expire only to be mounted again on the next statfs access.

I'm not sure that's the behaviour your looking for?

@huww98
Copy link
Author

huww98 commented Feb 4, 2021

I've verified that if the autofs is mounted but the actual filesystem is not, node-exporter will not trigger the mount. Maybe it will not issue statfs if the actual filesystem is not mounted.

@raven-au
Copy link

raven-au commented Feb 4, 2021

I've verified that if the autofs is mounted but the actual filesystem is not, node-exporter will not trigger the mount. Maybe it will not issue statfs if the actual filesystem is not mounted.

Ok, I had a quick look at the node-exporter source and it does ignore autofs file system mounts.
I didn't dig deeply into it but you have checked it so that's enough to say the strictexpire option would work in your case at least.

@raven-au
Copy link

raven-au commented Feb 4, 2021

@poettering , @yuwata could I have your thoughts on this request please?

@sebadoom
Copy link

sebadoom commented Feb 9, 2021

In recent versions of Gnome (3.38.3), gsd-housekeeping is also calling statfs periodically and causing mounted filesystems to be marked as recently used. I was forced to switch to autofs with strictexpire temporarily due to this.

@huww98
Copy link
Author

huww98 commented Feb 16, 2021

I realized that node-exporter has a flag "--collector.filesystem.ignored-mount-points" that can be used to ignore these autofs mount points. And it is sufficient for my use case. But I think the requested feature is still useful in other cases.

@tim-seoss
Copy link

Since #21315 (systemd 250 or later), this can be accomplished with:

[Automount]
ExtraOptions=strictexpire

So this can be closed now I think?

@huww98
Copy link
Author

huww98 commented Jul 24, 2022

Tried this on Fedora 36. Works as expected.

/etc/systemd/system/test_temp.automount

[Unit]
Before=local-fs.target

[Automount]
Where=/test_temp
ExtraOptions=strictexpire
TimeoutIdleSec=30s

/etc/systemd/system/test_temp.mount

[Unit]
Before=local-fs.target

[Mount]
Where=/test_temp
What=tmpfs
Type=tmpfs

@huww98 huww98 closed this as completed Jul 24, 2022
@Robertof
Copy link

Robertof commented Feb 4, 2023

Just wanted to hijack this issue to thank @huww98 for reporting this curious interaction and sparing me hours of painful debugging. You're a gem! I need to learn a thing or two from your kernel debugging skills.

I captured your findings and resolution steps in a Gist which will hopefully appear more easily in search results: https://gist.github.com/Robertof/abfc79660a3140a7c39057abd445e397

@raven-au
Copy link

raven-au commented Feb 5, 2023

There's a little more information about this you might be interested in.

The problem with statfs() has arisen because it's a stat like system call that was originally missed when other stat like system calls were changed to not trigger automounts when automounting support was added to the VFS (actually I think it was already a problem but that's a different thing).

This can't be fixed because now it's expected behaviour and the VFS maintainer won't allow it to be changed.

@Robertof
Copy link

Robertof commented Feb 6, 2023

Hey @raven-au, this is really great context - thank you! I've added it to the Gist as an insightful tidbit.

It's quite interesting! The autofs docs only mention stat in the d_automount() dentry operation (which should only be executed when accessing the directory and an automount might be required). The stack trace that OP posted seems to be in d_manage() which according to the docs should be called during path walks. I wonder whether stat somehow avoids that path walking codepath, though that would require more scouting in fs/namei.c than I am capable of!

@raven-au
Copy link

raven-au commented Feb 6, 2023

Most stat like system calls will have something like this (it's changed over the years):

   int statx_flags = flags | AT_NO_AUTOMOUNT;

which essentially prevents automounting during the path walk.

But statfs() has this:

unsigned int lookup_flags = LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT;

which explicitly enables automounting during the path walk.

That's pretty much all there is to it.

It was because of this problem (at least a large part of the reason) that the strictexpire option was added.

@Robertof
Copy link

Robertof commented Feb 6, 2023

That makes a lot of sense. Thank you so much Ian for taking the time to share your wisdom!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pid1 RFE 🎁 Request for Enhancement, i.e. a feature request
Development

No branches or pull requests

6 participants