Re: [RFC v3 1/1] fs/namespace: remove RCU sync for MNT_DETACH umount

[Christian Brauner <brauner@kernel.org>]

On Mon, Jul 01, 2024 at 12:15:36PM GMT, Jan Kara wrote:
> On Mon 01-07-24 10:41:40, Alexander Larsson wrote:
> > On Mon, Jul 1, 2024 at 7:50 AM Christian Brauner <brauner@kernel.org> 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.
> > >
> > > The crucial bit is really that synchronize_rcu_expedited() ensures that
> > > the final mntput() won't happen until path walk leaves RCU mode.
> > >
> > > 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.
> > 
> > But such behaviour could be kept even without an expedited RCU sync.
> > Such as in my alternative patch for this:
> > https://www.spinics.net/lists/linux-fsdevel/msg270117.html
> > 
> > I.e. we would still guarantee the final mput is called, but not block
> > the return of the unmount call.
> 
> So FWIW the approach of handing off the remainder of namespace_unlock()
> into rcu callback for lazy unmount looks workable to me. Just as Al Viro
> pointed out you cannot do all the stuff right from the RCU callback as the
> context doesn't allow all the work to happen there, so you just need to
> queue work from RCU callback and then do the real work from there (but OTOH
> you can avoid the task work in mnput_noexpire() in that case - will need a
> bit of refactoring).

Yes, but that wasn't what this patch did. As I said I'm not opposed to
trying a _working_ version of this but I suspect we'll slightly change
MNT_DETACH and cause user visible changes (But then we may end up adding
MNT_ASYNC or something which I wouldn't consider the worst idea ever.).