Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 23 Sep 2015 00:14:45 -0700
From:      Scott Long <scott4long@yahoo.com>
To:        Alan Cox <alc@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r288122 - in head/sys: kern vm
Message-ID:  <04359280-61C0-4A59-AF04-54FA3D4F98C3@yahoo.com>
In-Reply-To: <201509221816.t8MIGqxV069276@repo.freebsd.org>
References:  <201509221816.t8MIGqxV069276@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
It should be noted that Netflix has been running with an earlier version =
of this patch for nearly 10 months.

Scott

> On Sep 22, 2015, at 11:16 AM, Alan Cox <alc@freebsd.org> wrote:
>=20
> Author: alc
> Date: Tue Sep 22 18:16:52 2015
> New Revision: 288122
> URL: https://svnweb.freebsd.org/changeset/base/288122
>=20
> Log:
>  Change vm_page_unwire() such that it (1) accepts PQ_NONE as the =
specified
>  queue and (2) returns a Boolean indicating whether the page's wire =
count
>  transitioned to zero.
>=20
>  Exploit this change in vfs_vmio_release() to avoid pointlessly =
enqueueing
>  a page that is about to be freed.
>=20
>  (An earlier version of this change was developed by attilio@ and =
kmacy@.
>  Any errors in this version are my own.)
>=20
>  Reviewed by:	kib
>  Sponsored by:	EMC / Isilon Storage Division
>=20
> Modified:
>  head/sys/kern/vfs_bio.c
>  head/sys/vm/vm_page.c
>  head/sys/vm/vm_page.h
>=20
> Modified: head/sys/kern/vfs_bio.c
> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> --- head/sys/kern/vfs_bio.c	Tue Sep 22 17:34:51 2015	=
(r288121)
> +++ head/sys/kern/vfs_bio.c	Tue Sep 22 18:16:52 2015	=
(r288122)
> @@ -2076,6 +2076,7 @@ vfs_vmio_release(struct buf *bp)
> 	vm_object_t obj;
> 	vm_page_t m;
> 	int i;
> +	bool freed;
>=20
> 	if (buf_mapped(bp)) {
> 		BUF_CHECK_MAPPED(bp);
> @@ -2088,23 +2089,28 @@ vfs_vmio_release(struct buf *bp)
> 	for (i =3D 0; i < bp->b_npages; i++) {
> 		m =3D bp->b_pages[i];
> 		bp->b_pages[i] =3D NULL;
> -		/*
> -		 * In order to keep page LRU ordering consistent, put
> -		 * everything on the inactive queue.
> -		 */
> 		vm_page_lock(m);
> -		vm_page_unwire(m, PQ_INACTIVE);
> -
> -		/*
> -		 * Might as well free the page if we can and it has
> -		 * no valid data.  We also free the page if the
> -		 * buffer was used for direct I/O
> -		 */
> -		if ((bp->b_flags & B_ASYNC) =3D=3D 0 && !m->valid) {
> -			if (m->wire_count =3D=3D 0 && =
!vm_page_busied(m))
> -				vm_page_free(m);
> -		} else if (bp->b_flags & B_DIRECT)
> -			vm_page_try_to_free(m);
> +		if (vm_page_unwire(m, PQ_NONE)) {
> +			/*
> +			 * Determine if the page should be freed before =
adding
> +			 * it to the inactive queue.
> +			 */
> +			if ((bp->b_flags & B_ASYNC) =3D=3D 0 && m->valid =
=3D=3D 0) {
> +				freed =3D !vm_page_busied(m);
> +				if (freed)
> +					vm_page_free(m);
> +			} else if ((bp->b_flags & B_DIRECT) !=3D 0)
> +				freed =3D vm_page_try_to_free(m);
> +			else
> +				freed =3D false;
> +			if (!freed) {
> +				/*
> +				 * In order to maintain LRU page =
ordering, put
> +				 * the page at the tail of the inactive =
queue.
> +				 */
> +				vm_page_deactivate(m);
> +			}
> +		}
> 		vm_page_unlock(m);
> 	}
> 	if (obj !=3D NULL)
>=20
> Modified: head/sys/vm/vm_page.c
> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> --- head/sys/vm/vm_page.c	Tue Sep 22 17:34:51 2015	=
(r288121)
> +++ head/sys/vm/vm_page.c	Tue Sep 22 18:16:52 2015	=
(r288122)
> @@ -2476,42 +2476,46 @@ vm_page_wire(vm_page_t m)
> /*
>  * vm_page_unwire:
>  *
> - * Release one wiring of the specified page, potentially enabling it =
to be
> - * paged again.  If paging is enabled, then the value of the =
parameter
> - * "queue" determines the queue to which the page is added.
> - *
> - * However, unless the page belongs to an object, it is not enqueued =
because
> - * it cannot be paged out.
> + * Release one wiring of the specified page, potentially allowing it =
to be
> + * paged out.  Returns TRUE if the number of wirings transitions to =
zero and
> + * FALSE otherwise.
> + *
> + * Only managed pages belonging to an object can be paged out.  If =
the number
> + * of wirings transitions to zero and the page is eligible for page =
out, then
> + * the page is added to the specified paging queue (unless PQ_NONE is
> + * specified).
>  *
>  * If a page is fictitious, then its wire count must always be one.
>  *
>  * A managed page must be locked.
>  */
> -void
> +boolean_t
> vm_page_unwire(vm_page_t m, uint8_t queue)
> {
>=20
> -	KASSERT(queue < PQ_COUNT,
> +	KASSERT(queue < PQ_COUNT || queue =3D=3D PQ_NONE,
> 	    ("vm_page_unwire: invalid queue %u request for page %p",
> 	    queue, m));
> 	if ((m->oflags & VPO_UNMANAGED) =3D=3D 0)
> -		vm_page_lock_assert(m, MA_OWNED);
> +		vm_page_assert_locked(m);
> 	if ((m->flags & PG_FICTITIOUS) !=3D 0) {
> 		KASSERT(m->wire_count =3D=3D 1,
> 	    ("vm_page_unwire: fictitious page %p's wire count isn't =
one", m));
> -		return;
> +		return (FALSE);
> 	}
> 	if (m->wire_count > 0) {
> 		m->wire_count--;
> 		if (m->wire_count =3D=3D 0) {
> 			atomic_subtract_int(&vm_cnt.v_wire_count, 1);
> -			if ((m->oflags & VPO_UNMANAGED) !=3D 0 ||
> -			    m->object =3D=3D NULL)
> -				return;
> -			if (queue =3D=3D PQ_INACTIVE)
> -				m->flags &=3D ~PG_WINATCFLS;
> -			vm_page_enqueue(queue, m);
> -		}
> +			if ((m->oflags & VPO_UNMANAGED) =3D=3D 0 &&
> +			    m->object !=3D NULL && queue !=3D PQ_NONE) {
> +				if (queue =3D=3D PQ_INACTIVE)
> +					m->flags &=3D ~PG_WINATCFLS;
> +				vm_page_enqueue(queue, m);
> +			}
> +			return (TRUE);
> +		} else
> +			return (FALSE);
> 	} else
> 		panic("vm_page_unwire: page %p's wire count is zero", =
m);
> }
>=20
> Modified: head/sys/vm/vm_page.h
> =
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> --- head/sys/vm/vm_page.h	Tue Sep 22 17:34:51 2015	=
(r288121)
> +++ head/sys/vm/vm_page.h	Tue Sep 22 18:16:52 2015	=
(r288122)
> @@ -480,7 +480,7 @@ vm_offset_t vm_page_startup(vm_offset_t=20
> void vm_page_sunbusy(vm_page_t m);
> int vm_page_trysbusy(vm_page_t m);
> void vm_page_unhold_pages(vm_page_t *ma, int count);
> -void vm_page_unwire (vm_page_t m, uint8_t queue);
> +boolean_t vm_page_unwire(vm_page_t m, uint8_t queue);
> void vm_page_updatefake(vm_page_t m, vm_paddr_t paddr, vm_memattr_t =
memattr);
> void vm_page_wire (vm_page_t);
> void vm_page_xunbusy_hard(vm_page_t m);
>=20




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?04359280-61C0-4A59-AF04-54FA3D4F98C3>