In article <200607201616.38982.mark@mcs.vuw.ac.nz>,
Mark Davies  <mark@mcs.vuw.ac.nz> wrote:
>A couple of weeks ago I posted to current-users about one of my servers that 
>kept locking up every few days. 
>(http://mail-index.netbsd.org/current-users/2006/07/06/0007.html)
>After some very useful assistance by Juergen Hannken-Illjes we've tracked down 
>that the actual problem is a memory leak in nfsrv_readdir().  It looks like 
>this leak has been there since Lite2 was merged in 8 years ago.
>
>Whats happening is that nfsrv_readdir() calls VOP_READDIR which malloc's some 
>space to hold the directory seek cookies.  Normally the cookies are freed at 
>the end of the routine but somtimes the code jumps back to the "again" label 
>calling VOP_READDIR again so mallocing new cookies without ever freeing the 
>old ones.
>
>The below patch fixes the problem in the same way as FreeBSD does.  OK to 
>commit?
>
>I still don't know what it is about that particular server of mine that is 
>tickling the bug much more frequently than other boxes.

You don't need the caddr_t cast, and isn't it more efficient to free
the cookies before the statement 'goto again' unconditionally, and without
setting cookies to NULL? Yes, it is not as safe, but there is only
one goto again in each function (readdir/readdirplus)...

christos

>cheers
>mark
>
>Index: nfs_serv.c
>===================================================================
>RCS file: /src/cvs/netbsd/src/sys/nfs/nfs_serv.c,v
>retrieving revision 1.98
>diff -u -r1.98 nfs_serv.c
>--- nfs_serv.c	6 Oct 2005 10:23:01 -0000	1.98
>+++ nfs_serv.c	20 Jul 2006 03:35:09 -0000
>@@ -2668,6 +2668,11 @@
> 	eofflag = 0;
> 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
> 
>+	if (cookies) {
>+		free((caddr_t)cookies, M_TEMP);
>+		cookies = NULL;
>+	}
>+
> 	error = VOP_READDIR(vp, &io, cred, &eofflag, &cookies, &ncookies);
> 
> 	off = (off_t)io.uio_offset;
>@@ -2928,6 +2933,11 @@
> 
> 	vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
> 
>+	if (cookies) {
>+		free((caddr_t)cookies, M_TEMP);
>+		cookies = NULL;
>+	}
>+
> 	error = VOP_READDIR(vp, &io, cred, &eofflag, &cookies, &ncookies);
> 
> 	off = (u_quad_t)io.uio_offset;
>