On 1/7/24 13:50, Christian Brauner wrote:
I always thought the rcu delay was to ensure concurrent path walks "see" the
umount not to ensure correct operation of the following mntput()(s).
Isn't the sequence of operations roughly, resolve path, lock, deatch,
release
lock, rcu wait, mntput() subordinate mounts, put path.
Sorry but I'm still having trouble understanding the role of the rcu wait.
The crucial bit is really that synchronize_rcu_expedited() ensures that
the final mntput() won't happen until path walk leaves RCU mode.
Sure, that's easily seen, even for me, but the rcu read lock is held for
the duration of the rcu walk and not released until leaving rcu walk more
and, on fail, switches to ref walk mode and restarts. So the mount struct
won't be freed from under the process in rcu walk mode, correct?
This allows caller's like legitimize_mnt() which are called with only
the RCU read-lock during lazy path walk to simple check for
MNT_SYNC_UMOUNT and see that the mnt is about to be killed. If they see
that this mount is MNT_SYNC_UMOUNT then they know that the mount won't
be freed until an RCU grace period is up and so they know that they can
simply put the reference count they took _without having to actually
call mntput()_.
Because if they did have to call mntput() they might end up shutting the
filesystem down instead of umount() and that will cause said EBUSY
errors I mentioned in my earlier mails.
Again, I get this too, but where is the need for the rcu wait in this?
Originally I had the notion that it was to ensure any path walkers had seen
the mount become invalid before tearing down things that enable the
detection
but suddenly I don't get that any more ...
Please help me out here, I just don't get the need (and I'm sure there is
one) for the rcu wait.
Ian
So the mount gets detached in the critical section, then we wait followed by
the mntput()(s). The catch is that not waiting might increase the likelyhood
that concurrent path walks don't see the umount (so that possibly the umount
goes away before the walks see the umount) but I'm not certain. What looks
to
be as much of a problem is mntput() racing with a concurrent mount beacase
while
the detach is done in the critical section the super block instance list
deletion
is not and the wait will make the race possibility more likely. What's more
Concurrent mounters of the same filesystem will wait for each other via
grab_super(). That has it's own logic based on sb->s_active which goes
to zero when all mounts are gone.
mntput() delegates the mount cleanup (which deletes the list instance) to a
workqueue job so this can also occur serially in a following mount command.
No, that only happens when it's a kthread. Regular umount() call goes
via task work which finishes before the caller returns to userspace
(same as closing files work).
In fact I might have seen exactly this behavior in a recent xfs-tests run
where I
was puzzled to see occasional EBUSY return on mounting of mounts that should
not
have been in use following their umount.
That's usually very much other bugs. See commit 2ae4db5647d8 ("fs: don't
misleadingly warn during thaw operations") in vfs.fixes for example.
So I think there are problems here but I don't think the removal of the wait
for
lazy umount is the worst of it.
The question then becomes, to start with, how do we resolve this unjustified
EBUSY
return. Perhaps a completion (used between the umount and mount system
calls) would
work well here?
Again, this already exists deeper down the stack...
