Subject: Re: port-alpha/35448: memory management fault trap during heavy network I/O
To: None <mhitch@NetBSD.org, gnats-admin@netbsd.org,>
From: Christian Biere <christianbiere@gmx.de>
List: netbsd-bugs
Date: 02/01/2007 02:40:02
The following reply was made to PR port-alpha/35448; it has been noted by GNATS.

From: Christian Biere <christianbiere@gmx.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: port-alpha/35448: memory management fault trap during heavy network I/O
Date: Thu, 1 Feb 2007 03:37:52 +0100

 Michael L. Hitch wrote:
 >     Changing the UIO_ADVANCE() to a UIO_RETREAT() which passed 'backup' 
 >  directly and subtracted that from iov_base, and added it to iov_len gave 
 >  me a kernel which did not crash when nfs_writerpc() resent the data.  I've 
 >  also just verified that simply making 'backup' a signed 32 bit also works 
 >  using the UIO_ADVANCE() macro.
 
 I'd prefer the former because it's cleaner. I take "advance" as a strong
 emphasis that it's meant to move forward. At least I've written a similar
 function before and decided against the more flexible term "add" for
 exactly this reason.
 
 
 --- sys/nfs/nfs_vnops.c.orig	2007-01-26 21:52:50.000000000 +0100
 +++ sys/nfs/nfs_vnops.c	2007-02-01 03:21:00.000000000 +0100
 @@ -251,10 +251,22 @@ extern const nfstype nfsv3_type[9];
  
  int nfs_numasync = 0;
  #define	DIRHDSIZ	_DIRENT_NAMEOFF(dp)
 -#define UIO_ADVANCE(uio, siz) \
 -    (void)((uio)->uio_resid -= (siz), \
 -    (uio)->uio_iov->iov_base = (char *)(uio)->uio_iov->iov_base + (siz), \
 -    (uio)->uio_iov->iov_len -= (siz))
 +
 +static __inline void
 +UIO_ADVANCE(struct uio *uio, size_t n)
 +{
 +	uio->uio_resid -= n;
 +	uio->uio_iov->iov_base = (char *)uio->uio_iov->iov_base + n;
 +    	uio->uio_iov->iov_len -= n;
 +}
 +
 +static __inline void
 +UIO_RETREAT(struct uio *uio, size_t n)
 +{
 +	uio->uio_resid += n;
 +	uio->uio_iov->iov_base = (char *)uio->uio_iov->iov_base - n;
 +    	uio->uio_iov->iov_len += n;
 +}
  
  static void nfs_cache_enter(struct vnode *, struct vnode *,
      struct componentname *);
 @@ -1420,7 +1432,7 @@ retry:
  					break;
  				} else if (rlen < len) {
  					backup = len - rlen;
 -					UIO_ADVANCE(uiop, -backup);
 +					UIO_RETREAT(uiop, backup);
  					uiop->uio_offset -= backup;
  					len = rlen;
  				}
 @@ -1482,7 +1494,7 @@ retry:
  				 * then, we should resend them to nfsd.
  				 */
  				backup = origresid - tsiz;
 -				UIO_ADVANCE(uiop, -backup);
 +				UIO_RETREAT(uiop, backup);
  				uiop->uio_offset -= backup;
  				tsiz = origresid;
  				goto retry;