Page MenuHomeFreeBSD

autofs: add NOAUTOMOUNT flag for lookups initiated by stat(2) and family
AbandonedPublic

Authored by kib on Sep 20 2024, 6:29 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Oct 22, 5:55 AM
Unknown Object (File)
Tue, Oct 22, 5:15 AM
Unknown Object (File)
Tue, Oct 22, 3:55 AM
Unknown Object (File)
Oct 7 2024, 12:09 AM
Unknown Object (File)
Oct 5 2024, 12:47 PM
Unknown Object (File)
Oct 4 2024, 12:00 PM
Unknown Object (File)
Oct 4 2024, 5:59 AM
Unknown Object (File)
Oct 3 2024, 9:29 PM

Details

Reviewers
emaste
markj
olce
Summary

to avoid triggering automount from namei() calls initiated by stat(2)

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

kib requested review of this revision.Sep 20 2024, 6:29 AM

When is it useful? Is there something in the base system which needs this?

sys/fs/autofs/autofs_vnops.c
92
kib marked an inline comment as done.Sep 20 2024, 8:41 AM

When is it useful? Is there something in the base system which needs this?

It was prompted by the need of linuxolator. From the Linux man page, it always assume AT_NO_AUTOMOUNT since some version.

I believe it is useful for gui fs browsers.

When is it useful?

Prompted by PR281526 - linuxulator failure. @dchagin ignored LINUX_AT_NO_AUTOMOUNT for statx in ff39d74aa99a49749d1de26dc1f6b1e1bfebceb0 with the comment

Specific to Linux AT_NO_AUTOMOUNT flag tells the kernel to not automount the terminal component of pathname if it is a directory that is an automount point. As it is the default for FreeBSD silencly ignore this flag.

but other stat variants still reject AT_NO_AUTOMOUNT as an unsupported flag.

I've got a patch in progress to move the supported flag test and conversion to a common function to be shared by linux_fstatat64, linux_newfstatat, and linux_statx.

I've got a patch in progress to move the supported flag test and conversion to a common function

D46711

olce accepted this revision.EditedSep 20 2024, 1:12 PM

My original comment here:

Just wanted to point out that a side-effect of this patch is that now, using stat()will actually lead to auto-mounting the target.

was wrong.

This revision is now accepted and ready to land.Sep 20 2024, 1:12 PM

Just wanted to point out that a side-effect of this patch is that now, using stat()will actually lead to auto-mounting the target.

Do we need to look at changes to base system utilities to set AT_NO_AUTOMOUNT (under some option perhaps)?

Just wanted to point out that a side-effect of this patch is that now, using stat()will actually lead to auto-mounting the target.

Isn't stat() causes auto-mount without my patch? Lets formulate the question differently: doesn't the patch only adds a way to disable automount in some cases?

Just wanted to point out that a side-effect of this patch is that now, using stat()will actually lead to auto-mounting the target.

Do we need to look at changes to base system utilities to set AT_NO_AUTOMOUNT (under some option perhaps)?

I think this would indeed make sense for fts(3) and some of it consumers.

In D46710#1065117, @kib wrote:

Isn't stat() causes auto-mount without my patch? Lets formulate the question differently: doesn't the patch only adds a way to disable automount in some cases?

Yes, this is the question I was answering. Checking the code again, I was wrong, the patch only disables automount in some cases.

This dchagin@'s statement:

As it is the default for FreeBSD silencly ignore this flag.

is true only if vfs.autofs.mount_on_stat is 0 (its default value). So I think we should pass the native AT_NO_AUTOMOUNT internally if passed to Linuxulator's system calls.

I’m not sure I follow, but FreeBSD doesn’t mount on stat anyway, so why introduce the native flag instead of making the code consistently ignore the Linux one? Linux autofs semantics is broken in some interesting ways; we don’t need to port functionality that only exists to work around that brokenness.

making the code consistently ignore the Linux one

We should handle Linux AT_NO_AUTOMOUNT in a consistent way across the stat variants, independent of this change. I have D46711 open for that.

I’m not sure I follow, but FreeBSD doesn’t mount on stat anyway, so why introduce the native flag instead of making the code consistently ignore the Linux one? Linux autofs semantics is broken in some interesting ways; we don’t need to port functionality that only exists to work around that brokenness.

Could you please explain how do we avoid triggering automount on stat? The autofs_mount_on_stat knob is mostly nop because the trigger happen during lookup preceeding the call to VOP_GETATTR().

Since the intent seems to be to not trigger automount on stat at all, just fix the lookup trigger. Then we indeed always assume AT_NO_AUTOMOUNT on stat(2), same as linux.

This revision now requires review to proceed.Sep 21 2024, 9:14 PM
kib retitled this revision from autofs: add AT_NO_AUTOMOUNT flag for fstatat(2) to autofs: add NOAUTOMOUNT flag for lookups initiated by stat(2) and family.Sep 21 2024, 9:15 PM
kib edited the summary of this revision. (Show Details)
kib added inline comments.
sys/fs/autofs/autofs_vnops.c
92

I will do a pass to remove all == false comparisons.

This means that vfs.autofs.mount_on_stat continues to control the final autofs_getattr() behavior, regardless of passed (or not) flags, which is probably OK.

This revision is now accepted and ready to land.Sep 22 2024, 11:43 AM
In D46710#1065280, @kib wrote:

I’m not sure I follow, but FreeBSD doesn’t mount on stat anyway, so why introduce the native flag instead of making the code consistently ignore the Linux one? Linux autofs semantics is broken in some interesting ways; we don’t need to port functionality that only exists to work around that brokenness.

Could you please explain how do we avoid triggering automount on stat? The autofs_mount_on_stat knob is mostly nop because the trigger happen during lookup preceeding the call to VOP_GETATTR().

I honestly don't remember, I'm afraid. I remember some careful ordering of operations was involved. But I'm pretty sure we do it, because otherwise an "ls -al" in an autofs directory with mount points would trigger mounting all of them, which would be quite counterproductive with large maps.

In D46710#1065280, @kib wrote:

I’m not sure I follow, but FreeBSD doesn’t mount on stat anyway, so why introduce the native flag instead of making the code consistently ignore the Linux one? Linux autofs semantics is broken in some interesting ways; we don’t need to port functionality that only exists to work around that brokenness.

Could you please explain how do we avoid triggering automount on stat? The autofs_mount_on_stat knob is mostly nop because the trigger happen during lookup preceeding the call to VOP_GETATTR().

I honestly don't remember, I'm afraid. I remember some careful ordering of operations was involved. But I'm pretty sure we do it, because otherwise an "ls -al" in an autofs directory with mount points would trigger mounting all of them, which would be quite counterproductive with large maps.

It could be that namecache hide that VOP_LOOKUP() call from autofs, and you get straight to VOP_GETATTR() without triggering automount.

I think the way it works is that autofs_lookup() doesn't trigger when looking up the autofs directory itself, only when looking up inside that directory?

I think the way it works is that autofs_lookup() doesn't trigger when looking up the autofs directory itself, only when looking up inside that directory?

I do not think so. When lookup finds a mount point, it steps over it. So I do think that this is due to caching.

In D46710#1067752, @kib wrote:

I think the way it works is that autofs_lookup() doesn't trigger when looking up the autofs directory itself, only when looking up inside that directory?

I do not think so. When lookup finds a mount point, it steps over it. So I do think that this is due to caching.

It's not (typically) a mountpoint though? Usually you'd have something like /net/server/share/; the /net is the autofs mount point, and /net/server/ is an ordinary synthetic autofs directory, not a mount point. And this is about looking up /net/server/share - a stat(2) on that shouldn't trigger mounting it.

In D46710#1067752, @kib wrote:

I think the way it works is that autofs_lookup() doesn't trigger when looking up the autofs directory itself, only when looking up inside that directory?

I do not think so. When lookup finds a mount point, it steps over it. So I do think that this is due to caching.

It's not (typically) a mountpoint though? Usually you'd have something like /net/server/share/; the /net is the autofs mount point, and /net/server/ is an ordinary synthetic autofs directory, not a mount point. And this is about looking up /net/server/share - a stat(2) on that shouldn't trigger mounting it.

Right. And what does prevent the autofs_lookup() from triggering the mount in this situation? It should, unless the vnode is present in namecache.

In D46710#1068643, @kib wrote:
In D46710#1067752, @kib wrote:

I think the way it works is that autofs_lookup() doesn't trigger when looking up the autofs directory itself, only when looking up inside that directory?

I do not think so. When lookup finds a mount point, it steps over it. So I do think that this is due to caching.

It's not (typically) a mountpoint though? Usually you'd have something like /net/server/share/; the /net is the autofs mount point, and /net/server/ is an ordinary synthetic autofs directory, not a mount point. And this is about looking up /net/server/share - a stat(2) on that shouldn't trigger mounting it.

Right. And what does prevent the autofs_lookup() from triggering the mount in this situation? It should, unless the vnode is present in namecache.

But autofs_lookup() triggers mount for dvp, not for the node being looked up inside it. And it doesn’t trigger on looking up the “.”.