On Dec 14, 2022, at 12:37 AM, Ian Kent [off-list ref] wrote:

On 14/12/22 08:39, Jeff Layton wrote:

quoted

On Wed, 2022-12-14 at 07:14 +0800, Ian Kent wrote:

quoted

On 14/12/22 04:02, Jeff Layton wrote:

quoted

On Tue, 2022-12-13 at 19:00 +0000, Chuck Lever III wrote:

quoted

quoted

On Dec 13, 2022, at 1:08 PM, Jeff Layton [off-list ref] wrote:

If v4 READDIR operation hits a mountpoint and gets back an error,
then it will include that entry in the reply and set RDATTR_ERROR for it
to the error.

That's fine for "normal" exported filesystems, but on the v4root, we
need to be more careful to only expose the existence of dentries that
lead to exports.

If the mountd upcall times out while checking to see whether a
mountpoint on the v4root is exported, then we have no recourse other
than to fail the whole operation.

Thank you for chasing this down!

Failing the whole READDIR when mountd times out might be a bad idea.
If the mountd upcall times out every time, the client can't make
any progress and will continue to emit the failing READDIR request.

Would it be better to skip the unresolvable entry instead and let
the READDIR succeed without that entry?

Mounting doesn't usually require working READDIR. In that situation, a
readdir() might hang (until the client kills), but a lookup of other
dentries that aren't perpetually stalled should be ok in this situation.

If mountd is that hosed then I think it's unlikely that any progress
will be possible anyway.

The READDIR shouldn't trigger a mount yes, but if it's a valid automount

point (basically a valid dentry in this case I think) it should be listed.

It certainly shouldn't hold up the READDIR, passing into it is when a

mount should occur.


That's usually the behavior we want for automounts, we don't want mount

storms on directories full of automount points.

We only want to display it if it's a valid _exported_ mountpoint.

The idea here is to only reveal the parts of the namespace that are
exported in the nfsv4 pseudoroot. The "normal" contents are not shown --
only exported mountpoints and ancestor directories of those mountpoints.

We don't want mountd triggering automounts, in general. If the
underlying filesystem was exported, then it should also already be
mounted, since nfsd doesn't currently trigger automounts in
follow_down().

Umm ... must they already be mounted?


Can't it be a valid mount point either not yet mounted or timed out

and umounted. In that case shouldn't it be listed, I know that's

not the that good an outcome because its stat info will change when

it gets walked into but it's usually the only sane choice.

quoted

There is also a separate patchset by Richard Weinberger to allow nfsd to
trigger automounts if the parent filesystem is exported with -o
crossmnt. That should be ok with this patch, since the automount will be
triggered before the upcall to mountd. That should ensure that it's
already mounted by the time we get to upcalling for its export.

Yep, saw that, ;)

I'm not sure if there is consensus on this patch.

It's been pushed to nfsd's for-rc branch for wider testing, but if
there's a strong objection I can pull it out before the next -rc PR.


--
Chuck Lever