2005-05-06 16:10:23

by Christoph Hellwig

[permalink] [raw]
Subject: how to test rpc_pipefs

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


2005-05-06 20:09:42

by Trond Myklebust

[permalink] [raw]
Subject: Re: how to test rpc_pipefs

fr den 06.05.2005 Klokka 17:10 (+0100) skreiv Christoph Hellwig:
> 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

2005-05-10 12:28:22

by Christoph Hellwig

[permalink] [raw]
Subject: Re: how to test rpc_pipefs

On Fri, May 06, 2005 at 04:09:31PM -0400, Trond Myklebust wrote:
> fr den 06.05.2005 Klokka 17:10 (+0100) skreiv Christoph Hellwig:
> > 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).

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

>
> 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:

Index: linux-2.6/net/sunrpc/rpc_pipe.c
===================================================================
--- linux-2.6.orig/net/sunrpc/rpc_pipe.c 2005-05-09 13:50:05.000000000 +0200
+++ linux-2.6/net/sunrpc/rpc_pipe.c 2005-05-09 15:02:34.000000000 +0200
@@ -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

2005-05-11 02:36:32

by liz

[permalink] [raw]
Subject: 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

2005-05-11 03:07:58

by liz

[permalink] [raw]
Subject: 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

2005-05-11 03:32:27

by seth vidal

[permalink] [raw]
Subject: Re: Memory usage


> 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

2005-05-11 13:42:10

by Neil Horman

[permalink] [raw]
Subject: Re: Memory usage

On Tue, May 10, 2005 at 08:07:52PM -0700, liz wrote:
> -----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)
>
>
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

> |~ 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

--
/***************************************************
*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

2005-06-28 11:29:30

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH, RFC] rework rpc_pipefs

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


Index: linux-2.6/net/sunrpc/rpc_pipe.c
===================================================================
--- linux-2.6.orig/net/sunrpc/rpc_pipe.c 2005-05-09 13:50:05.000000000 +0200
+++ linux-2.6/net/sunrpc/rpc_pipe.c 2005-05-09 15:02:34.000000000 +0200
@@ -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

2005-06-28 11:40:01

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH, RFC] rework rpc_pipefs

ty den 28.06.2005 Klokka 12:29 (+0100) skreiv 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

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

2005-06-28 11:49:55

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH, RFC] rework rpc_pipefs

On Tue, Jun 28, 2005 at 07:39:41AM -0400, Trond Myklebust wrote:
> ty den 28.06.2005 Klokka 12:29 (+0100) skreiv 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
>
> 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]>

Index: linux-2.6/net/sunrpc/rpc_pipe.c
===================================================================
--- linux-2.6.orig/net/sunrpc/rpc_pipe.c 2005-05-09 13:50:05.000000000 +0200
+++ linux-2.6/net/sunrpc/rpc_pipe.c 2005-05-09 15:02:34.000000000 +0200
@@ -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