Commit dafbe689 authored by Dominique Martinet's avatar Dominique Martinet Committed by Dominique Martinet
Browse files

9p fid refcount: cleanup p9_fid_put calls 

Simplify p9_fid_put cleanup path in many 9p functions since the function
is noop on null or error fids.

Also make the *_add_fid() helpers "steal" the fid by nulling its
pointer, so put after them will be noop.

This should lead to no change of behaviour

Link: https://lkml.kernel.org/r/20220612085330.1451496-7-asmadeus@codewreck.org


Reviewed-by: default avatarTyler Hicks <tyhicks@linux.microsoft.com>
Signed-off-by: default avatarDominique Martinet <asmadeus@codewreck.org>
parent 286c171b

fs/9p/fid.c

+16 −8
Original line number Diff line number Diff line
@@ -31,11 +31,15 @@ static inline void __add_fid(struct dentry *dentry, struct p9_fid *fid) 
 * @fid: fid to add

 *

 */

void v9fs_fid_add(struct dentry *dentry, struct p9_fid *fid)

void v9fs_fid_add(struct dentry *dentry, struct p9_fid **pfid)

{

	struct p9_fid *fid = *pfid;



	spin_lock(&dentry->d_lock);

	__add_fid(dentry, fid);

	spin_unlock(&dentry->d_lock);



	*pfid = NULL;

}



/**
@@ -72,11 +76,15 @@ static struct p9_fid *v9fs_fid_find_inode(struct inode *inode, kuid_t uid) 
 *

 */



void v9fs_open_fid_add(struct inode *inode, struct p9_fid *fid)

void v9fs_open_fid_add(struct inode *inode, struct p9_fid **pfid)

{

	struct p9_fid *fid = *pfid;



	spin_lock(&inode->i_lock);

	hlist_add_head(&fid->ilist, (struct hlist_head *)&inode->i_private);

	spin_unlock(&inode->i_lock);



	*pfid = NULL;

}
@@ -189,13 +197,13 @@ static struct p9_fid *v9fs_fid_lookup_with_uid(struct dentry *dentry, 
		else

			uname = v9ses->uname;



		root_fid = p9_client_attach(v9ses->clnt, NULL, uname, uid,

		fid = p9_client_attach(v9ses->clnt, NULL, uname, uid,

				       v9ses->aname);

		if (IS_ERR(root_fid))

			return root_fid;

		if (IS_ERR(fid))

			return fid;



		p9_fid_get(root_fid);

		v9fs_fid_add(dentry->d_sb->s_root, root_fid);

		root_fid = p9_fid_get(fid);

		v9fs_fid_add(dentry->d_sb->s_root, &fid);

	}

	/* If we are root ourself just return that */

	if (dentry->d_sb->s_root == dentry)

fs/9p/fid.h

+2 −2
Original line number Diff line number Diff line
@@ -13,9 +13,9 @@ static inline struct p9_fid *v9fs_parent_fid(struct dentry *dentry) 
{

	return v9fs_fid_lookup(dentry->d_parent);

}

void v9fs_fid_add(struct dentry *dentry, struct p9_fid *fid);

void v9fs_fid_add(struct dentry *dentry, struct p9_fid **fid);

struct p9_fid *v9fs_writeback_fid(struct dentry *dentry);

void v9fs_open_fid_add(struct inode *inode, struct p9_fid *fid);

void v9fs_open_fid_add(struct inode *inode, struct p9_fid **fid);

static inline struct p9_fid *clone_fid(struct p9_fid *fid)

{

	return IS_ERR(fid) ? fid :  p9_client_walk(fid, 0, NULL, 1);

fs/9p/vfs_file.c

+3 −2
Original line number Diff line number Diff line
@@ -69,9 +69,10 @@ int v9fs_file_open(struct inode *inode, struct file *file) 
		if ((file->f_flags & O_APPEND) &&

			(!v9fs_proto_dotu(v9ses) && !v9fs_proto_dotl(v9ses)))

			generic_file_llseek(file, 0, SEEK_END);

	}



		file->private_data = fid;

	}



	mutex_lock(&v9inode->v_mutex);

	if ((v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE) &&

	    !v9inode->writeback_fid &&
@@ -95,7 +96,7 @@ int v9fs_file_open(struct inode *inode, struct file *file) 
	if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE)

		fscache_use_cookie(v9fs_inode_cookie(v9inode),

				   file->f_mode & FMODE_WRITE);

	v9fs_open_fid_add(inode, fid);

	v9fs_open_fid_add(inode, &fid);

	return 0;

out_error:

	p9_fid_put(file->private_data);

fs/9p/vfs_inode.c

+24 −37
Original line number Diff line number Diff line
@@ -399,11 +399,9 @@ void v9fs_evict_inode(struct inode *inode) 


	fscache_relinquish_cookie(v9fs_inode_cookie(v9inode), false);

	/* clunk the fid stashed in writeback_fid */

	if (v9inode->writeback_fid) {

	p9_fid_put(v9inode->writeback_fid);

	v9inode->writeback_fid = NULL;

}

}



static int v9fs_test_inode(struct inode *inode, void *data)

{
@@ -633,14 +631,12 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir, 
	if (IS_ERR(ofid)) {

		err = PTR_ERR(ofid);

		p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n", err);

		p9_fid_put(dfid);

		return ERR_PTR(err);

		goto error;

	}



	err = p9_client_fcreate(ofid, name, perm, mode, extension);

	if (err < 0) {

		p9_debug(P9_DEBUG_VFS, "p9_client_fcreate failed %d\n", err);

		p9_fid_put(dfid);

		goto error;

	}
@@ -651,8 +647,6 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir, 
			err = PTR_ERR(fid);

			p9_debug(P9_DEBUG_VFS,

				   "p9_client_walk failed %d\n", err);

			fid = NULL;

			p9_fid_put(dfid);

			goto error;

		}

		/*
@@ -663,21 +657,17 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir, 
			err = PTR_ERR(inode);

			p9_debug(P9_DEBUG_VFS,

				   "inode creation failed %d\n", err);

			p9_fid_put(dfid);

			goto error;

		}

		v9fs_fid_add(dentry, fid);

		v9fs_fid_add(dentry, &fid);

		d_instantiate(dentry, inode);

	}

	p9_fid_put(dfid);

	return ofid;

error:

	if (ofid)

	p9_fid_put(dfid);

	p9_fid_put(ofid);



	if (fid)

	p9_fid_put(fid);



	return ERR_PTR(err);

}
@@ -804,9 +794,9 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry, 
	res = d_splice_alias(inode, dentry);

	if (!IS_ERR(fid)) {

		if (!res)

			v9fs_fid_add(dentry, fid);

			v9fs_fid_add(dentry, &fid);

		else if (!IS_ERR(res))

			v9fs_fid_add(res, fid);

			v9fs_fid_add(res, &fid);

		else

			p9_fid_put(fid);

	}
@@ -847,7 +837,6 @@ v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry, 
						v9fs_proto_dotu(v9ses)));

	if (IS_ERR(fid)) {

		err = PTR_ERR(fid);

		fid = NULL;

		goto error;

	}
@@ -882,7 +871,7 @@ v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry, 
	if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE)

		fscache_use_cookie(v9fs_inode_cookie(v9inode),

				   file->f_mode & FMODE_WRITE);

	v9fs_open_fid_add(inode, fid);

	v9fs_open_fid_add(inode, &fid);



	file->f_mode |= FMODE_CREATED;

out:
@@ -890,7 +879,6 @@ v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry, 
	return err;



error:

	if (fid)

	p9_fid_put(fid);

	goto out;

}
@@ -939,9 +927,9 @@ v9fs_vfs_rename(struct user_namespace *mnt_userns, struct inode *old_dir, 
	struct inode *old_inode;

	struct inode *new_inode;

	struct v9fs_session_info *v9ses;

	struct p9_fid *oldfid, *dfid;

	struct p9_fid *olddirfid;

	struct p9_fid *newdirfid;

	struct p9_fid *oldfid = NULL, *dfid = NULL;

	struct p9_fid *olddirfid = NULL;

	struct p9_fid *newdirfid = NULL;

	struct p9_wstat wstat;



	if (flags)
@@ -958,21 +946,22 @@ v9fs_vfs_rename(struct user_namespace *mnt_userns, struct inode *old_dir, 


	dfid = v9fs_parent_fid(old_dentry);

	olddirfid = clone_fid(dfid);

	if (dfid && !IS_ERR(dfid))

	p9_fid_put(dfid);

	dfid = NULL;



	if (IS_ERR(olddirfid)) {

		retval = PTR_ERR(olddirfid);

		goto done;

		goto error;

	}



	dfid = v9fs_parent_fid(new_dentry);

	newdirfid = clone_fid(dfid);

	p9_fid_put(dfid);

	dfid = NULL;



	if (IS_ERR(newdirfid)) {

		retval = PTR_ERR(newdirfid);

		goto clunk_olddir;

		goto error;

	}



	down_write(&v9ses->rename_sem);
@@ -983,7 +972,7 @@ v9fs_vfs_rename(struct user_namespace *mnt_userns, struct inode *old_dir, 
			retval = p9_client_rename(oldfid, newdirfid,

						  new_dentry->d_name.name);

		if (retval != -EOPNOTSUPP)

			goto clunk_newdir;

			goto error_locked;

	}

	if (old_dentry->d_parent != new_dentry->d_parent) {

		/*
@@ -992,14 +981,14 @@ v9fs_vfs_rename(struct user_namespace *mnt_userns, struct inode *old_dir, 


		p9_debug(P9_DEBUG_ERROR, "old dir and new dir are different\n");

		retval = -EXDEV;

		goto clunk_newdir;

		goto error_locked;

	}

	v9fs_blank_wstat(&wstat);

	wstat.muid = v9ses->uname;

	wstat.name = new_dentry->d_name.name;

	retval = p9_client_wstat(oldfid, &wstat);



clunk_newdir:

error_locked:

	if (!retval) {

		if (new_inode) {

			if (S_ISDIR(new_inode->i_mode))
@@ -1020,12 +1009,10 @@ v9fs_vfs_rename(struct user_namespace *mnt_userns, struct inode *old_dir, 
		d_move(old_dentry, new_dentry);

	}

	up_write(&v9ses->rename_sem);

	p9_fid_put(newdirfid);



clunk_olddir:

error:

	p9_fid_put(newdirfid);

	p9_fid_put(olddirfid);



done:

	p9_fid_put(oldfid);

	return retval;

}

fs/9p/vfs_inode_dotl.c

+18 −40
Original line number Diff line number Diff line
@@ -238,7 +238,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry, 
	struct inode *inode;

	struct p9_fid *fid = NULL;

	struct v9fs_inode *v9inode;

	struct p9_fid *dfid, *ofid, *inode_fid;

	struct p9_fid *dfid = NULL, *ofid = NULL, *inode_fid = NULL;

	struct v9fs_session_info *v9ses;

	struct posix_acl *pacl = NULL, *dacl = NULL;

	struct dentry *res = NULL;
@@ -274,7 +274,6 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry, 
	if (IS_ERR(ofid)) {

		err = PTR_ERR(ofid);

		p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n", err);

		p9_fid_put(dfid);

		goto out;

	}
@@ -286,38 +285,34 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry, 
	if (err) {

		p9_debug(P9_DEBUG_VFS, "Failed to get acl values in creat %d\n",

			 err);

		p9_fid_put(dfid);

		goto error;

		goto out;

	}

	err = p9_client_create_dotl(ofid, name, v9fs_open_to_dotl_flags(flags),

				    mode, gid, &qid);

	if (err < 0) {

		p9_debug(P9_DEBUG_VFS, "p9_client_open_dotl failed in creat %d\n",

			 err);

		p9_fid_put(dfid);

		goto error;

		goto out;

	}

	v9fs_invalidate_inode_attr(dir);



	/* instantiate inode and assign the unopened fid to the dentry */

	fid = p9_client_walk(dfid, 1, &name, 1);

	p9_fid_put(dfid);

	if (IS_ERR(fid)) {

		err = PTR_ERR(fid);

		p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n", err);

		fid = NULL;

		goto error;

		goto out;

	}

	inode = v9fs_get_new_inode_from_fid(v9ses, fid, dir->i_sb);

	if (IS_ERR(inode)) {

		err = PTR_ERR(inode);

		p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n", err);

		goto error;

		goto out;

	}

	/* Now set the ACL based on the default value */

	v9fs_set_create_acl(inode, fid, dacl, pacl);



	v9fs_fid_add(dentry, fid);

	v9fs_fid_add(dentry, &fid);

	d_instantiate(dentry, inode);



	v9inode = V9FS_I(inode);
@@ -336,7 +331,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry, 
		if (IS_ERR(inode_fid)) {

			err = PTR_ERR(inode_fid);

			mutex_unlock(&v9inode->v_mutex);

			goto err_clunk_old_fid;

			goto out;

		}

		v9inode->writeback_fid = (void *) inode_fid;

	}
@@ -344,25 +339,20 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry, 
	/* Since we are opening a file, assign the open fid to the file */

	err = finish_open(file, dentry, generic_file_open);

	if (err)

		goto err_clunk_old_fid;

		goto out;

	file->private_data = ofid;

	if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE)

		fscache_use_cookie(v9fs_inode_cookie(v9inode),

				   file->f_mode & FMODE_WRITE);

	v9fs_open_fid_add(inode, ofid);

	v9fs_open_fid_add(inode, &ofid);

	file->f_mode |= FMODE_CREATED;

out:

	p9_fid_put(dfid);

	p9_fid_put(ofid);

	p9_fid_put(fid);

	v9fs_put_acl(dacl, pacl);

	dput(res);

	return err;



error:

	if (fid)

		p9_fid_put(fid);

err_clunk_old_fid:

	if (ofid)

		p9_fid_put(ofid);

	goto out;

}



/**
@@ -400,7 +390,6 @@ static int v9fs_vfs_mkdir_dotl(struct user_namespace *mnt_userns, 
	if (IS_ERR(dfid)) {

		err = PTR_ERR(dfid);

		p9_debug(P9_DEBUG_VFS, "fid lookup failed %d\n", err);

		dfid = NULL;

		goto error;

	}
@@ -422,7 +411,6 @@ static int v9fs_vfs_mkdir_dotl(struct user_namespace *mnt_userns, 
		err = PTR_ERR(fid);

		p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n",

			 err);

		fid = NULL;

		goto error;

	}
@@ -435,10 +423,9 @@ static int v9fs_vfs_mkdir_dotl(struct user_namespace *mnt_userns, 
				 err);

			goto error;

		}

		v9fs_fid_add(dentry, fid);

		v9fs_fid_add(dentry, &fid);

		v9fs_set_create_acl(inode, fid, dacl, pacl);

		d_instantiate(dentry, inode);

		fid = NULL;

		err = 0;

	} else {

		/*
@@ -457,7 +444,6 @@ static int v9fs_vfs_mkdir_dotl(struct user_namespace *mnt_userns, 
	inc_nlink(dir);

	v9fs_invalidate_inode_attr(dir);

error:

	if (fid)

	p9_fid_put(fid);

	v9fs_put_acl(dacl, pacl);

	p9_fid_put(dfid);
@@ -743,7 +729,6 @@ v9fs_vfs_symlink_dotl(struct user_namespace *mnt_userns, struct inode *dir, 
			err = PTR_ERR(fid);

			p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n",

				 err);

			fid = NULL;

			goto error;

		}
@@ -755,9 +740,8 @@ v9fs_vfs_symlink_dotl(struct user_namespace *mnt_userns, struct inode *dir, 
				 err);

			goto error;

		}

		v9fs_fid_add(dentry, fid);

		v9fs_fid_add(dentry, &fid);

		d_instantiate(dentry, inode);

		fid = NULL;

		err = 0;

	} else {

		/* Not in cached mode. No need to populate inode with stat */
@@ -770,9 +754,7 @@ v9fs_vfs_symlink_dotl(struct user_namespace *mnt_userns, struct inode *dir, 
	}



error:

	if (fid)

	p9_fid_put(fid);



	p9_fid_put(dfid);

	return err;

}
@@ -866,7 +848,6 @@ v9fs_vfs_mknod_dotl(struct user_namespace *mnt_userns, struct inode *dir, 
	if (IS_ERR(dfid)) {

		err = PTR_ERR(dfid);

		p9_debug(P9_DEBUG_VFS, "fid lookup failed %d\n", err);

		dfid = NULL;

		goto error;

	}
@@ -891,7 +872,6 @@ v9fs_vfs_mknod_dotl(struct user_namespace *mnt_userns, struct inode *dir, 
		err = PTR_ERR(fid);

		p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n",

			 err);

		fid = NULL;

		goto error;

	}
@@ -905,9 +885,8 @@ v9fs_vfs_mknod_dotl(struct user_namespace *mnt_userns, struct inode *dir, 
			goto error;

		}

		v9fs_set_create_acl(inode, fid, dacl, pacl);

		v9fs_fid_add(dentry, fid);

		v9fs_fid_add(dentry, &fid);

		d_instantiate(dentry, inode);

		fid = NULL;

		err = 0;

	} else {

		/*
@@ -923,7 +902,6 @@ v9fs_vfs_mknod_dotl(struct user_namespace *mnt_userns, struct inode *dir, 
		d_instantiate(dentry, inode);

	}

error:

	if (fid)

	p9_fid_put(fid);

	v9fs_put_acl(dacl, pacl);

	p9_fid_put(dfid);