Download raw body.
pgo_fault & VM_PAGER_* values
On 05/12/24(Thu) 00:22, Theo Buehler wrote:
> On Wed, Dec 04, 2024 at 10:52:59AM +0100, Martin Pieuchot wrote:
> > Similar to the previous change in uvmfault_anonget(), the diff below
> > gets rid of the VM_PAGER_* values and use errno values instead in all
> > the pgo_fault() handlers.
> >
> > The goal of this change is to reduce the differences with NetBSD's UVM
> > and then to return different error codes.
> >
> > I've annotated return values that are currently EACCES but could be
> > something else.
> >
> > While here grab the KERNEL_LOCK() after the UVM object lock to reduce
> > contention.
>
> This generally looks good to me - for a while I thought the diff was
> incomplete since I looked at the wrong version of some functions under
> #ifdef __linux__
>
> I dislike that in a few handlers the vm_fault_t ret changes from holding
> a VM_* code to a type holding an errno. But that's C being the terrible
> type unsafe language that it is.
>
> In udv_fault() there is a retval = VM_PAGER_OK that should be retval = 0.
>
> In ttm_bo_vm_fault() you deduplicated the return value handling of
> ttm_bo_vm_fault() with the exit path, which you didn't do in the other
> functions. I would either undo that or do it for all the functions, e.g.,
> in amdgpu_gem_fault() do
>
> unlock:
> dma_resv_unlock(bo->base.resv);
> out:
> switch (ret) {
> case VM_FAULT_NOPAGE:
> ...
Updated diff below that addresses both issues.
ok?
Index: dev/pci/drm/drm_gem.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/drm_gem.c,v
diff -u -p -r1.23 drm_gem.c
--- dev/pci/drm/drm_gem.c 16 Jan 2024 23:37:51 -0000 1.23
+++ dev/pci/drm/drm_gem.c 9 Dec 2024 09:56:24 -0000
@@ -102,10 +102,9 @@ drm_fault(struct uvm_faultinfo *ufi, vad
* we do not allow device mappings to be mapped copy-on-write
* so we kill any attempt to do so here.
*/
-
if (UVM_ET_ISCOPYONWRITE(entry)) {
uvmfault_unlockall(ufi, ufi->entry->aref.ar_amap, uobj);
- return(VM_PAGER_ERROR);
+ return EACCES;
}
/*
@@ -125,7 +124,7 @@ drm_fault(struct uvm_faultinfo *ufi, vad
PZERO, "drmflt", INFSLP);
}
mtx_leave(&dev->quiesce_mtx);
- return(VM_PAGER_REFAULT);
+ return ERESTART;
}
dev->quiesce_count++;
mtx_leave(&dev->quiesce_mtx);
@@ -141,7 +140,7 @@ drm_fault(struct uvm_faultinfo *ufi, vad
wakeup(&dev->quiesce_count);
mtx_leave(&dev->quiesce_mtx);
- return (ret);
+ return ret;
}
boolean_t
Index: dev/pci/drm/drm_gem_dma_helper.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/drm_gem_dma_helper.c,v
diff -u -p -r1.4 drm_gem_dma_helper.c
--- dev/pci/drm/drm_gem_dma_helper.c 10 Nov 2024 06:51:59 -0000 1.4
+++ dev/pci/drm/drm_gem_dma_helper.c 9 Dec 2024 09:56:24 -0000
@@ -208,7 +208,7 @@ drm_gem_dma_fault(struct drm_gem_object
paddr = bus_dmamem_mmap(obj->dmat, obj->dmasegs, 1,
offset, access_type, BUS_DMA_NOCACHE);
if (paddr == -1) {
- retval = VM_PAGER_BAD;
+ retval = EACCES;
break;
}
@@ -218,7 +218,7 @@ drm_gem_dma_fault(struct drm_gem_object
uvmfault_unlockall(ufi, ufi->entry->aref.ar_amap,
uobj);
uvm_wait("drm_gem_dma_fault");
- return VM_PAGER_REFAULT;
+ return ERESTART;
}
}
Index: dev/pci/drm/amd/amdgpu/amdgpu_gem.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/amd/amdgpu/amdgpu_gem.c,v
diff -u -p -r1.10 amdgpu_gem.c
--- dev/pci/drm/amd/amdgpu/amdgpu_gem.c 16 Jan 2024 23:37:52 -0000 1.10
+++ dev/pci/drm/amd/amdgpu/amdgpu_gem.c 9 Dec 2024 09:59:49 -0000
@@ -99,19 +99,7 @@ amdgpu_gem_fault(struct uvm_faultinfo *u
ret = ttm_bo_vm_reserve(bo);
if (ret) {
- switch (ret) {
- case VM_FAULT_NOPAGE:
- ret = VM_PAGER_OK;
- break;
- case VM_FAULT_RETRY:
- ret = VM_PAGER_REFAULT;
- break;
- default:
- ret = VM_PAGER_BAD;
- break;
- }
- uvmfault_unlockall(ufi, NULL, uobj);
- return ret;
+ goto out;
}
if (drm_dev_enter(ddev, &idx)) {
@@ -137,18 +125,19 @@ amdgpu_gem_fault(struct uvm_faultinfo *u
#endif
unlock:
+ dma_resv_unlock(bo->base.resv);
+out:
switch (ret) {
case VM_FAULT_NOPAGE:
- ret = VM_PAGER_OK;
+ ret = 0;
break;
case VM_FAULT_RETRY:
- ret = VM_PAGER_REFAULT;
+ ret = ERESTART;
break;
default:
- ret = VM_PAGER_BAD;
+ ret = EACCES;
break;
}
- dma_resv_unlock(bo->base.resv);
uvmfault_unlockall(ufi, NULL, uobj);
return ret;
}
Index: dev/pci/drm/i915/gem/i915_gem_mman.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_mman.c,v
diff -u -p -r1.20 i915_gem_mman.c
--- dev/pci/drm/i915/gem/i915_gem_mman.c 19 Aug 2024 11:18:29 -0000 1.20
+++ dev/pci/drm/i915/gem/i915_gem_mman.c 9 Dec 2024 09:56:24 -0000
@@ -591,10 +591,10 @@ static int i915_error_to_vmf_fault(int e
case -EFAULT: /* purged object */
case -ENODEV: /* bad object, how did you get here! */
case -ENXIO: /* unable to access backing store (on device) */
- return VM_PAGER_ERROR;
+ return EACCES;
case -ENOMEM: /* our allocation failure */
- return VM_PAGER_ERROR;
+ return EACCES; /* XXX */
case 0:
case -EAGAIN:
@@ -607,7 +607,7 @@ static int i915_error_to_vmf_fault(int e
* EBUSY is ok: this just means that another thread
* already did the job.
*/
- return VM_PAGER_OK;
+ return 0;
}
}
@@ -629,11 +629,11 @@ vm_fault_cpu(struct i915_mmap_offset *mm
/* Sanity check that we allow writing into this object */
if (unlikely(i915_gem_object_is_readonly(obj) && write)) {
uvmfault_unlockall(ufi, NULL, &obj->base.uobj);
- return VM_PAGER_BAD;
+ return EACCES;
}
if (i915_gem_object_lock_interruptible(obj, NULL))
- return VM_PAGER_ERROR;
+ return EACCES;
err = i915_gem_object_pin_pages(obj);
if (err)
@@ -921,7 +921,7 @@ i915_gem_fault(struct drm_gem_object *ge
drm_vma_offset_unlock_lookup(dev->vma_offset_manager);
if (!mmo) {
uvmfault_unlockall(ufi, NULL, &gem_obj->uobj);
- return VM_PAGER_BAD;
+ return EACCES;
}
KASSERT(gem_obj == &mmo->obj->base);
Index: dev/pci/drm/i915/gem/i915_gem_ttm.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/i915/gem/i915_gem_ttm.c,v
diff -u -p -r1.8 i915_gem_ttm.c
--- dev/pci/drm/i915/gem/i915_gem_ttm.c 11 Oct 2024 02:48:48 -0000 1.8
+++ dev/pci/drm/i915/gem/i915_gem_ttm.c 9 Dec 2024 09:56:24 -0000
@@ -1240,20 +1240,20 @@ vm_fault_ttm(struct uvm_faultinfo *ufi,
/* Sanity check that we allow writing into this object */
if (unlikely(i915_gem_object_is_readonly(obj) && write)) {
uvmfault_unlockall(ufi, NULL, &obj->base.uobj);
- return VM_PAGER_BAD;
+ return EACCES;
}
ret = ttm_bo_vm_reserve(bo);
if (ret) {
switch (ret) {
case VM_FAULT_NOPAGE:
- ret = VM_PAGER_OK;
+ ret = 0;
break;
case VM_FAULT_RETRY:
- ret = VM_PAGER_REFAULT;
+ ret = ERESTART;
break;
default:
- ret = VM_PAGER_BAD;
+ ret = EACCES;
break;
}
uvmfault_unlockall(ufi, NULL, &obj->base.uobj);
@@ -1263,7 +1263,7 @@ vm_fault_ttm(struct uvm_faultinfo *ufi,
if (obj->mm.madv != I915_MADV_WILLNEED) {
dma_resv_unlock(bo->base.resv);
uvmfault_unlockall(ufi, NULL, &obj->base.uobj);
- return VM_PAGER_BAD;
+ return EACCES;
}
/*
@@ -1285,7 +1285,7 @@ vm_fault_ttm(struct uvm_faultinfo *ufi,
if (err) {
dma_resv_unlock(bo->base.resv);
uvmfault_unlockall(ufi, NULL, &obj->base.uobj);
- return VM_PAGER_BAD;
+ return EACCES;
}
} else if (!i915_ttm_resource_mappable(bo->resource)) {
int err = -ENODEV;
@@ -1359,13 +1359,13 @@ vm_fault_ttm(struct uvm_faultinfo *ufi,
out_rpm:
switch (ret) {
case VM_FAULT_NOPAGE:
- ret = VM_PAGER_OK;
+ ret = 0;
break;
case VM_FAULT_RETRY:
- ret = VM_PAGER_REFAULT;
+ ret = ERESTART;
break;
default:
- ret = VM_PAGER_BAD;
+ ret = EACCES;
break;
}
Index: dev/pci/drm/radeon/radeon_gem.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/radeon/radeon_gem.c,v
diff -u -p -r1.19 radeon_gem.c
--- dev/pci/drm/radeon/radeon_gem.c 26 Jul 2024 03:42:02 -0000 1.19
+++ dev/pci/drm/radeon/radeon_gem.c 9 Dec 2024 09:56:24 -0000
@@ -115,13 +115,13 @@ unlock_resv:
unlock_mclk:
switch (ret) {
case VM_FAULT_NOPAGE:
- ret = VM_PAGER_OK;
+ ret = 0;
break;
case VM_FAULT_RETRY:
- ret = VM_PAGER_REFAULT;
+ ret = ERESTART;
break;
default:
- ret = VM_PAGER_BAD;
+ ret = EACCES;
break;
}
up_read(&rdev->pm.mclk_lock);
Index: dev/pci/drm/ttm/ttm_bo_vm.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/ttm/ttm_bo_vm.c,v
diff -u -p -r1.32 ttm_bo_vm.c
--- dev/pci/drm/ttm/ttm_bo_vm.c 16 Jan 2024 23:38:14 -0000 1.32
+++ dev/pci/drm/ttm/ttm_bo_vm.c 9 Dec 2024 09:56:24 -0000
@@ -598,37 +598,23 @@ ttm_bo_vm_fault(struct uvm_faultinfo *uf
ret = ttm_bo_vm_reserve(bo);
if (ret) {
- switch (ret) {
- case VM_FAULT_NOPAGE:
- ret = VM_PAGER_OK;
- break;
- case VM_FAULT_RETRY:
- ret = VM_PAGER_REFAULT;
- break;
- default:
- ret = VM_PAGER_BAD;
- break;
- }
-
- uvmfault_unlockall(ufi, NULL, uobj);
- return ret;
+ goto out;
}
ret = ttm_bo_vm_fault_reserved(ufi, vaddr, TTM_BO_VM_NUM_PREFAULT, 1);
+ dma_resv_unlock(bo->base.resv);
+out:
switch (ret) {
case VM_FAULT_NOPAGE:
- ret = VM_PAGER_OK;
+ ret = 0;
break;
case VM_FAULT_RETRY:
- ret = VM_PAGER_REFAULT;
+ ret = ERESTART;
break;
default:
- ret = VM_PAGER_BAD;
+ ret = EACCES;
break;
}
-
- dma_resv_unlock(bo->base.resv);
-
uvmfault_unlockall(ufi, NULL, uobj);
return ret;
}
Index: uvm/uvm_device.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_device.c,v
diff -u -p -r1.67 uvm_device.c
--- uvm/uvm_device.c 24 Jul 2024 12:15:55 -0000 1.67
+++ uvm/uvm_device.c 9 Dec 2024 09:57:12 -0000
@@ -331,7 +331,7 @@ udv_fault(struct uvm_faultinfo *ufi, vad
*/
if (UVM_ET_ISCOPYONWRITE(entry)) {
uvmfault_unlockall(ufi, ufi->entry->aref.ar_amap, uobj);
- return(VM_PAGER_ERROR);
+ return EACCES;
}
/*
@@ -354,7 +354,7 @@ udv_fault(struct uvm_faultinfo *ufi, vad
/*
* loop over the page range entering in as needed
*/
- retval = VM_PAGER_OK;
+ retval = 0;
for (lcv = 0 ; lcv < npages ; lcv++, curr_offset += PAGE_SIZE,
curr_va += PAGE_SIZE) {
if ((flags & PGO_ALLPAGES) == 0 && lcv != centeridx)
@@ -365,7 +365,7 @@ udv_fault(struct uvm_faultinfo *ufi, vad
paddr = (*mapfn)(device, curr_offset, access_type);
if (paddr == -1) {
- retval = VM_PAGER_ERROR;
+ retval = EACCES; /* XXX */
break;
}
mapprot = ufi->entry->protection;
@@ -387,11 +387,11 @@ udv_fault(struct uvm_faultinfo *ufi, vad
/* sync what we have so far */
pmap_update(ufi->orig_map->pmap);
uvm_wait("udv_fault");
- return (VM_PAGER_REFAULT);
+ return ERESTART;
}
}
uvmfault_unlockall(ufi, ufi->entry->aref.ar_amap, uobj);
pmap_update(ufi->orig_map->pmap);
- return (retval);
+ return retval;
}
Index: uvm/uvm_fault.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
diff -u -p -r1.152 uvm_fault.c
--- uvm/uvm_fault.c 4 Dec 2024 09:21:06 -0000 1.152
+++ uvm/uvm_fault.c 9 Dec 2024 09:56:24 -0000
@@ -621,20 +621,13 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
* providing a pgo_fault routine.
*/
if (uobj != NULL && uobj->pgops->pgo_fault != NULL) {
- KERNEL_LOCK();
rw_enter(uobj->vmobjlock, RW_WRITE);
+ KERNEL_LOCK();
error = uobj->pgops->pgo_fault(&ufi,
flt.startva, pages, flt.npages,
flt.centeridx, fault_type, flt.access_type,
PGO_LOCKED);
KERNEL_UNLOCK();
-
- if (error == VM_PAGER_OK)
- error = 0;
- else if (error == VM_PAGER_REFAULT)
- error = ERESTART;
- else
- error = EACCES;
} else {
/* case 2: fault on backing obj or zero fill */
error = uvm_fault_lower(&ufi, &flt, pages);
pgo_fault & VM_PAGER_* values