Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 22 Jan 2020 19:33:00 -1000 (HST)
From:      Jeff Roberson <jroberson@jroberson.net>
To:        Jeff Roberson <jeff@FreeBSD.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r357028 - head/sys/vm
Message-ID:  <alpine.BSF.2.21.9999.2001221925100.1198@desktop>
In-Reply-To: <202001230523.00N5NbbB078486@repo.freebsd.org>
References:  <202001230523.00N5NbbB078486@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
You may be asking yourself, why did jeff just squish a bunch of code 
around with little functional change.  I could not justify adding more 
complexity to a 900 line function with a handful of gotos.  It is ~300 
lines now.  I tested the whole set together.  I apologize if there are 
bugs.

Other than comment cleanup, I will probably not commit again for a little 
while.  The commits that follow will enable lockless vm object lookup 
which has given a 10x improvement in massively parallel builds on large 
machines (100core+).

Thanks,
Jeff

On Thu, 23 Jan 2020, Jeff Roberson wrote:

> Author: jeff
> Date: Thu Jan 23 05:23:37 2020
> New Revision: 357028
> URL: https://svnweb.freebsd.org/changeset/base/357028
>
> Log:
>  (fault 9/9) Move zero fill into a dedicated function to make the object lock
>  state more clear.
>
>  Reviewed by:	kib
>  Differential Revision:	https://reviews.freebsd.org/D23326
>
> Modified:
>  head/sys/vm/vm_fault.c
>
> Modified: head/sys/vm/vm_fault.c
> ==============================================================================
> --- head/sys/vm/vm_fault.c	Thu Jan 23 05:22:02 2020	(r357027)
> +++ head/sys/vm/vm_fault.c	Thu Jan 23 05:23:37 2020	(r357028)
> @@ -960,35 +960,8 @@ vm_fault_next(struct faultstate *fs)
> 	 */
> 	VM_OBJECT_ASSERT_WLOCKED(fs->object);
> 	next_object = fs->object->backing_object;
> -	if (next_object == NULL) {
> -		/*
> -		 * If there's no object left, fill the page in the top
> -		 * object with zeros.
> -		 */
> -		VM_OBJECT_WUNLOCK(fs->object);
> -		if (fs->object != fs->first_object) {
> -			vm_object_pip_wakeup(fs->object);
> -			fs->object = fs->first_object;
> -			fs->pindex = fs->first_pindex;
> -		}
> -		MPASS(fs->first_m != NULL);
> -		MPASS(fs->m == NULL);
> -		fs->m = fs->first_m;
> -		fs->first_m = NULL;
> -
> -		/*
> -		 * Zero the page if necessary and mark it valid.
> -		 */
> -		if ((fs->m->flags & PG_ZERO) == 0) {
> -			pmap_zero_page(fs->m);
> -		} else {
> -			VM_CNT_INC(v_ozfod);
> -		}
> -		VM_CNT_INC(v_zfod);
> -		vm_page_valid(fs->m);
> -
> +	if (next_object == NULL)
> 		return (false);
> -	}
> 	MPASS(fs->first_m != NULL);
> 	KASSERT(fs->object != next_object, ("object loop %p", next_object));
> 	VM_OBJECT_WLOCK(next_object);
> @@ -1002,6 +975,36 @@ vm_fault_next(struct faultstate *fs)
> 	return (true);
> }
>
> +static void
> +vm_fault_zerofill(struct faultstate *fs)
> +{
> +
> +	/*
> +	 * If there's no object left, fill the page in the top
> +	 * object with zeros.
> +	 */
> +	if (fs->object != fs->first_object) {
> +		vm_object_pip_wakeup(fs->object);
> +		fs->object = fs->first_object;
> +		fs->pindex = fs->first_pindex;
> +	}
> +	MPASS(fs->first_m != NULL);
> +	MPASS(fs->m == NULL);
> +	fs->m = fs->first_m;
> +	fs->first_m = NULL;
> +
> +	/*
> +	 * Zero the page if necessary and mark it valid.
> +	 */
> +	if ((fs->m->flags & PG_ZERO) == 0) {
> +		pmap_zero_page(fs->m);
> +	} else {
> +		VM_CNT_INC(v_ozfod);
> +	}
> +	VM_CNT_INC(v_zfod);
> +	vm_page_valid(fs->m);
> +}
> +
> /*
>  * Allocate a page directly or via the object populate method.
>  */
> @@ -1407,11 +1410,13 @@ RetryFault:
> 		 * traverse into a backing object or zero fill if none is
> 		 * found.
> 		 */
> -		if (!vm_fault_next(&fs)) {
> -			/* Don't try to prefault neighboring pages. */
> -			faultcount = 1;
> -			break;	/* break to PAGE HAS BEEN FOUND. */
> -		}
> +		if (vm_fault_next(&fs))
> +			continue;
> +		VM_OBJECT_WUNLOCK(fs.object);
> +		vm_fault_zerofill(&fs);
> +		/* Don't try to prefault neighboring pages. */
> +		faultcount = 1;
> +		break;	/* break to PAGE HAS BEEN FOUND. */
> 	}
>
> 	/*
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.BSF.2.21.9999.2001221925100.1198>