svn commit: r358451 - in head/sys: kern vm
Gleb Smirnoff
glebius at freebsd.org
Thu Mar 5 00:36:39 UTC 2020
Hi Jeff,
may be I'm missing something, but from a quick review this change doesn't
seem to be correct for sendfile with INVARIANTS on.
sendfile_swapin does immediate vm_page_xunbusy() for all valid pages
that are substituted to bogus one. With this change KASSERT in
vm_page_relookup() would fire:
KASSERT(m != NULL && vm_page_busied(m) &&
On Fri, Feb 28, 2020 at 09:42:48PM +0000, Jeff Roberson wrote:
J> Author: jeff
J> Date: Fri Feb 28 21:42:48 2020
J> New Revision: 358451
J> URL: https://svnweb.freebsd.org/changeset/base/358451
J>
J> Log:
J> Provide a lock free alternative to resolve bogus pages. This is not likely
J> to be much of a perf win, just a nice code simplification.
J>
J> Reviewed by: markj, kib
J> Differential Revision: https://reviews.freebsd.org/D23866
J>
J> Modified:
J> head/sys/kern/kern_sendfile.c
J> head/sys/kern/vfs_bio.c
J> head/sys/vm/vm_page.c
J> head/sys/vm/vm_page.h
J>
J> Modified: head/sys/kern/kern_sendfile.c
J> ==============================================================================
J> --- head/sys/kern/kern_sendfile.c Fri Feb 28 21:31:40 2020 (r358450)
J> +++ head/sys/kern/kern_sendfile.c Fri Feb 28 21:42:48 2020 (r358451)
J> @@ -350,7 +350,6 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> {
J> vm_page_t *pa = sfio->pa;
J> int grabbed;
J> - bool locked;
J>
J> *nios = 0;
J> flags = (flags & SF_NODISKIO) ? VM_ALLOC_NOWAIT : 0;
J> @@ -359,8 +358,6 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> * First grab all the pages and wire them. Note that we grab
J> * only required pages. Readahead pages are dealt with later.
J> */
J> - locked = false;
J> -
J> grabbed = vm_page_grab_pages_unlocked(obj, OFF_TO_IDX(off),
J> VM_ALLOC_NORMAL | VM_ALLOC_WIRED | flags, pa, npages);
J> if (grabbed < npages) {
J> @@ -381,10 +378,6 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> i++;
J> continue;
J> }
J> - if (!locked) {
J> - VM_OBJECT_WLOCK(obj);
J> - locked = true;
J> - }
J>
J> /*
J> * Next page is invalid. Check if it belongs to pager. It
J> @@ -396,8 +389,10 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> * stored in 'a', about how many pages we can pagein after
J> * this page in a single I/O.
J> */
J> + VM_OBJECT_RLOCK(obj);
J> if (!vm_pager_has_page(obj, OFF_TO_IDX(vmoff(i, off)), NULL,
J> &a)) {
J> + VM_OBJECT_RUNLOCK(obj);
J> pmap_zero_page(pa[i]);
J> vm_page_valid(pa[i]);
J> MPASS(pa[i]->dirty == 0);
J> @@ -405,6 +400,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> i++;
J> continue;
J> }
J> + VM_OBJECT_RUNLOCK(obj);
J>
J> /*
J> * We want to pagein as many pages as possible, limited only
J> @@ -435,11 +431,9 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> }
J>
J> refcount_acquire(&sfio->nios);
J> - VM_OBJECT_WUNLOCK(obj);
J> rv = vm_pager_get_pages_async(obj, pa + i, count, NULL,
J> i + count == npages ? &rhpages : NULL,
J> &sendfile_iodone, sfio);
J> - VM_OBJECT_WLOCK(obj);
J> if (__predict_false(rv != VM_PAGER_OK)) {
J> /*
J> * Perform full pages recovery before returning EIO.
J> @@ -451,7 +445,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> for (j = 0; j < npages; j++) {
J> if (j > i && j < i + count - 1 &&
J> pa[j] == bogus_page)
J> - pa[j] = vm_page_lookup(obj,
J> + pa[j] = vm_page_relookup(obj,
J> OFF_TO_IDX(vmoff(j, off)));
J> else if (j >= i)
J> vm_page_xunbusy(pa[j]);
J> @@ -460,7 +454,6 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> __func__, pa, j));
J> vm_page_unwire(pa[j], PQ_INACTIVE);
J> }
J> - VM_OBJECT_WUNLOCK(obj);
J> return (EIO);
J> }
J>
J> @@ -475,7 +468,7 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> */
J> for (j = i + 1; j < i + count - 1; j++)
J> if (pa[j] == bogus_page) {
J> - pa[j] = vm_page_lookup(obj,
J> + pa[j] = vm_page_relookup(obj,
J> OFF_TO_IDX(vmoff(j, off)));
J> KASSERT(pa[j], ("%s: page %p[%d] disappeared",
J> __func__, pa, j));
J> @@ -484,9 +477,6 @@ sendfile_swapin(vm_object_t obj, struct sf_io *sfio, i
J> i += count;
J> (*nios)++;
J> }
J> -
J> - if (locked)
J> - VM_OBJECT_WUNLOCK(obj);
J>
J> if (*nios == 0 && npages != 0)
J> SFSTAT_INC(sf_noiocnt);
J>
J> Modified: head/sys/kern/vfs_bio.c
J> ==============================================================================
J> --- head/sys/kern/vfs_bio.c Fri Feb 28 21:31:40 2020 (r358450)
J> +++ head/sys/kern/vfs_bio.c Fri Feb 28 21:42:48 2020 (r358451)
J> @@ -2878,11 +2878,8 @@ vfs_vmio_iodone(struct buf *bp)
J> */
J> m = bp->b_pages[i];
J> if (m == bogus_page) {
J> - if (bogus == false) {
J> - bogus = true;
J> - VM_OBJECT_RLOCK(obj);
J> - }
J> - m = vm_page_lookup(obj, OFF_TO_IDX(foff));
J> + bogus = true;
J> + m = vm_page_relookup(obj, OFF_TO_IDX(foff));
J> if (m == NULL)
J> panic("biodone: page disappeared!");
J> bp->b_pages[i] = m;
J> @@ -2905,8 +2902,6 @@ vfs_vmio_iodone(struct buf *bp)
J> foff = (foff + PAGE_SIZE) & ~(off_t)PAGE_MASK;
J> iosize -= resid;
J> }
J> - if (bogus)
J> - VM_OBJECT_RUNLOCK(obj);
J> vm_object_pip_wakeupn(obj, bp->b_npages);
J> if (bogus && buf_mapped(bp)) {
J> BUF_CHECK_MAPPED(bp);
J> @@ -4470,22 +4465,16 @@ vfs_unbusy_pages(struct buf *bp)
J> int i;
J> vm_object_t obj;
J> vm_page_t m;
J> - bool bogus;
J>
J> runningbufwakeup(bp);
J> if (!(bp->b_flags & B_VMIO))
J> return;
J>
J> obj = bp->b_bufobj->bo_object;
J> - bogus = false;
J> for (i = 0; i < bp->b_npages; i++) {
J> m = bp->b_pages[i];
J> if (m == bogus_page) {
J> - if (bogus == false) {
J> - bogus = true;
J> - VM_OBJECT_RLOCK(obj);
J> - }
J> - m = vm_page_lookup(obj, OFF_TO_IDX(bp->b_offset) + i);
J> + m = vm_page_relookup(obj, OFF_TO_IDX(bp->b_offset) + i);
J> if (!m)
J> panic("vfs_unbusy_pages: page missing\n");
J> bp->b_pages[i] = m;
J> @@ -4498,8 +4487,6 @@ vfs_unbusy_pages(struct buf *bp)
J> }
J> vm_page_sunbusy(m);
J> }
J> - if (bogus)
J> - VM_OBJECT_RUNLOCK(obj);
J> vm_object_pip_wakeupn(obj, bp->b_npages);
J> }
J>
J>
J> Modified: head/sys/vm/vm_page.c
J> ==============================================================================
J> --- head/sys/vm/vm_page.c Fri Feb 28 21:31:40 2020 (r358450)
J> +++ head/sys/vm/vm_page.c Fri Feb 28 21:42:48 2020 (r358451)
J> @@ -1671,6 +1671,24 @@ vm_page_lookup(vm_object_t object, vm_pindex_t pindex)
J> }
J>
J> /*
J> + * vm_page_relookup:
J> + *
J> + * Returns a page that must already have been busied by
J> + * the caller. Used for bogus page replacement.
J> + */
J> +vm_page_t
J> +vm_page_relookup(vm_object_t object, vm_pindex_t pindex)
J> +{
J> + vm_page_t m;
J> +
J> + m = vm_radix_lookup_unlocked(&object->rtree, pindex);
J> + KASSERT(m != NULL && vm_page_busied(m) &&
J> + m->object == object && m->pindex == pindex,
J> + ("vm_page_relookup: Invalid page %p", m));
J> + return (m);
J> +}
J> +
J> +/*
J> * This should only be used by lockless functions for releasing transient
J> * incorrect acquires. The page may have been freed after we acquired a
J> * busy lock. In this case busy_lock == VPB_FREED and we have nothing
J>
J> Modified: head/sys/vm/vm_page.h
J> ==============================================================================
J> --- head/sys/vm/vm_page.h Fri Feb 28 21:31:40 2020 (r358450)
J> +++ head/sys/vm/vm_page.h Fri Feb 28 21:42:48 2020 (r358451)
J> @@ -653,6 +653,7 @@ void vm_page_reference(vm_page_t m);
J> #define VPR_NOREUSE 0x02
J> void vm_page_release(vm_page_t m, int flags);
J> void vm_page_release_locked(vm_page_t m, int flags);
J> +vm_page_t vm_page_relookup(vm_object_t, vm_pindex_t);
J> bool vm_page_remove(vm_page_t);
J> bool vm_page_remove_xbusy(vm_page_t);
J> int vm_page_rename(vm_page_t, vm_object_t, vm_pindex_t);
J> _______________________________________________
J> svn-src-all at freebsd.org mailing list
J> https://lists.freebsd.org/mailman/listinfo/svn-src-all
J> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"
--
Gleb Smirnoff
More information about the svn-src-head
mailing list