Re: [RFC][PATCH 23/27] elevate write count over calls to vfs_rename()

[Herbert Poetzl <herbert@13thfloor.at>]

On Wed, Jun 07, 2006 at 05:10:36PM -0700, Dave Hansen wrote:
> 
> 
> Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
> ---
> 
>  fs/xattr.c             |    0 
>  lxc-dave/fs/namei.c    |   40 ++++++++++++++++++++++++++--------------
>  lxc-dave/fs/nfsd/vfs.c |   12 +++++++++++-
>  3 files changed, 37 insertions(+), 15 deletions(-)
> 
> diff -puN fs/namei.c~elevate-writers-vfs_rename-part1 fs/namei.c
> --- lxc/fs/namei.c~elevate-writers-vfs_rename-part1	2006-06-07 16:53:26.000000000 -0700
> +++ lxc-dave/fs/namei.c	2006-06-07 16:53:26.000000000 -0700
> @@ -2507,29 +2507,37 @@ static int do_rename(int olddfd, const c
>  	if (error)
>  		goto exit;
>  
> -	error = do_path_lookup(newdfd, newname, LOOKUP_PARENT, &newnd);
> +	error = mnt_want_write(oldnd.mnt);
>  	if (error)
>  		goto exit1;

same here, I'd suspect it changes the return code 
for the case where the 'files' reside on different
mount points from -EXDEV and for the busy case
from -EBUSY to -EROFS ....

don't get me wrong, I'm probably fine with those
changes if they are consistant and do not depend
on whether the inode is RO or the entire mnt point

best,
Herbert

PS: IIRC, my test does not check this case either.
    bugger!

>  
> +	error = do_path_lookup(newdfd, newname, LOOKUP_PARENT, &newnd);
> +	if (error)
> +		goto exit2;
> +
> +	error = mnt_want_write(oldnd.mnt);
> +	if (error)
> +		goto exit3;
> +
>  	error = -EXDEV;
>  	if (oldnd.mnt != newnd.mnt)
> -		goto exit2;
> +		goto exit4;
>  
>  	old_dir = oldnd.dentry;
>  	error = -EBUSY;
>  	if (oldnd.last_type != LAST_NORM)
> -		goto exit2;
> +		goto exit4;
>  
>  	new_dir = newnd.dentry;
>  	if (newnd.last_type != LAST_NORM)
> -		goto exit2;
> +		goto exit4;
>  
>  	trap = lock_rename(new_dir, old_dir);
>  
>  	old_dentry = lookup_hash(&oldnd);
>  	error = PTR_ERR(old_dentry);
>  	if (IS_ERR(old_dentry))
> -		goto exit3;
> +		goto exit5;
>  	/* source must exist */
>  	error = -ENOENT;
>  	if (!old_dentry->d_inode)
> @@ -2538,33 +2546,37 @@ static int do_rename(int olddfd, const c
>  	if (!S_ISDIR(old_dentry->d_inode->i_mode)) {
>  		error = -ENOTDIR;
>  		if (oldnd.last.name[oldnd.last.len])
> -			goto exit4;
> +			goto exit6;
>  		if (newnd.last.name[newnd.last.len])
> -			goto exit4;
> +			goto exit6;
>  	}
>  	/* source should not be ancestor of target */
>  	error = -EINVAL;
>  	if (old_dentry == trap)
> -		goto exit4;
> +		goto exit6;
>  	new_dentry = lookup_hash(&newnd);
>  	error = PTR_ERR(new_dentry);
>  	if (IS_ERR(new_dentry))
> -		goto exit4;
> +		goto exit6;
>  	/* target should not be an ancestor of source */
>  	error = -ENOTEMPTY;
>  	if (new_dentry == trap)
> -		goto exit5;
> +		goto exit7;
>  
>  	error = vfs_rename(old_dir->d_inode, old_dentry,
>  				   new_dir->d_inode, new_dentry);
> -exit5:
> +exit7:
>  	dput(new_dentry);
> -exit4:
> +exit6:
>  	dput(old_dentry);
> -exit3:
> +exit5:
>  	unlock_rename(new_dir, old_dir);
> -exit2:
> +exit4:
> +	mnt_drop_write(newnd.mnt);
> +exit3:
>  	path_release(&newnd);
> +exit2:
> +	mnt_drop_write(oldnd.mnt);
>  exit1:
>  	path_release(&oldnd);
>  exit:
> diff -L ser -puN /dev/null /dev/null
> diff -puN fs/ecryptfs/inode.c~elevate-writers-vfs_rename-part1 fs/ecryptfs/inode.c
> diff -puN fs/nfsd/vfs.c~elevate-writers-vfs_rename-part1 fs/nfsd/vfs.c
> --- lxc/fs/nfsd/vfs.c~elevate-writers-vfs_rename-part1	2006-06-07 16:53:26.000000000 -0700
> +++ lxc-dave/fs/nfsd/vfs.c	2006-06-07 16:53:26.000000000 -0700
> @@ -1601,13 +1601,23 @@ nfsd_rename(struct svc_rqst *rqstp, stru
>  			err = -EPERM;
>  	} else
>  #endif
> +	err = mnt_want_write(ffhp->fh_export->ex_mnt);
> +	if (err)
> +		goto out_dput_new;
> +
> +	err = mnt_want_write(tfhp->fh_export->ex_mnt);
> +	if (err)
> +		goto out_mnt_drop_write_old;
> +
>  	err = vfs_rename(fdir, odentry, tdir, ndentry);
>  	if (!err && EX_ISSYNC(tfhp->fh_export)) {
>  		err = nfsd_sync_dir(tdentry);
>  		if (!err)
>  			err = nfsd_sync_dir(fdentry);
>  	}
> -
> +	mnt_drop_write(tfhp->fh_export->ex_mnt);
> + out_mnt_drop_write_old:
> +	mnt_drop_write(ffhp->fh_export->ex_mnt);
>   out_dput_new:
>  	dput(ndentry);
>   out_dput_old:
> diff -puN fs/nfsd/nfsfh.c~elevate-writers-vfs_rename-part1 fs/nfsd/nfsfh.c
> diff -puN fs/nfsd/nfs3proc.c~elevate-writers-vfs_rename-part1 fs/nfsd/nfs3proc.c
> diff -puN fs/xattr.c~elevate-writers-vfs_rename-part1 fs/xattr.c
> _