I've reviewed the nfsd code to check the dentry use count tracking, and
have found a few places where it appears that counts were being leaked
(but no double dputs).
The attached patch consists mostly of annotations with a few fixes, and
there were a couple of places where I wasn't sure what to do (e.g. in
nfsd3_proc_remove, is there really a attrstat return?)
Also, I'm still not clear at what point the xdr release functions are
called. Is it unconditional after the main procedure, or as part of the
return encoding? If the latter, we still have a problem -- return
encoding is contigent on no error in the main procedure, and the fh_put
cleanup needs to be called unconditionally.
One other point I noted -- the double_down call should use the dcache
version, as some fixes have been made to it. dcache should export that
for everyone to use ...
Regards,
Bill
--------------39EC06D5E22BA7150B0C6E18
Content-Type: text/plain; charset=us-ascii; name="nfsd_51-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="nfsd_51-patch"
--- linux-2.1.51/fs/nfsd/vfs.c.old Thu Jul 24 17:36:07 1997
+++ linux-2.1.51/fs/nfsd/vfs.c Tue Aug 26 19:34:19 1997
@@ -95,6 +95,7 @@
/*
* Look up one component of a pathname.
+ * N.B. After this call _both_ fhp and resfh need an fh_put
*/
int
nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
@@ -139,6 +140,7 @@
/*
* Set various file attributes.
+ * N.B. After this call fhp needs an fh_put
*/
int
nfsd_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp, struct iattr *iap)
@@ -216,6 +218,7 @@
/*
* Open an existing file or directory.
* The wflag argument indicates write access.
+ * N.B. After this call fhp needs an fh_put
*/
int
nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
@@ -328,6 +331,7 @@
/*
* Read data from a file. count must contain the requested read count
* on entry. On return, *count contains the number of bytes actually read.
+ * N.B. After this call fhp needs an fh_put
*/
int
nfsd_read(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset, char *buf,
@@ -387,6 +391,7 @@
/*
* Write data to a file.
* The stable flag requests synchronous writes.
+ * N.B. After this call fhp needs an fh_put
*/
int
nfsd_write(struct svc_rqst *rqstp, struct svc_fh *fhp, loff_t offset,
@@ -490,6 +495,7 @@
/*
* Create a file (regular, directory, device, fifo).
* UNIX sockets not yet implemented.
+ * N.B. Every call to nfsd_create needs an fh_put for _both_ fhp and resfhp
*/
int
nfsd_create(struct svc_rqst *rqstp, struct svc_fh *fhp,
@@ -578,6 +584,7 @@
* field and call notify_change.
*
* XXX Nobody calls this thing? -DaveM
+ * N.B. After this call fhp needs an fh_put
*/
int
nfsd_truncate(struct svc_rqst *rqstp, struct svc_fh *fhp, unsigned long size)
@@ -615,6 +622,7 @@
/*
* Read a symlink. On entry, *lenp must contain the maximum path length that
* fits into the buffer. On return, it contains the true length.
+ * N.B. After this call fhp needs an fh_put
*/
int
nfsd_readlink(struct svc_rqst *rqstp, struct svc_fh *fhp, char *buf, int *lenp)
@@ -647,6 +655,7 @@
/*
* Create a symlink and look up its inode
+ * N.B. After this call _both_ fhp and resfhp need an fh_put
*/
int
nfsd_symlink(struct svc_rqst *rqstp, struct svc_fh *fhp,
@@ -697,6 +706,7 @@
/*
* Create a hardlink
+ * N.B. After this call _both_ ffhp and tfhp need an fh_put
*/
int
nfsd_link(struct svc_rqst *rqstp, struct svc_fh *ffhp,
@@ -753,6 +763,7 @@
}
/* More "hidden treasure" from the generic VFS. -DaveM */
+/* N.B. VFS double_down was modified to fix a bug ... should use VFS one */
static inline void nfsd_double_down(struct semaphore *s1, struct semaphore *s2)
{
if((unsigned long) s1 < (unsigned long) s2) {
@@ -769,6 +780,7 @@
/*
* Rename a file
+ * N.B. After this call _both_ ffhp and tfhp need an fh_put
*/
int
nfsd_rename(struct svc_rqst *rqstp, struct svc_fh *ffhp, char *fname, int flen,
@@ -829,6 +841,7 @@
/*
* Unlink a file or directory
+ * N.B. After this call fhp needs an fh_put
*/
int
nfsd_unlink(struct svc_rqst *rqstp, struct svc_fh *fhp, int type,
@@ -951,6 +964,7 @@
/*
* Get file system stats
+ * N.B. After this call fhp needs an fh_put
*/
int
nfsd_statfs(struct svc_rqst *rqstp, struct svc_fh *fhp, struct statfs *stat)
--- linux-2.1.51/fs/nfsd/nfsproc.c.old Thu Jul 24 17:36:07 1997
+++ linux-2.1.51/fs/nfsd/nfsproc.c Tue Aug 26 19:09:30 1997
@@ -50,6 +50,7 @@
/*
* Get a file's attributes
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp,
@@ -63,6 +64,7 @@
/*
* Set a file's attributes
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd_proc_setattr(struct svc_rqst *rqstp, struct nfsd_sattrargs *argp,
@@ -76,6 +78,7 @@
/*
* Look up a path name component
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd_proc_lookup(struct svc_rqst *rqstp, struct nfsd_diropargs *argp,
@@ -119,6 +122,7 @@
/*
* Read a portion of a file.
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd_proc_read(struct svc_rqst *rqstp, struct nfsd_readargs *argp,
@@ -156,6 +160,7 @@
/*
* Write data to a file
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd_proc_write(struct svc_rqst *rqstp, struct nfsd_writeargs *argp,
@@ -181,6 +186,7 @@
* and the actual create() call, but one could even consider this a
* feature because this only happens if someone else creates the file
* at the same time.
+ * N.B. After this call _both_ argp->fh and resp->fh need an fh_put
*/
static int
nfsd_proc_create(struct svc_rqst *rqstp, struct nfsd_createargs *argp,
@@ -188,7 +194,7 @@
{
struct inode *dirp, *inode = NULL;
struct iattr *attr;
- svc_fh *dirfhp, *newfhp = NULL;
+ svc_fh *dirfhp, *newfhp;
int nfserr, type, mode;
int rdonly = 0, exists;
dev_t rdev = NODEV;
@@ -202,7 +208,7 @@
/* Get the directory inode */
nfserr = fh_verify(rqstp, dirfhp, S_IFDIR, MAY_EXEC);
if (nfserr)
- RETURN(nfserr);
+ goto done; /* must fh_put dirfhp even on error */
dirp = dirfhp->fh_handle.fh_dentry->d_inode;
/* Check for MAY_WRITE separately. */
@@ -211,10 +217,8 @@
MAY_WRITE);
if (nfserr == nfserr_rofs) {
rdonly = 1; /* Non-fatal error for echo > /dev/null */
- } else if (nfserr) {
- fh_put(dirfhp);
- RETURN(nfserr);
- }
+ } else if (nfserr)
+ goto done;
/* First, check if the file already exists. */
exists = !nfsd_lookup(rqstp, dirfhp, argp->name, argp->len, newfhp);
@@ -378,6 +382,7 @@
/*
* Make directory. This operation is not idempotent.
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd_proc_mkdir(struct svc_rqst *rqstp, struct nfsd_createargs *argp,
@@ -389,6 +394,7 @@
SVCFH_DENTRY(&argp->fh),
argp->name);
+ /* N.B. what about the dentry count?? */
resp->fh.fh_dverified = 0; /* paranoia */
nfserr = nfsd_create(rqstp, &argp->fh, argp->name, argp->len,
&argp->attrs, S_IFDIR, 0, &resp->fh);
--- linux-2.1.51/fs/nfsd/nfs3proc.c.old Thu Jul 24 17:36:07 1997
+++ linux-2.1.51/fs/nfsd/nfs3proc.c Tue Aug 26 19:44:43 1997
@@ -46,6 +46,7 @@
/*
* Get a file's attributes
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd3_proc_getattr(struct svc_rqst *rqstp, struct nfsd_fhandle *argp,
@@ -64,6 +65,7 @@
/*
* Set a file's attributes
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd3_proc_setattr(struct svc_rqst *rqstp, struct nfsd3_sattrargs *argp,
@@ -82,6 +84,7 @@
/*
* Look up a path name component
+ * N.B. After this call _both_ resp->dirfh and resp->fh need an fh_put
*/
static int
nfsd3_proc_lookup(struct svc_rqst *rqstp, struct nfsd3_diropargs *argp,
@@ -134,11 +137,13 @@
/* Read the symlink. */
resp->len = NFS3_MAXPATHLEN;
nfserr = nfsd_readlink(rqstp, &argp->fh, (char *) path, &resp->len);
+ fh_put(&argp->fh);
RETURN(nfserr);
}
/*
* Read a portion of a file.
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd3_proc_read(struct svc_rqst *rqstp, struct nfsd3_readargs *argp,
@@ -180,6 +185,7 @@
/*
* Write data to a file
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd3_proc_write(struct svc_rqst *rqstp, struct nfsd3_writeargs *argp,
@@ -207,6 +213,7 @@
* With NFSv3, CREATE processing is a lot easier than with NFSv2.
* At least in theory; we'll see how it fares in practice when the
* first reports about SunOS compatibility problems start to pour in...
+ * N.B. After this call _both_ resp->dirfh and resp->fh need an fh_put
*/
static int
nfsd3_proc_create(struct svc_rqst *rqstp, struct nfsd3_createargs *argp,
@@ -246,6 +253,7 @@
RETURN(nfserr);
}
+/* N.B. Is nfsd3_attrstat * correct for resp?? table says "void" */
static int
nfsd3_proc_remove(struct svc_rqst *rqstp, struct nfsd3_diropargs *argp,
struct nfsd3_attrstat *resp)
@@ -257,11 +265,16 @@
SVCFH_INO(&argp->fh),
argp->name);
+ /* Is this correct?? */
fh_copy(&resp->fh, &argp->fh);
/* Unlink. -S_IFDIR means file must not be a directory */
- nfserr = nfsd_unlink(rqstp, &resp->fh, -S_IFDIR,
- argp->name, argp->len);
+ nfserr = nfsd_unlink(rqstp, &resp->fh, -S_IFDIR, argp->name, argp->len);
+ /*
+ * N.B. Should be an fh_put here ... nfsd3_proc_rmdir has one,
+ * or else as an xdr release function
+ */
+ fh_put(&resp->fh);
RETURN(nfserr);
}
@@ -336,6 +349,7 @@
/*
* Make directory. This operation is not idempotent.
+ * N.B. After this call resp->fh needs an fh_put
*/
static int
nfsd3_proc_mkdir(struct svc_rqst *rqstp, struct nfsd3_createargs *argp,
@@ -449,13 +463,13 @@
PROC(getattr, fhandle, attrstat, fhandle, RC_NOCACHE),
PROC(setattr, sattrargs, attrstat, fhandle, RC_REPLBUFF),
PROC(none, void, void, void, RC_NOCACHE),
- PROC(lookup, diropargs, diropres, fhandle, RC_NOCACHE),
+ PROC(lookup, diropargs, diropres, fhandle2,RC_NOCACHE),
PROC(readlink, fhandle, readlinkres, void, RC_NOCACHE),
PROC(read, readargs, readres, fhandle, RC_NOCACHE),
PROC(none, void, void, void, RC_NOCACHE),
PROC(write, writeargs, attrstat, fhandle, RC_REPLBUFF),
- PROC(create, createargs, diropres, fhandle, RC_REPLBUFF),
- PROC(remove, diropargs, void, void, RC_REPLSTAT),
+ PROC(create, createargs, diropres, fhandle2,RC_REPLBUFF),
+ PROC(remove, diropargs, void,/* ??*/ void, RC_REPLSTAT),
PROC(rename, renameargs, void, void, RC_REPLSTAT),
PROC(link, linkargs, void, void, RC_REPLSTAT),
PROC(symlink, symlinkargs, void, void, RC_REPLSTAT),
--------------39EC06D5E22BA7150B0C6E18--