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>
index | next in thread | previous in thread | raw e-mail
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: > > Author: alc > Date: Tue Sep 22 18:16:52 2015 > New Revision: 288122 > URL: https://svnweb.freebsd.org/changeset/base/288122 > > 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. > > Exploit this change in vfs_vmio_release() to avoid pointlessly enqueueing > a page that is about to be freed. > > (An earlier version of this change was developed by attilio@ and kmacy@. > Any errors in this version are my own.) > > Reviewed by: kib > Sponsored by: EMC / Isilon Storage Division > > Modified: > head/sys/kern/vfs_bio.c > head/sys/vm/vm_page.c > head/sys/vm/vm_page.h > > Modified: head/sys/kern/vfs_bio.c > ============================================================================== > --- 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; > > if (buf_mapped(bp)) { > BUF_CHECK_MAPPED(bp); > @@ -2088,23 +2089,28 @@ vfs_vmio_release(struct buf *bp) > for (i = 0; i < bp->b_npages; i++) { > m = bp->b_pages[i]; > bp->b_pages[i] = 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) == 0 && !m->valid) { > - if (m->wire_count == 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) == 0 && m->valid == 0) { > + freed = !vm_page_busied(m); > + if (freed) > + vm_page_free(m); > + } else if ((bp->b_flags & B_DIRECT) != 0) > + freed = vm_page_try_to_free(m); > + else > + freed = 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 != NULL) > > Modified: head/sys/vm/vm_page.c > ============================================================================== > --- 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) > { > > - KASSERT(queue < PQ_COUNT, > + KASSERT(queue < PQ_COUNT || queue == PQ_NONE, > ("vm_page_unwire: invalid queue %u request for page %p", > queue, m)); > if ((m->oflags & VPO_UNMANAGED) == 0) > - vm_page_lock_assert(m, MA_OWNED); > + vm_page_assert_locked(m); > if ((m->flags & PG_FICTITIOUS) != 0) { > KASSERT(m->wire_count == 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 == 0) { > atomic_subtract_int(&vm_cnt.v_wire_count, 1); > - if ((m->oflags & VPO_UNMANAGED) != 0 || > - m->object == NULL) > - return; > - if (queue == PQ_INACTIVE) > - m->flags &= ~PG_WINATCFLS; > - vm_page_enqueue(queue, m); > - } > + if ((m->oflags & VPO_UNMANAGED) == 0 && > + m->object != NULL && queue != PQ_NONE) { > + if (queue == PQ_INACTIVE) > + m->flags &= ~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); > } > > Modified: head/sys/vm/vm_page.h > ============================================================================== > --- 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 > 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); >help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?04359280-61C0-4A59-AF04-54FA3D4F98C3>