From 2854a88cc4245e2bc1b92a21ab431517b136a51d Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Wed, 4 Nov 2020 13:51:40 -0800 Subject: [PATCH] kernel - Change pager interface to pass page index 2/2 * Adjust the DRM calls to vm_pager_get_page() to pass the page index. * Greatly simplify drm/linux_shmem.c and drm/ttm/ttm_bo_vm.c, removing the need to deal with placemarker pages for VM faults on OBJT_MGTDEVICE objects. --- sys/dev/drm/i915/i915_gem.c | 58 ++-------------------- sys/dev/drm/linux_shmem.c | 47 ++++++++++++------ sys/dev/drm/ttm/ttm_bo_vm.c | 98 ++++++------------------------------- sys/dev/drm/ttm/ttm_tt.c | 2 +- 4 files changed, 50 insertions(+), 155 deletions(-) diff --git a/sys/dev/drm/i915/i915_gem.c b/sys/dev/drm/i915/i915_gem.c index 8f47bab363..7a442b3e46 100644 --- a/sys/dev/drm/i915/i915_gem.c +++ b/sys/dev/drm/i915/i915_gem.c @@ -1887,12 +1887,8 @@ compute_partial_view(struct drm_i915_gem_object *obj, * * vm_obj is locked on entry and expected to be locked on return. * - * The vm_pager has placemarked the object with an anonymous memory page - * which we must replace atomically to avoid races against concurrent faults - * on the same page. XXX we currently are unable to do this atomically. - * - * If we are to return an error we should not touch the anonymous page, - * the caller will deallocate it. + * This is a OBJT_MGTDEVICE object, *mres will be NULL and should be set + * to the desired vm_page. The page is not indexed into the vm_obj. * * XXX Most GEM calls appear to be interruptable, but we can't hard loop * in that case. Release all resources and wait 1 tick before retrying. @@ -1935,7 +1931,6 @@ int i915_gem_fault(vm_object_t vm_obj, vm_ooffset_t offset, int prot, vm_page_t vm_page_t m; unsigned int flags; int ret; -#ifdef __DragonFly__ int didref = 0; struct vm_area_struct tmp_vm_area; struct vm_area_struct *area = &tmp_vm_area; @@ -1944,7 +1939,6 @@ int i915_gem_fault(vm_object_t vm_obj, vm_ooffset_t offset, int prot, vm_page_t area->vm_private_data = vm_obj->handle; area->vm_start = 0; area->vm_end = obj->base.size; -#endif /* We don't use vmf->pgoff since that has the fake offset */ page_offset = (unsigned long)offset >> PAGE_SHIFT; @@ -1958,16 +1952,7 @@ int i915_gem_fault(vm_object_t vm_obj, vm_ooffset_t offset, int prot, vm_page_t * and then dealing with the potential for a new placeholder when * we try to insert later. */ - if (*mres != NULL) { - m = *mres; - *mres = NULL; - if ((m->busy_count & PBUSY_LOCKED) == 0) - kprintf("i915_gem_fault: Page was not busy\n"); - else - vm_page_remove(m); - vm_page_free(m); - } - + KKASSERT(*mres == NULL); m = NULL; retry: @@ -2059,26 +2044,6 @@ retry: didref = 1; ret = 0; - m = NULL; - - /* - * Since the object lock was dropped, another thread might have - * faulted on the same GTT address and instantiated the mapping. - * Recheck. - */ - m = vm_page_lookup(vm_obj, OFF_TO_IDX(offset)); - if (m != NULL) { - /* - * Try to busy the page, retry on failure (non-zero ret). - */ - if (vm_page_busy_try(m, false)) { - kprintf("i915_gem_fault: BUSY\n"); - ret = -EINTR; - goto err_unpin; - } - goto have_page; - } - /* END FREEBSD MAGIC */ /* Mark as being mmapped into userspace for later revocation */ assert_rpm_wakelock_held(dev_priv); @@ -2105,23 +2070,6 @@ retry: goto err_unpin; } m->valid = VM_PAGE_BITS_ALL; - -#if 1 - /* - * This should always work since we already checked via a lookup - * above. - */ - if (vm_page_insert(m, vm_obj, OFF_TO_IDX(offset)) == FALSE) { - kprintf("i915:gem_fault: page %p,%jd already in object\n", - vm_obj, - OFF_TO_IDX(offset)); - vm_page_wakeup(m); - ret = -EINTR; - goto err_unpin; - } -#endif - -have_page: *mres = m; __i915_vma_unpin(vma); diff --git a/sys/dev/drm/linux_shmem.c b/sys/dev/drm/linux_shmem.c index 9f4d000b34..e5796d19d1 100644 --- a/sys/dev/drm/linux_shmem.c +++ b/sys/dev/drm/linux_shmem.c @@ -37,6 +37,11 @@ #include #include +/* + * This code is typically called with a normal VM object to access + * data from a userspace shared memory mapping. However, handle the + * case where it might be called with OBJT_MGTDEVICE anyway. + */ struct page * shmem_read_mapping_page(vm_object_t object, vm_pindex_t pindex) { @@ -44,24 +49,36 @@ shmem_read_mapping_page(vm_object_t object, vm_pindex_t pindex) int rv; VM_OBJECT_LOCK(object); - m = vm_page_grab(object, pindex, VM_ALLOC_NORMAL | VM_ALLOC_RETRY); - if (m->valid != VM_PAGE_BITS_ALL) { - if (vm_pager_has_page(object, pindex)) { - rv = vm_pager_get_page(object, &m, 1); - m = vm_page_lookup(object, pindex); - if (m == NULL) - return ERR_PTR(-ENOMEM); - if (rv != VM_PAGER_OK) { - vm_page_free(m); - return ERR_PTR(-ENOMEM); + if (object->type == OBJT_MGTDEVICE) { + m = NULL; + rv = vm_pager_get_page(object, pindex, &m, 1); + if (m == NULL) + return ERR_PTR(-ENOMEM); + if (rv != VM_PAGER_OK) { + vm_page_free(m); + return ERR_PTR(-ENOMEM); + } + } else { + m = vm_page_grab(object, pindex, + VM_ALLOC_NORMAL | VM_ALLOC_RETRY); + if (m->valid != VM_PAGE_BITS_ALL) { + if (vm_pager_has_page(object, pindex)) { + rv = vm_pager_get_page(object, pindex, &m, 1); + m = vm_page_lookup(object, pindex); + if (m == NULL) + return ERR_PTR(-ENOMEM); + if (rv != VM_PAGER_OK) { + vm_page_free(m); + return ERR_PTR(-ENOMEM); + } + } else { + pmap_zero_page(VM_PAGE_TO_PHYS(m)); + m->valid = VM_PAGE_BITS_ALL; + m->dirty = 0; } - } else { - pmap_zero_page(VM_PAGE_TO_PHYS(m)); - m->valid = VM_PAGE_BITS_ALL; - m->dirty = 0; } } - vm_page_wire(m); + vm_page_wire(m); /* put_page() undoes this */ vm_page_wakeup(m); VM_OBJECT_UNLOCK(object); diff --git a/sys/dev/drm/ttm/ttm_bo_vm.c b/sys/dev/drm/ttm/ttm_bo_vm.c index 9e9971221b..62b234cacb 100644 --- a/sys/dev/drm/ttm/ttm_bo_vm.c +++ b/sys/dev/drm/ttm/ttm_bo_vm.c @@ -431,7 +431,7 @@ ttm_bo_vm_fault_dfly(vm_object_t vm_obj, vm_ooffset_t offset, struct ttm_buffer_object *bo = vm_obj->handle; struct ttm_bo_device *bdev = bo->bdev; struct ttm_tt *ttm = NULL; - vm_page_t m, mtmp; + vm_page_t m; int ret; int retval = VM_PAGER_OK; struct ttm_mem_type_manager *man = @@ -456,30 +456,11 @@ ttm_bo_vm_fault_dfly(vm_object_t vm_obj, vm_ooffset_t offset, vm_object_pip_add(vm_obj, 1); /* - * We must atomically clean up any possible placeholder page to avoid - * the DRM subsystem attempting to use it. We can determine if this - * is a place holder page by checking m->valid. - * - * We have to do this before any potential fault_reserve_notify() - * which might try to free the map (and thus deadlock on our busy - * page). + * OBJT_MGTDEVICE does not pre-allocate the page. */ - m = *mres; - *mres = NULL; - if (m) { - if (m->valid == VM_PAGE_BITS_ALL) { - /* actual page */ - vm_page_wakeup(m); - } else { - /* placeholder page */ - KKASSERT((m->flags & PG_FICTITIOUS) == 0); - vm_page_remove(m); - vm_page_free(m); - } - } + KKASSERT(*mres == NULL); retry: - VM_OBJECT_UNLOCK(vm_obj); m = NULL; /* @@ -492,23 +473,17 @@ retry: if (unlikely(ret != 0)) { if (ret != -EBUSY) { retval = VM_PAGER_ERROR; - VM_OBJECT_LOCK(vm_obj); goto out_unlock2; } - if (vmf->flags & FAULT_FLAG_ALLOW_RETRY || 1) { + if ((vmf->flags & FAULT_FLAG_ALLOW_RETRY) || 1) { if (!(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) { up_read(&vma->vm_mm->mmap_sem); (void) ttm_bo_wait_unreserved(bo); } -#ifndef __DragonFly__ - return VM_FAULT_RETRY; -#else - VM_OBJECT_LOCK(vm_obj); lwkt_yield(); goto retry; -#endif } /* @@ -516,13 +491,8 @@ retry: * mmap_sem -> bo::reserve, we'd use a blocking reserve here * instead of retrying the fault... */ -#ifndef __DragonFly__ - return VM_FAULT_NOPAGE; -#else retval = VM_PAGER_ERROR; - VM_OBJECT_LOCK(vm_obj); goto out_unlock2; -#endif } if (bdev->driver->fault_reserve_notify) { @@ -536,10 +506,10 @@ retry: case -ERESTARTSYS: case -EINTR: retval = VM_PAGER_ERROR; - goto out_unlock; + goto out_unlock1; default: retval = VM_PAGER_ERROR; - goto out_unlock; + goto out_unlock1; } } @@ -549,26 +519,23 @@ retry: */ ret = ttm_bo_vm_fault_idle(bo, vma, vmf); if (unlikely(ret != 0)) { - retval = ret; -#ifdef __DragonFly__ retval = VM_PAGER_ERROR; -#endif - goto out_unlock; + goto out_unlock1; } ret = ttm_mem_io_lock(man, true); if (unlikely(ret != 0)) { retval = VM_PAGER_ERROR; - goto out_unlock; + goto out_unlock1; } ret = ttm_mem_io_reserve_vm(bo); if (unlikely(ret != 0)) { retval = VM_PAGER_ERROR; - goto out_io_unlock; + goto out_io_unlock1; } if (unlikely(OFF_TO_IDX(offset) >= bo->num_pages)) { retval = VM_PAGER_ERROR; - goto out_io_unlock; + goto out_io_unlock1; } /* @@ -615,21 +582,19 @@ retry: /* Allocate all page at once, most common usage */ if (ttm->bdev->driver->ttm_tt_populate(ttm)) { retval = VM_PAGER_ERROR; - goto out_io_unlock; + goto out_io_unlock1; } m = (struct vm_page *)ttm->pages[OFF_TO_IDX(offset)]; if (unlikely(!m)) { retval = VM_PAGER_ERROR; - goto out_io_unlock; + goto out_io_unlock1; } pmap_page_set_memattr(m, (bo->mem.placement & TTM_PL_FLAG_CACHED) ? VM_MEMATTR_WRITE_BACK : ttm_io_prot(bo->mem.placement, 0)); } - VM_OBJECT_LOCK(vm_obj); - if (vm_page_busy_try(m, FALSE)) { kprintf("r"); vm_page_sleep_busy(m, FALSE, "ttmvmf"); @@ -639,39 +604,12 @@ retry: } /* - * We want our fake page in the VM object, not the page the OS - * allocatedd for us as a placeholder. + * Return our fake page BUSYd. Do not index it into the VM object. + * The caller will enter it into the pmap. */ m->valid = VM_PAGE_BITS_ALL; *mres = m; - /* - * Insert the page into the object if not already inserted. - */ - if (m->object) { - if (m->object != vm_obj || m->pindex != OFF_TO_IDX(offset)) { - retval = VM_PAGER_ERROR; - kprintf("ttm_bo_vm_fault_dfly: m(%p) already inserted " - "in obj %p, attempt obj %p\n", - m, m->object, vm_obj); - while (drm_unstall == 0) { - tsleep(&retval, 0, "DEBUG", hz/10); - } - if (drm_unstall > 0) - --drm_unstall; - } - } else { - mtmp = vm_page_lookup(vm_obj, OFF_TO_IDX(offset)); - if (mtmp == NULL) { - vm_page_insert(m, vm_obj, OFF_TO_IDX(offset)); - } else { - panic("inconsistent insert bo %p m %p mtmp %p " - "offset %jx", - bo, m, mtmp, - (uintmax_t)offset); - } - } - out_io_unlock1: ttm_mem_io_unlock(man); out_unlock1: @@ -679,14 +617,6 @@ out_unlock1: out_unlock2: vm_object_pip_wakeup(vm_obj); return (retval); - -out_io_unlock: - VM_OBJECT_LOCK(vm_obj); - goto out_io_unlock1; - -out_unlock: - VM_OBJECT_LOCK(vm_obj); - goto out_unlock1; } static int diff --git a/sys/dev/drm/ttm/ttm_tt.c b/sys/dev/drm/ttm/ttm_tt.c index 2bfc6b362c..9940acf0be 100644 --- a/sys/dev/drm/ttm/ttm_tt.c +++ b/sys/dev/drm/ttm/ttm_tt.c @@ -304,7 +304,7 @@ int ttm_tt_swapin(struct ttm_tt *ttm) VM_ALLOC_RETRY); if (((struct vm_page *)from_page)->valid != VM_PAGE_BITS_ALL) { if (vm_pager_has_page(swap_storage, i)) { - if (vm_pager_get_page(swap_storage, + if (vm_pager_get_page(swap_storage, i, (struct vm_page **)&from_page, 1) != VM_PAGER_OK) { vm_page_free((struct vm_page *)from_page); ret = -EIO; -- 2.41.0