Re: [PATCH v2 4/5] fs: use a for loop when locking a mount

[Seth Forshee <sforshee@kernel.org>]

On Tue, Mar 28, 2023 at 06:13:09PM +0200, Christian Brauner wrote:
> Currently, lock_mount() uses a goto to retry the lookup until it
> succeeded in acquiring the namespace_lock() preventing the top mount
> from being overmounted. While that's perfectly fine we want to lookup
> the mountpoint on the parent of the top mount in later patches. So adapt
> the code to make this easier to implement. Also, the for loop is
> arguably a little cleaner and makes the code easier to follow. No
> functional changes intended.
> 
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>  fs/namespace.c | 50 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 42dc87f86f34..7f22fcfd8eab 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2308,31 +2308,39 @@ static int attach_recursive_mnt(struct mount *source_mnt,
>  
>  static struct mountpoint *lock_mount(struct path *path)
>  {
> -	struct vfsmount *mnt;
> -	struct dentry *dentry = path->dentry;
> -retry:
> -	inode_lock(dentry->d_inode);
> -	if (unlikely(cant_mount(dentry))) {
> -		inode_unlock(dentry->d_inode);
> -		return ERR_PTR(-ENOENT);
> -	}
> -	namespace_lock();
> -	mnt = lookup_mnt(path);
> -	if (likely(!mnt)) {
> -		struct mountpoint *mp = get_mountpoint(dentry);
> -		if (IS_ERR(mp)) {
> -			namespace_unlock();
> +	struct vfsmount *mnt = path->mnt;

One small complaint, mnt is reassigned before it is used, so this
assignment is unnecessary. Otherwise looks good.

Reviewed-by: Seth Forshee (DigitalOcean) <sforshee@kernel.org>