Skip to contentSkip to conversation

how to test rpc_pipefs

RFC PATCH In review Christoph Hellwig 5 participants 10 messages May 6, 2005 – Jun 28, 2005 1 message not in the archive

Subsystems (3)
Networking NFS client NFS server
lore
Share & tools
lore

Christoph Hellwig

author
#0 lore raw

I have some simple changes to make rpc_pipefs use less obscure VFS APIs and I'd like to test it before submitting, any idea how to give rpc_pipefs some coverage without settign up a full-blown NFSv4 env?

-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4 opportunities to win big! Highest score wins.NEC IT Guy Games. Play to win an NEC 61 plasma display. Visit http://www.necitguy.com/?r=20
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#1 lore raw

fr den 06.05.2005 Klokka 17:10 (+0100) skreiv Christoph Hellwig:

Quoted 3 lines

I have some simple changes to make rpc_pipefs use less obscure VFS APIs and I'd like to test it before submitting, any idea how to give rpc_pipefs some coverage without settign up a full-blown NFSv4 env?

Hi Christoph,

Well, all NFS environments (including v2 and v3) automatically create a basic set of files in the nfs/clnt* subdirectory, but beware of generalising that file behaviour since the open(), read(), write() and close() are directly hooked by the objects you are exporting (krb5, and idmap).

Could you be a bit more specific about what kind of API changes you are proposing?

Cheers,
Trond

-------------------------------------------------------
This SF.Net email is sponsored by: NEC IT Guy Games.
Get your fingers limbered up and give it your best shot. 4 great events, 4 opportunities to win big! Highest score wins.NEC IT Guy Games. Play to win an NEC 61 plasma display. Visit http://www.necitguy.com/?r=20
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

author
#2 lore raw
Quoted 5 lines · Trond Myklebust

fr den 06.05.2005 Klokka 17:10 (+0100) skreiv Christoph Hellwig:

Quoted 3 lines

I have some simple changes to make rpc_pipefs use less obscure VFS APIs and I'd like to test it before submitting, any idea how to give rpc_pipefs some coverage without settign up a full-blown NFSv4 env?

Quoted 7 lines

Hi Christoph,

Well, all NFS environments (including v2 and v3) automatically create a basic set of files in the nfs/clnt* subdirectory, but beware of generalising that file behaviour since the open(), read(), write() and close() are directly hooked by the objects you are exporting (krb5, and idmap).

Right - it's the non-default ones this patch is all about. Sorry for beeing unclear.

Quoted 3 lines

Could you be a bit more specific about what kind of API changes you are proposing?

Sure. Currently rpc_mkdir/rpc_rmdir and rpc_mkpipe/mk_unlink have an API that's a little unfortunate. They take an path relative to the rpc_pipefs root and thus need to perform a full lookup. If you look at debugfs or usbfs they always store the dentry for directories they created and thus can pass in a dentry + single pathname component pair into their equivalents of the above functions.

And in fact rpc_pipefs actually stores a dentry for all but one component so this change not only simplifies the core rpc_pipe code but also the callers.

Untested patch is below:

net/sunrpc/rpc_pipe.c +91 -176
Index: linux-2.6/net/sunrpc/rpc_pipe.c
===================================================================
@@ -414,38 +414,6 @@
 	simple_release_fs(&rpc_mount, &rpc_mount_count);
 }
 
-static int
-rpc_lookup_parent(char *path, struct nameidata *nd)
-{
-	if (path[0] == '\0')
-		return -ENOENT;
-	if (rpc_get_mount()) {
-		printk(KERN_WARNING "%s: %s failed to mount "
-			       "pseudofilesystem \n", __FILE__, __FUNCTION__);
-		return -ENODEV;
-	}
-	nd->mnt = mntget(rpc_mount);
-	nd->dentry = dget(rpc_mount->mnt_root);
-	nd->last_type = LAST_ROOT;
-	nd->flags = LOOKUP_PARENT;
-	nd->depth = 0;
-
-	if (path_walk(path, nd)) {
-		printk(KERN_WARNING "%s: %s failed to find path %s\n",
-				__FILE__, __FUNCTION__, path);
-		rpc_put_mount();
-		return -ENOENT;
-	}
-	return 0;
-}
-
-static void
-rpc_release_path(struct nameidata *nd)
-{
-	path_release(nd);
-	rpc_put_mount();
-}
-
 static struct inode *
 rpc_get_inode(struct super_block *sb, int mode)
 {

@@ -550,197 +518,144 @@
 	return -ENOMEM;
 }
 
-static int
-__rpc_mkdir(struct inode *dir, struct dentry *dentry)
-{
-	struct inode *inode;
-
-	inode = rpc_get_inode(dir->i_sb, S_IFDIR | S_IRUSR | S_IXUSR);
-	if (!inode)
-		goto out_err;
-	inode->i_ino = iunique(dir->i_sb, 100);
-	d_instantiate(dentry, inode);
-	dir->i_nlink++;
-	inode_dir_notify(dir, DN_CREATE);
-	rpc_get_mount();
-	return 0;
-out_err:
-	printk(KERN_WARNING "%s: %s failed to allocate inode for dentry %s\n",
-			__FILE__, __FUNCTION__, dentry->d_name.name);
-	return -ENOMEM;
-}
-
-static int
-__rpc_rmdir(struct inode *dir, struct dentry *dentry)
-{
-	int error;
-
-	shrink_dcache_parent(dentry);
-	if (dentry->d_inode) {
-		rpc_close_pipes(dentry->d_inode);
-		rpc_inode_setowner(dentry->d_inode, NULL);
-	}
-	if ((error = simple_rmdir(dir, dentry)) != 0)
-		return error;
-	if (!error) {
-		inode_dir_notify(dir, DN_DELETE);
-		d_drop(dentry);
-		rpc_put_mount();
-	}
-	return 0;
-}
-
-static struct dentry *
-rpc_lookup_negative(char *path, struct nameidata *nd)
+struct dentry *
+rpc_mkdir(struct dentry *parent, char *name, struct rpc_clnt *rpc_client)
 {
+	struct inode *dir = parent->d_inode;
 	struct dentry *dentry;
-	struct inode *dir;
+	struct inode *inode;
 	int error;
-
-	if ((error = rpc_lookup_parent(path, nd)) != 0)
+	
+	error = rpc_get_mount();
+	if (error)
 		return ERR_PTR(error);
-	dir = nd->dentry->d_inode;
+
 	down(&dir->i_sem);
-	dentry = lookup_hash(&nd->last, nd->dentry);
+	dentry = lookup_one_len(name, parent, strlen(name));
 	if (IS_ERR(dentry))
-		goto out_err;
+		goto out_unlock;
 	if (dentry->d_inode) {
-		dput(dentry);
 		dentry = ERR_PTR(-EEXIST);
-		goto out_err;
+		goto out_dput;
 	}
-	return dentry;
-out_err:
-	up(&dir->i_sem);
-	rpc_release_path(nd);
-	return dentry;
-}
 
+	inode = rpc_get_inode(dir->i_sb, S_IFDIR | S_IRUSR | S_IXUSR);
+	if (!inode)
+		goto out_dput;
+	inode->i_ino = iunique(dir->i_sb, 100);
+	dir->i_nlink++;
+	RPC_I(dentry->d_inode)->private = rpc_client;
 
-struct dentry *
-rpc_mkdir(char *path, struct rpc_clnt *rpc_client)
-{
-	struct nameidata nd;
-	struct dentry *dentry;
-	struct inode *dir;
-	int error;
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	up(&dir->i_sem);
+
+	inode_dir_notify(dir, DN_CREATE);
 
-	dentry = rpc_lookup_negative(path, &nd);
-	if (IS_ERR(dentry))
-		return dentry;
-	dir = nd.dentry->d_inode;
-	if ((error = __rpc_mkdir(dir, dentry)) != 0)
-		goto err_dput;
-	RPC_I(dentry->d_inode)->private = rpc_client;
 	error = rpc_populate(dentry, authfiles,
 			RPCAUTH_info, RPCAUTH_EOF);
 	if (error)
-		goto err_depopulate;
-out:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
+		goto out_depopulate;
+
 	return dentry;
-err_depopulate:
-	rpc_depopulate(dentry);
-	__rpc_rmdir(dir, dentry);
-err_dput:
+
+ out_depopulate:
+	rpc_rmdir(dentry);
+ out_dput:
 	dput(dentry);
-	printk(KERN_WARNING "%s: %s() failed to create directory %s (errno = %d)\n",
-			__FILE__, __FUNCTION__, path, error);
-	dentry = ERR_PTR(error);
-	goto out;
+ out_unlock:
+	up(&dir->i_sem);
+	rpc_put_mount();
+	return dentry;
 }
 
-int
-rpc_rmdir(char *path)
+void
+rpc_rmdir(struct dentry *dentry)
 {
-	struct nameidata nd;
-	struct dentry *dentry;
-	struct inode *dir;
-	int error;
+	struct dentry *parent = dentry->d_parent;
 
-	if ((error = rpc_lookup_parent(path, &nd)) != 0)
-		return error;
-	dir = nd.dentry->d_inode;
-	down(&dir->i_sem);
-	dentry = lookup_hash(&nd.last, nd.dentry);
-	if (IS_ERR(dentry)) {
-		error = PTR_ERR(dentry);
-		goto out_release;
-	}
 	rpc_depopulate(dentry);
-	error = __rpc_rmdir(dir, dentry);
-	dput(dentry);
-out_release:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
-	return error;
+
+	down(&parent->d_inode->i_sem);
+	if (dentry->d_inode) {
+		rpc_close_pipes(dentry->d_inode);
+		rpc_inode_setowner(dentry->d_inode, NULL);
+		simple_rmdir(parent->d_inode, dentry);
+	}
+	up(&parent->d_inode->i_sem);
+
+	inode_dir_notify(parent->d_inode, DN_DELETE);
+	rpc_put_mount();
 }
 
 struct dentry *
-rpc_mkpipe(char *path, void *private, struct rpc_pipe_ops *ops, int flags)
+rpc_mkpipe(struct dentry *parent, char *name, void *private,
+	   struct rpc_pipe_ops *ops, int flags)
 {
-	struct nameidata nd;
+	struct inode *dir = parent->d_inode;
 	struct dentry *dentry;
-	struct inode *dir, *inode;
+	struct inode *inode;
 	struct rpc_inode *rpci;
+	int error;
 
-	dentry = rpc_lookup_negative(path, &nd);
+	error = rpc_get_mount();
+	if (error)
+		return ERR_PTR(error);
+
+	down(&parent->d_inode->i_sem);
+	dentry = lookup_one_len(name, parent, strlen(name));
 	if (IS_ERR(dentry))
-		return dentry;
-	dir = nd.dentry->d_inode;
-	inode = rpc_get_inode(dir->i_sb, S_IFSOCK | S_IRUSR | S_IWUSR);
-	if (!inode)
-		goto err_dput;
+		goto out_unlock;
+	if (dentry->d_inode) {
+		dentry = ERR_PTR(-EEXIST);
+		goto out_dput;
+	}
+
+	inode = rpc_get_inode(parent->d_inode->i_sb,
+			S_IFSOCK | S_IRUSR | S_IWUSR);
+	if (!inode) {
+		dentry = ERR_PTR(-ENOMEM);
+		goto out_dput;
+	}
+
 	inode->i_ino = iunique(dir->i_sb, 100);
 	inode->i_fop = &rpc_pipe_fops;
-	d_instantiate(dentry, inode);
+
 	rpci = RPC_I(inode);
 	rpci->private = private;
 	rpci->flags = flags;
 	rpci->ops = ops;
+
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	up(&parent->d_inode->i_sem);
+
 	inode_dir_notify(dir, DN_CREATE);
-out:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
 	return dentry;
-err_dput:
+
+ out_dput:
 	dput(dentry);
-	dentry = ERR_PTR(-ENOMEM);
-	printk(KERN_WARNING "%s: %s() failed to create pipe %s (errno = %d)\n",
-			__FILE__, __FUNCTION__, path, -ENOMEM);
-	goto out;
+ out_unlock:
+	up(&parent->d_inode->i_sem);
+	rpc_put_mount();
+	return dentry;
 }
 
-int
-rpc_unlink(char *path)
+void
+rpc_unlink(struct dentry *dentry)
 {
-	struct nameidata nd;
-	struct dentry *dentry;
-	struct inode *dir;
-	int error;
+	struct dentry *parent = dentry->d_parent;
 
-	if ((error = rpc_lookup_parent(path, &nd)) != 0)
-		return error;
-	dir = nd.dentry->d_inode;
-	down(&dir->i_sem);
-	dentry = lookup_hash(&nd.last, nd.dentry);
-	if (IS_ERR(dentry)) {
-		error = PTR_ERR(dentry);
-		goto out_release;
-	}
-	d_drop(dentry);
+	down(&parent->d_inode->i_sem);
 	if (dentry->d_inode) {
 		rpc_close_pipes(dentry->d_inode);
 		rpc_inode_setowner(dentry->d_inode, NULL);
-		error = simple_unlink(dir, dentry);
+		simple_unlink(parent->d_inode, dentry);
 	}
-	dput(dentry);
-	inode_dir_notify(dir, DN_DELETE);
-out_release:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
-	return error;
+	up(&parent->d_inode->i_sem);
+
+	inode_dir_notify(parent->d_inode, DN_DELETE);
+	rpc_put_mount();
 }
 
 /*

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_id=7393&alloc_id=16281&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#3 lore raw

Memory usage

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Greetings,

How does NFS handle caching? more particularly is there a way to limit the memory usage for caching?

Does it show up as kernel cache or is it displayed in the process list?

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)

iD8DBQFCgW+nlt/irWun80cRAszLAJ90pQz2BXCjV8N/4a39wRISQ1XUXgCdHFMd o3aqWUymcZ40hJVTH03CWKc=
=yr4r
-----END PGP SIGNATURE-----

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_id=7393&alloc_id=16281&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw

Re: Memory usage

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

NFS server.

We have a box running as a NFS server and a mysql server. What were running into is that we have a large amount of memory being sucked up by cache and the box is swapping like crazy. It doesnt seem to be release the cache back so im stumped. The only thing using memory is mysql.

All we can think of is the NFS server is using kernel cache for caching (a reach i know)

|~        total:    used:    free:  shared: buffers:  cached:
|Mem:  4224462848 4206002176 18460672        0 41304064 2938032128
|Swap: 2146754560 1073373184 1073381376
|MemTotal:      4125452 kB
|MemFree:         18028 kB
|MemShared:           0 kB
|Buffers:         40336 kB
|Cached:        2095616 kB                        <---- 2G disk cache
|SwapCached:     773556 kB
|Active:        3186300 kB
|ActiveAnon:    1707252 kB
|ActiveCache:   1479048 kB
|Inact_dirty:    607944 kB
|Inact_laundry:  117300 kB
|Inact_clean:     65180 kB
|Inact_target:   795344 kB
|HighTotal:     3276224 kB
|HighFree:         1200 kB
|LowTotal:       849228 kB
|LowFree:         16828 kB
|SwapTotal:     2096440 kB                      <-- 1G swap usage
|SwapFree:      1048224 kB
|HugePages_Total:     0
|HugePages_Free:      0
|Hugepagesize:     2048 kB

If it is indeed NFS is there a way to make it not do that :P I looked in my o'reilly book, googled and posted to kernel dev (WIth my asbestos jacket on and zipped up ;)

The machine is running 2.4.21 EL-SMP

Im completely stumped...

Thanks for looking at it :)

Liz

|> How does NFS handle caching? more particularly is there a way to limit
|> the memory usage for caching?
|>
|>
|> Does it show up as kernel cache or is it displayed in the process
|> list?
|>
| Liz,
|
| Are you referring to a nfs server or client?
|
| -Sean
|

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)

iD8DBQFCgXcIlt/irWun80cRAqtaAJ9LaRgz6LN44xh9d0bbvpLizWW2egCePjTD QWAHmxtHMeyCOKqw3JcPVwg=
=wkVW
-----END PGP SIGNATURE-----

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_id=7393&alloc_id=16281&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#6 lore raw

Re: Memory usage

Quoted 6 lines

If it is indeed NFS is there a way to make it not do that :P I looked in my o'reilly book, googled and posted to kernel dev (WIth my asbestos jacket on and zipped up ;)

The machine is running 2.4.21 EL-SMP

Go to red hat's bugzilla. Read up on slab values and how to adjust your caching. This is what's going on.
Also:
http://people.redhat.com/nhorman/papers/papers.html
read the top paper. i think it will help your problem some.

-sv

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_id=7393&alloc_id=16281&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#7 lore raw

Re: Memory usage

In reply to liz
Quoted 5 lines · liz

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

NFS server.

Quoted 10 lines

We have a box running as a NFS server and a mysql server. What were running into is that we have a large amount of memory being sucked up by cache and the box is swapping like crazy. It doesnt seem to be release the cache back so im stumped. The only thing using memory is mysql.

All we can think of is the NFS server is using kernel cache for caching (a reach i know)

Depending on how frequently your NFS client accesses its file data in cache, swapping process pages may be the logical thing to do. But if not, you can, in the 2.4.21 RHEL kernels use the pagecache sysctl to help reduce pagecache usage more quickly when under VM pressure. Note that in 2.4.21, setting pagecache.max does not place a cieling on how much memory is used for pagecache, it just sets policy for where memory should be reclaimed from first when needed. Its all in the paper that Seth Referenced.

Hope that helps
Neil

Quoted 72 lines
|~        total:    used:    free:  shared: buffers:  cached:
|Mem:  4224462848 4206002176 18460672        0 41304064 2938032128
|Swap: 2146754560 1073373184 1073381376
|MemTotal:      4125452 kB
|MemFree:         18028 kB
|MemShared:           0 kB
|Buffers:         40336 kB
|Cached:        2095616 kB                        <---- 2G disk cache
|SwapCached:     773556 kB
|Active:        3186300 kB
|ActiveAnon:    1707252 kB
|ActiveCache:   1479048 kB
|Inact_dirty:    607944 kB
|Inact_laundry:  117300 kB
|Inact_clean:     65180 kB
|Inact_target:   795344 kB
|HighTotal:     3276224 kB
|HighFree:         1200 kB
|LowTotal:       849228 kB
|LowFree:         16828 kB
|SwapTotal:     2096440 kB                      <-- 1G swap usage
|SwapFree:      1048224 kB
|HugePages_Total:     0
|HugePages_Free:      0
|Hugepagesize:     2048 kB

If it is indeed NFS is there a way to make it not do that :P I looked in my o'reilly book, googled and posted to kernel dev (WIth my asbestos jacket on and zipped up ;)

The machine is running 2.4.21 EL-SMP

Im completely stumped...

Thanks for looking at it :)

Liz

|> How does NFS handle caching? more particularly is there a way to limit
|> the memory usage for caching?
|>
|>
|> Does it show up as kernel cache or is it displayed in the process
|> list?
|>
| Liz,
|
| Are you referring to a nfs server or client?
|
| -Sean
|

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.0 (GNU/Linux)

iD8DBQFCgXcIlt/irWun80cRAqtaAJ9LaRgz6LN44xh9d0bbvpLizWW2egCePjTD QWAHmxtHMeyCOKqw3JcPVwg=
=wkVW
-----END PGP SIGNATURE-----

-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes Want to be the first software developer in space? Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_id=7393&alloc_id=16281&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Signature · 18 lines
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *[email protected]
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/


-------------------------------------------------------
This SF.Net email is sponsored by Oracle Space Sweepstakes
Want to be the first software developer in space?
Enter now for the Oracle Space Sweepstakes!
http://ads.osdn.com/?ad_id=7393&alloc_id=16281&op=click
_______________________________________________
NFS maillist  -  [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs
author
#8 lore raw

RFC PATCH 1/1 rework rpc_pipefs

In reply to Christoph Hellwig

Currently rpc_mkdir/rpc_rmdir and rpc_mkpipe/mk_unlink have an API that's a little unfortunate. They take a path relative to the rpc_pipefs root and thus need to perform a full lookup. If you look at debugfs or usbfs they always store the dentry for directories they created and thus can pass in a dentry + single pathname component pair into their equivalents of the above functions.

And in fact rpc_pipefs actually stores a dentry for all but one component so this change not only simplifies the core rpc_pipe code but also the callers.

Unfortuntately this code path is only used by the NFS4 idmapper and AUTH_GSSAPI for which I don't have a test enviroment. Could someone give it a spin? It's the last bit needed before we can rework the lookup_hash API

net/sunrpc/rpc_pipe.c +91 -176
Index: linux-2.6/net/sunrpc/rpc_pipe.c
===================================================================
@@ -414,38 +414,6 @@
 	simple_release_fs(&rpc_mount, &rpc_mount_count);
 }
 
-static int
-rpc_lookup_parent(char *path, struct nameidata *nd)
-{
-	if (path[0] == '\0')
-		return -ENOENT;
-	if (rpc_get_mount()) {
-		printk(KERN_WARNING "%s: %s failed to mount "
-			       "pseudofilesystem \n", __FILE__, __FUNCTION__);
-		return -ENODEV;
-	}
-	nd->mnt = mntget(rpc_mount);
-	nd->dentry = dget(rpc_mount->mnt_root);
-	nd->last_type = LAST_ROOT;
-	nd->flags = LOOKUP_PARENT;
-	nd->depth = 0;
-
-	if (path_walk(path, nd)) {
-		printk(KERN_WARNING "%s: %s failed to find path %s\n",
-				__FILE__, __FUNCTION__, path);
-		rpc_put_mount();
-		return -ENOENT;
-	}
-	return 0;
-}
-
-static void
-rpc_release_path(struct nameidata *nd)
-{
-	path_release(nd);
-	rpc_put_mount();
-}
-
 static struct inode *
 rpc_get_inode(struct super_block *sb, int mode)
 {

@@ -550,197 +518,144 @@
 	return -ENOMEM;
 }
 
-static int
-__rpc_mkdir(struct inode *dir, struct dentry *dentry)
-{
-	struct inode *inode;
-
-	inode = rpc_get_inode(dir->i_sb, S_IFDIR | S_IRUSR | S_IXUSR);
-	if (!inode)
-		goto out_err;
-	inode->i_ino = iunique(dir->i_sb, 100);
-	d_instantiate(dentry, inode);
-	dir->i_nlink++;
-	inode_dir_notify(dir, DN_CREATE);
-	rpc_get_mount();
-	return 0;
-out_err:
-	printk(KERN_WARNING "%s: %s failed to allocate inode for dentry %s\n",
-			__FILE__, __FUNCTION__, dentry->d_name.name);
-	return -ENOMEM;
-}
-
-static int
-__rpc_rmdir(struct inode *dir, struct dentry *dentry)
-{
-	int error;
-
-	shrink_dcache_parent(dentry);
-	if (dentry->d_inode) {
-		rpc_close_pipes(dentry->d_inode);
-		rpc_inode_setowner(dentry->d_inode, NULL);
-	}
-	if ((error = simple_rmdir(dir, dentry)) != 0)
-		return error;
-	if (!error) {
-		inode_dir_notify(dir, DN_DELETE);
-		d_drop(dentry);
-		rpc_put_mount();
-	}
-	return 0;
-}
-
-static struct dentry *
-rpc_lookup_negative(char *path, struct nameidata *nd)
+struct dentry *
+rpc_mkdir(struct dentry *parent, char *name, struct rpc_clnt *rpc_client)
 {
+	struct inode *dir = parent->d_inode;
 	struct dentry *dentry;
-	struct inode *dir;
+	struct inode *inode;
 	int error;
-
-	if ((error = rpc_lookup_parent(path, nd)) != 0)
+	
+	error = rpc_get_mount();
+	if (error)
 		return ERR_PTR(error);
-	dir = nd->dentry->d_inode;
+
 	down(&dir->i_sem);
-	dentry = lookup_hash(&nd->last, nd->dentry);
+	dentry = lookup_one_len(name, parent, strlen(name));
 	if (IS_ERR(dentry))
-		goto out_err;
+		goto out_unlock;
 	if (dentry->d_inode) {
-		dput(dentry);
 		dentry = ERR_PTR(-EEXIST);
-		goto out_err;
+		goto out_dput;
 	}
-	return dentry;
-out_err:
-	up(&dir->i_sem);
-	rpc_release_path(nd);
-	return dentry;
-}
 
+	inode = rpc_get_inode(dir->i_sb, S_IFDIR | S_IRUSR | S_IXUSR);
+	if (!inode)
+		goto out_dput;
+	inode->i_ino = iunique(dir->i_sb, 100);
+	dir->i_nlink++;
+	RPC_I(dentry->d_inode)->private = rpc_client;
 
-struct dentry *
-rpc_mkdir(char *path, struct rpc_clnt *rpc_client)
-{
-	struct nameidata nd;
-	struct dentry *dentry;
-	struct inode *dir;
-	int error;
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	up(&dir->i_sem);
+
+	inode_dir_notify(dir, DN_CREATE);
 
-	dentry = rpc_lookup_negative(path, &nd);
-	if (IS_ERR(dentry))
-		return dentry;
-	dir = nd.dentry->d_inode;
-	if ((error = __rpc_mkdir(dir, dentry)) != 0)
-		goto err_dput;
-	RPC_I(dentry->d_inode)->private = rpc_client;
 	error = rpc_populate(dentry, authfiles,
 			RPCAUTH_info, RPCAUTH_EOF);
 	if (error)
-		goto err_depopulate;
-out:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
+		goto out_depopulate;
+
 	return dentry;
-err_depopulate:
-	rpc_depopulate(dentry);
-	__rpc_rmdir(dir, dentry);
-err_dput:
+
+ out_depopulate:
+	rpc_rmdir(dentry);
+ out_dput:
 	dput(dentry);
-	printk(KERN_WARNING "%s: %s() failed to create directory %s (errno = %d)\n",
-			__FILE__, __FUNCTION__, path, error);
-	dentry = ERR_PTR(error);
-	goto out;
+ out_unlock:
+	up(&dir->i_sem);
+	rpc_put_mount();
+	return dentry;
 }
 
-int
-rpc_rmdir(char *path)
+void
+rpc_rmdir(struct dentry *dentry)
 {
-	struct nameidata nd;
-	struct dentry *dentry;
-	struct inode *dir;
-	int error;
+	struct dentry *parent = dentry->d_parent;
 
-	if ((error = rpc_lookup_parent(path, &nd)) != 0)
-		return error;
-	dir = nd.dentry->d_inode;
-	down(&dir->i_sem);
-	dentry = lookup_hash(&nd.last, nd.dentry);
-	if (IS_ERR(dentry)) {
-		error = PTR_ERR(dentry);
-		goto out_release;
-	}
 	rpc_depopulate(dentry);
-	error = __rpc_rmdir(dir, dentry);
-	dput(dentry);
-out_release:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
-	return error;
+
+	down(&parent->d_inode->i_sem);
+	if (dentry->d_inode) {
+		rpc_close_pipes(dentry->d_inode);
+		rpc_inode_setowner(dentry->d_inode, NULL);
+		simple_rmdir(parent->d_inode, dentry);
+	}
+	up(&parent->d_inode->i_sem);
+
+	inode_dir_notify(parent->d_inode, DN_DELETE);
+	rpc_put_mount();
 }
 
 struct dentry *
-rpc_mkpipe(char *path, void *private, struct rpc_pipe_ops *ops, int flags)
+rpc_mkpipe(struct dentry *parent, char *name, void *private,
+	   struct rpc_pipe_ops *ops, int flags)
 {
-	struct nameidata nd;
+	struct inode *dir = parent->d_inode;
 	struct dentry *dentry;
-	struct inode *dir, *inode;
+	struct inode *inode;
 	struct rpc_inode *rpci;
+	int error;
 
-	dentry = rpc_lookup_negative(path, &nd);
+	error = rpc_get_mount();
+	if (error)
+		return ERR_PTR(error);
+
+	down(&parent->d_inode->i_sem);
+	dentry = lookup_one_len(name, parent, strlen(name));
 	if (IS_ERR(dentry))
-		return dentry;
-	dir = nd.dentry->d_inode;
-	inode = rpc_get_inode(dir->i_sb, S_IFSOCK | S_IRUSR | S_IWUSR);
-	if (!inode)
-		goto err_dput;
+		goto out_unlock;
+	if (dentry->d_inode) {
+		dentry = ERR_PTR(-EEXIST);
+		goto out_dput;
+	}
+
+	inode = rpc_get_inode(parent->d_inode->i_sb,
+			S_IFSOCK | S_IRUSR | S_IWUSR);
+	if (!inode) {
+		dentry = ERR_PTR(-ENOMEM);
+		goto out_dput;
+	}
+
 	inode->i_ino = iunique(dir->i_sb, 100);
 	inode->i_fop = &rpc_pipe_fops;
-	d_instantiate(dentry, inode);
+
 	rpci = RPC_I(inode);
 	rpci->private = private;
 	rpci->flags = flags;
 	rpci->ops = ops;
+
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	up(&parent->d_inode->i_sem);
+
 	inode_dir_notify(dir, DN_CREATE);
-out:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
 	return dentry;
-err_dput:
+
+ out_dput:
 	dput(dentry);
-	dentry = ERR_PTR(-ENOMEM);
-	printk(KERN_WARNING "%s: %s() failed to create pipe %s (errno = %d)\n",
-			__FILE__, __FUNCTION__, path, -ENOMEM);
-	goto out;
+ out_unlock:
+	up(&parent->d_inode->i_sem);
+	rpc_put_mount();
+	return dentry;
 }
 
-int
-rpc_unlink(char *path)
+void
+rpc_unlink(struct dentry *dentry)
 {
-	struct nameidata nd;
-	struct dentry *dentry;
-	struct inode *dir;
-	int error;
+	struct dentry *parent = dentry->d_parent;
 
-	if ((error = rpc_lookup_parent(path, &nd)) != 0)
-		return error;
-	dir = nd.dentry->d_inode;
-	down(&dir->i_sem);
-	dentry = lookup_hash(&nd.last, nd.dentry);
-	if (IS_ERR(dentry)) {
-		error = PTR_ERR(dentry);
-		goto out_release;
-	}
-	d_drop(dentry);
+	down(&parent->d_inode->i_sem);
 	if (dentry->d_inode) {
 		rpc_close_pipes(dentry->d_inode);
 		rpc_inode_setowner(dentry->d_inode, NULL);
-		error = simple_unlink(dir, dentry);
+		simple_unlink(parent->d_inode, dentry);
 	}
-	dput(dentry);
-	inode_dir_notify(dir, DN_DELETE);
-out_release:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
-	return error;
+	up(&parent->d_inode->i_sem);
+
+	inode_dir_notify(parent->d_inode, DN_DELETE);
+	rpc_put_mount();
 }
 
 /*

-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

#9 lore raw

Re: [PATCH, RFC] rework rpc_pipefs

ty den 28.06.2005 Klokka 12:29 (+0100) skreiv Christoph Hellwig:

Quoted 7 lines

Currently rpc_mkdir/rpc_rmdir and rpc_mkpipe/mk_unlink have an API that's a little unfortunate. They take a path relative to the rpc_pipefs root and thus need to perform a full lookup. If you look at debugfs or usbfs they always store the dentry for directories they created and thus can pass in a dentry + single pathname component pair into their equivalents of the above functions.

Quoted 6 lines

And in fact rpc_pipefs actually stores a dentry for all but one component so this change not only simplifies the core rpc_pipe code but also the callers.

Unfortuntately this code path is only used by the NFS4 idmapper and AUTH_GSSAPI for which I don't have a test enviroment. Could someone give it a spin? It's the last bit needed before we can rework the lookup_hash API

Sure. May I add the usual "Signed-off-by" line so that I can put it into the NFS_ALL patch stream?

Cheers,
Trond

-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

author
#10 lore raw

Re: [PATCH, RFC] rework rpc_pipefs

Quoted 11 lines · Trond Myklebust

ty den 28.06.2005 Klokka 12:29 (+0100) skreiv Christoph Hellwig:

Quoted 10 lines

Currently rpc_mkdir/rpc_rmdir and rpc_mkpipe/mk_unlink have an API that's a little unfortunate. They take a path relative to the rpc_pipefs root and thus need to perform a full lookup. If you look at debugfs or usbfs they always store the dentry for directories they created and thus can pass in a dentry + single pathname component pair into their equivalents of the above functions.

And in fact rpc_pipefs actually stores a dentry for all but one component so this change not only simplifies the core rpc_pipe code but also the callers.

Quoted 6 lines
Quoted 3 lines

Unfortuntately this code path is only used by the NFS4 idmapper and AUTH_GSSAPI for which I don't have a test enviroment. Could someone give it a spin? It's the last bit needed before we can rework the lookup_hash API

Sure. May I add the usual "Signed-off-by" line so that I can put it into the NFS_ALL patch stream?

Ok, here's the patch again with Signed-off-by line:

Signed-off-by Christoph Hellwig <[email protected]>
net/sunrpc/rpc_pipe.c +91 -176
Index: linux-2.6/net/sunrpc/rpc_pipe.c
===================================================================
@@ -414,38 +414,6 @@
 	simple_release_fs(&rpc_mount, &rpc_mount_count);
 }
 
-static int
-rpc_lookup_parent(char *path, struct nameidata *nd)
-{
-	if (path[0] == '\0')
-		return -ENOENT;
-	if (rpc_get_mount()) {
-		printk(KERN_WARNING "%s: %s failed to mount "
-			       "pseudofilesystem \n", __FILE__, __FUNCTION__);
-		return -ENODEV;
-	}
-	nd->mnt = mntget(rpc_mount);
-	nd->dentry = dget(rpc_mount->mnt_root);
-	nd->last_type = LAST_ROOT;
-	nd->flags = LOOKUP_PARENT;
-	nd->depth = 0;
-
-	if (path_walk(path, nd)) {
-		printk(KERN_WARNING "%s: %s failed to find path %s\n",
-				__FILE__, __FUNCTION__, path);
-		rpc_put_mount();
-		return -ENOENT;
-	}
-	return 0;
-}
-
-static void
-rpc_release_path(struct nameidata *nd)
-{
-	path_release(nd);
-	rpc_put_mount();
-}
-
 static struct inode *
 rpc_get_inode(struct super_block *sb, int mode)
 {

@@ -550,197 +518,144 @@
 	return -ENOMEM;
 }
 
-static int
-__rpc_mkdir(struct inode *dir, struct dentry *dentry)
-{
-	struct inode *inode;
-
-	inode = rpc_get_inode(dir->i_sb, S_IFDIR | S_IRUSR | S_IXUSR);
-	if (!inode)
-		goto out_err;
-	inode->i_ino = iunique(dir->i_sb, 100);
-	d_instantiate(dentry, inode);
-	dir->i_nlink++;
-	inode_dir_notify(dir, DN_CREATE);
-	rpc_get_mount();
-	return 0;
-out_err:
-	printk(KERN_WARNING "%s: %s failed to allocate inode for dentry %s\n",
-			__FILE__, __FUNCTION__, dentry->d_name.name);
-	return -ENOMEM;
-}
-
-static int
-__rpc_rmdir(struct inode *dir, struct dentry *dentry)
-{
-	int error;
-
-	shrink_dcache_parent(dentry);
-	if (dentry->d_inode) {
-		rpc_close_pipes(dentry->d_inode);
-		rpc_inode_setowner(dentry->d_inode, NULL);
-	}
-	if ((error = simple_rmdir(dir, dentry)) != 0)
-		return error;
-	if (!error) {
-		inode_dir_notify(dir, DN_DELETE);
-		d_drop(dentry);
-		rpc_put_mount();
-	}
-	return 0;
-}
-
-static struct dentry *
-rpc_lookup_negative(char *path, struct nameidata *nd)
+struct dentry *
+rpc_mkdir(struct dentry *parent, char *name, struct rpc_clnt *rpc_client)
 {
+	struct inode *dir = parent->d_inode;
 	struct dentry *dentry;
-	struct inode *dir;
+	struct inode *inode;
 	int error;
-
-	if ((error = rpc_lookup_parent(path, nd)) != 0)
+	
+	error = rpc_get_mount();
+	if (error)
 		return ERR_PTR(error);
-	dir = nd->dentry->d_inode;
+
 	down(&dir->i_sem);
-	dentry = lookup_hash(&nd->last, nd->dentry);
+	dentry = lookup_one_len(name, parent, strlen(name));
 	if (IS_ERR(dentry))
-		goto out_err;
+		goto out_unlock;
 	if (dentry->d_inode) {
-		dput(dentry);
 		dentry = ERR_PTR(-EEXIST);
-		goto out_err;
+		goto out_dput;
 	}
-	return dentry;
-out_err:
-	up(&dir->i_sem);
-	rpc_release_path(nd);
-	return dentry;
-}
 
+	inode = rpc_get_inode(dir->i_sb, S_IFDIR | S_IRUSR | S_IXUSR);
+	if (!inode)
+		goto out_dput;
+	inode->i_ino = iunique(dir->i_sb, 100);
+	dir->i_nlink++;
+	RPC_I(dentry->d_inode)->private = rpc_client;
 
-struct dentry *
-rpc_mkdir(char *path, struct rpc_clnt *rpc_client)
-{
-	struct nameidata nd;
-	struct dentry *dentry;
-	struct inode *dir;
-	int error;
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	up(&dir->i_sem);
+
+	inode_dir_notify(dir, DN_CREATE);
 
-	dentry = rpc_lookup_negative(path, &nd);
-	if (IS_ERR(dentry))
-		return dentry;
-	dir = nd.dentry->d_inode;
-	if ((error = __rpc_mkdir(dir, dentry)) != 0)
-		goto err_dput;
-	RPC_I(dentry->d_inode)->private = rpc_client;
 	error = rpc_populate(dentry, authfiles,
 			RPCAUTH_info, RPCAUTH_EOF);
 	if (error)
-		goto err_depopulate;
-out:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
+		goto out_depopulate;
+
 	return dentry;
-err_depopulate:
-	rpc_depopulate(dentry);
-	__rpc_rmdir(dir, dentry);
-err_dput:
+
+ out_depopulate:
+	rpc_rmdir(dentry);
+ out_dput:
 	dput(dentry);
-	printk(KERN_WARNING "%s: %s() failed to create directory %s (errno = %d)\n",
-			__FILE__, __FUNCTION__, path, error);
-	dentry = ERR_PTR(error);
-	goto out;
+ out_unlock:
+	up(&dir->i_sem);
+	rpc_put_mount();
+	return dentry;
 }
 
-int
-rpc_rmdir(char *path)
+void
+rpc_rmdir(struct dentry *dentry)
 {
-	struct nameidata nd;
-	struct dentry *dentry;
-	struct inode *dir;
-	int error;
+	struct dentry *parent = dentry->d_parent;
 
-	if ((error = rpc_lookup_parent(path, &nd)) != 0)
-		return error;
-	dir = nd.dentry->d_inode;
-	down(&dir->i_sem);
-	dentry = lookup_hash(&nd.last, nd.dentry);
-	if (IS_ERR(dentry)) {
-		error = PTR_ERR(dentry);
-		goto out_release;
-	}
 	rpc_depopulate(dentry);
-	error = __rpc_rmdir(dir, dentry);
-	dput(dentry);
-out_release:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
-	return error;
+
+	down(&parent->d_inode->i_sem);
+	if (dentry->d_inode) {
+		rpc_close_pipes(dentry->d_inode);
+		rpc_inode_setowner(dentry->d_inode, NULL);
+		simple_rmdir(parent->d_inode, dentry);
+	}
+	up(&parent->d_inode->i_sem);
+
+	inode_dir_notify(parent->d_inode, DN_DELETE);
+	rpc_put_mount();
 }
 
 struct dentry *
-rpc_mkpipe(char *path, void *private, struct rpc_pipe_ops *ops, int flags)
+rpc_mkpipe(struct dentry *parent, char *name, void *private,
+	   struct rpc_pipe_ops *ops, int flags)
 {
-	struct nameidata nd;
+	struct inode *dir = parent->d_inode;
 	struct dentry *dentry;
-	struct inode *dir, *inode;
+	struct inode *inode;
 	struct rpc_inode *rpci;
+	int error;
 
-	dentry = rpc_lookup_negative(path, &nd);
+	error = rpc_get_mount();
+	if (error)
+		return ERR_PTR(error);
+
+	down(&parent->d_inode->i_sem);
+	dentry = lookup_one_len(name, parent, strlen(name));
 	if (IS_ERR(dentry))
-		return dentry;
-	dir = nd.dentry->d_inode;
-	inode = rpc_get_inode(dir->i_sb, S_IFSOCK | S_IRUSR | S_IWUSR);
-	if (!inode)
-		goto err_dput;
+		goto out_unlock;
+	if (dentry->d_inode) {
+		dentry = ERR_PTR(-EEXIST);
+		goto out_dput;
+	}
+
+	inode = rpc_get_inode(parent->d_inode->i_sb,
+			S_IFSOCK | S_IRUSR | S_IWUSR);
+	if (!inode) {
+		dentry = ERR_PTR(-ENOMEM);
+		goto out_dput;
+	}
+
 	inode->i_ino = iunique(dir->i_sb, 100);
 	inode->i_fop = &rpc_pipe_fops;
-	d_instantiate(dentry, inode);
+
 	rpci = RPC_I(inode);
 	rpci->private = private;
 	rpci->flags = flags;
 	rpci->ops = ops;
+
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	up(&parent->d_inode->i_sem);
+
 	inode_dir_notify(dir, DN_CREATE);
-out:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
 	return dentry;
-err_dput:
+
+ out_dput:
 	dput(dentry);
-	dentry = ERR_PTR(-ENOMEM);
-	printk(KERN_WARNING "%s: %s() failed to create pipe %s (errno = %d)\n",
-			__FILE__, __FUNCTION__, path, -ENOMEM);
-	goto out;
+ out_unlock:
+	up(&parent->d_inode->i_sem);
+	rpc_put_mount();
+	return dentry;
 }
 
-int
-rpc_unlink(char *path)
+void
+rpc_unlink(struct dentry *dentry)
 {
-	struct nameidata nd;
-	struct dentry *dentry;
-	struct inode *dir;
-	int error;
+	struct dentry *parent = dentry->d_parent;
 
-	if ((error = rpc_lookup_parent(path, &nd)) != 0)
-		return error;
-	dir = nd.dentry->d_inode;
-	down(&dir->i_sem);
-	dentry = lookup_hash(&nd.last, nd.dentry);
-	if (IS_ERR(dentry)) {
-		error = PTR_ERR(dentry);
-		goto out_release;
-	}
-	d_drop(dentry);
+	down(&parent->d_inode->i_sem);
 	if (dentry->d_inode) {
 		rpc_close_pipes(dentry->d_inode);
 		rpc_inode_setowner(dentry->d_inode, NULL);
-		error = simple_unlink(dir, dentry);
+		simple_unlink(parent->d_inode, dentry);
 	}
-	dput(dentry);
-	inode_dir_notify(dir, DN_DELETE);
-out_release:
-	up(&dir->i_sem);
-	rpc_release_path(&nd);
-	return error;
+	up(&parent->d_inode->i_sem);
+
+	inode_dir_notify(parent->d_inode, DN_DELETE);
+	rpc_put_mount();
 }
 
 /*

-------------------------------------------------------
SF.Net email is sponsored by: Discover Easy Linux Migration Strategies from IBM. Find simple to follow Roadmaps, straightforward articles, informative Webcasts and more! Get everything you need to get up to speed, fast. http://ads.osdn.com/?ad_id=7477&alloc_id=16492&op=click
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs