Subject: Re: kern/34959
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Julio M. Merino Vidal <jmmv@NetBSD.org>
List: netbsd-bugs
Date: 11/01/2006 19:45:02
The following reply was made to PR kern/34959; it has been noted by GNATS.

From: "Julio M. Merino Vidal" <jmmv@NetBSD.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/34959
Date: Wed, 1 Nov 2006 19:43:34 +0000

 Given that uvm_loanuobjpages is only used by the NFS code, I guess we can
 make some assumptions and avoid considering what I explained above a bug
 although it might have a tiny impact on performance when not using NFS.
 
 Please check out the patch below and specially read the big comment in
 it, because it explains the above comment in more detail.
 
 Thanks to mlelstv@ for some tips.
 
 
 
 Index: fs/tmpfs/tmpfs_vnops.c
 ===================================================================
 RCS file: /cvsroot/src/sys/fs/tmpfs/tmpfs_vnops.c,v
 retrieving revision 1.27
 diff -u -p -r1.27 tmpfs_vnops.c
 --- fs/tmpfs/tmpfs_vnops.c	30 Oct 2006 15:23:20 -0000	1.27
 +++ fs/tmpfs/tmpfs_vnops.c	1 Nov 2006 19:39:51 -0000
 @@ -1383,6 +1383,7 @@ tmpfs_getpages(void *v)
  	int flags = ((struct vop_getpages_args *)v)->a_flags;
  
  	int error;
 +	int i;
  	struct tmpfs_node *node;
  	struct uvm_object *uobj;
  	int npages = *count;
 @@ -1418,9 +1419,34 @@ tmpfs_getpages(void *v)
  
  	simple_unlock(&vp->v_interlock);
  
 +	/*
 +	 * Make sure that the array on which we will store the
 +	 * gotten pages is clean.  Otherwise uao_get (pointed to by
 +	 * the pgo_get below) gets confused and does not return the
 +	 * appropriate pages.
 +	 * 
 +	 * This problem arises specifically in nfsrv_read which calls
 +	 * uvm_loanuobjpages with a non-clean (uninitialized) vector
 +	 * in m.  I (jmmv) am not sure if there is any other case where
 +	 * this may happen; otherwise, the following could just become
 +	 * a sanity (DEBUG) check and the array cleaning could be put
 +	 * in nfsrv_read.
 +	 */
 +	for (i = 0; i < npages; i++)
 +		m[i] = NULL;
  	simple_lock(&uobj->vmobjlock);
  	error = (*uobj->pgops->pgo_get)(uobj, offset, m, &npages, centeridx,
 -	    access_type, advice, flags);
 +	    access_type, advice, flags | PGO_ALLPAGES);
 +#if defined(DEBUG)
 +	{
 +		/* Make sure that all the pages we return are valid. */
 +		int dbgi;
 +		if (error != 0) {
 +			for (dbgi = 0; dbgi < npages; dbgi++)
 +				KASSERT(m[dbgi] != NULL);
 +		}
 +	}
 +#endif
  
  	return error;
  }
 
 -- 
 Julio M. Merino Vidal <jmmv@NetBSD.org>