search for: vm_fault_retry

In every place where sb_start_pagefault was called now we must manage the error code and return VM_FAULT_RETRY. Signed-off-by: Marco Stornelli <marco.stornelli at gmail.com> --- fs/btrfs/inode.c | 4 +++- fs/buffer.c | 4 +++- fs/ext4/inode.c | 4 +++- fs/f2fs/file.c | 4 +++- fs/gfs2/file.c | 4 +++- fs/nilfs2/file.c | 4 +++- fs/ocfs2/mmap.c | 4 +++-...

...134 if (ret) 135 return ret; 136 137 nouveau_bo_del_io_reserve_lru(bo); 138 139 prot = vm_get_page_prot(vma->vm_flags); 140 ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); 141 if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) 142 return ret; ^^^^^^^^^^ Call dma_resv_unlock() before returning? 143 144 nouveau_bo_add_io_reserve_lru(bo); 145 146 dma_resv_unlock(bo->base.resv); 147...

...9 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -301,8 +301,10 @@ static int hmm_vma_do_fault(struct mm_walk *walk, unsigned long addr, flags |= hmm_vma_walk->block ? 0 : FAULT_FLAG_ALLOW_RETRY; flags |= write_fault ? FAULT_FLAG_WRITE : 0; ret = handle_mm_fault(vma, addr, flags); - if (ret & VM_FAULT_RETRY) + if (ret & VM_FAULT_RETRY) { + /* Note, handle_mm_fault did up_read(&mm->mmap_sem)) */ return -EAGAIN; + } if (ret & VM_FAULT_ERROR) { *pfn = range->values[HMM_PFN_ERROR]; return -EFAULT;

...uveau_bo_del_io_reserve_lru(bo); > prot = vm_get_page_prot(vma->vm_flags); > - ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT); > + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, true); > nouveau_bo_add_io_reserve_lru(bo); > if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) > return ret; > diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c > index 3fec3acdaf28..b21cf00ae162 100644 > --- a/drivers/gpu/drm/radeon/radeon_gem.c > +++ b/drivers/gpu/drm/radeon/r...

...VM_NUM_PREFAULT); >> >>>>> + ret = ttm_bo_vm_fault_reserved(vmf, prot, >> >>>> TTM_BO_VM_NUM_PREFAULT, >> >>>>> +true); >> >>>>> nouveau_bo_add_io_reserve_lru(bo); >> >>>>> if (ret == VM_FAULT_RETRY && !(vmf->flags & >> >>>> FAULT_FLAG_RETRY_NOWAIT)) >> >>>>> return ret; >> >>>>> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c >> >>>>> b/drivers/gpu/drm/radeon/radeon_gem.c >> &g...

...d can remove the + * device-exclusive entry and unmap the page. If the page is + * free the entry must have been removed already. + */ + if (!get_page_unless_zero(vmf->page)) + return 0; + + if (!folio_lock_or_retry(folio, vma->vm_mm, vmf->flags)) { + put_page(vmf->page); return VM_FAULT_RETRY; + } mmu_notifier_range_init_owner(&range, MMU_NOTIFY_EXCLUSIVE, 0, vma, vma->vm_mm, vmf->address & PAGE_MASK, (vmf->address & PAGE_MASK) + PAGE_SIZE, NULL); @@ -3637,6 +3648,7 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf) pte_unmap_un...

...;vm_flags); >>> - ret = ttm_bo_vm_fault_reserved(vmf, prot, >> TTM_BO_VM_NUM_PREFAULT); >>> + ret = ttm_bo_vm_fault_reserved(vmf, prot, >> TTM_BO_VM_NUM_PREFAULT, >>> +true); >>> nouveau_bo_add_io_reserve_lru(bo); >>> if (ret == VM_FAULT_RETRY && !(vmf->flags & >> FAULT_FLAG_RETRY_NOWAIT)) >>> return ret; >>> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c >>> b/drivers/gpu/drm/radeon/radeon_gem.c >>> index 3fec3acdaf28..b21cf00ae162 100644 >>> --- a/drivers/...

..._reserved(vmf, prot, >>>> TTM_BO_VM_NUM_PREFAULT); >>>>> + ret = ttm_bo_vm_fault_reserved(vmf, prot, >>>> TTM_BO_VM_NUM_PREFAULT, >>>>> +true); >>>>> nouveau_bo_add_io_reserve_lru(bo); >>>>> if (ret == VM_FAULT_RETRY && !(vmf->flags & >>>> FAULT_FLAG_RETRY_NOWAIT)) >>>>> return ret; >>>>> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c >>>>> b/drivers/gpu/drm/radeon/radeon_gem.c >>>>> index 3fec3acdaf28..b21cf0...

On Mon, Jul 22, 2019 at 3:14 PM Christoph Hellwig <hch at lst.de> wrote: > > We should not have two different error codes for the same condition. In > addition this really complicates the code due to the special handling of > EAGAIN that drops the mmap_sem due to the FAULT_FLAG_ALLOW_RETRY logic > in the core vm. > > Signed-off-by: Christoph Hellwig <hch at

...entry must + * have been removed already. If it happens to have already + * been re-allocated after being freed all we do is lock and + * unlock it. + */ + if (!folio_try_get(folio)) + return 0; + + if (!folio_lock_or_retry(folio, vma->vm_mm, vmf->flags)) { + folio_put(folio); return VM_FAULT_RETRY; + } mmu_notifier_range_init_owner(&range, MMU_NOTIFY_EXCLUSIVE, 0, vma->vm_mm, vmf->address & PAGE_MASK, (vmf->address & PAGE_MASK) + PAGE_SIZE, NULL); @@ -3577,6 +3590,7 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fault *vmf) pte_unmap_unlock(...

Hi Jérôme, Ben and Jason, below is a series against the hmm tree which fixes up the mmap_sem locking in nouveau and while at it also removes leftover legacy HMM APIs only used by nouveau. The first 4 patches are a bug fix for nouveau, which I suspect should go into this merge window even if the code is marked as staging, just to avoid people copying the breakage. Changes since v2: - new patch

...fcounts; all the refcounts are actually taken on the folio. So this should be: if (!folio_try_get(folio)) return 0; (you can fix up the comment yourself) > + if (!folio_lock_or_retry(folio, vma->vm_mm, vmf->flags)) { > + put_page(vmf->page); folio_put(folio); > return VM_FAULT_RETRY; > + } > mmu_notifier_range_init_owner(&range, MMU_NOTIFY_EXCLUSIVE, 0, vma, > vma->vm_mm, vmf->address & PAGE_MASK, > (vmf->address & PAGE_MASK) + PAGE_SIZE, NULL); > @@ -3637,6 +3648,7 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fau...

...ntry and unmap the page. If the page is > + * free the entry must have been removed already. > + */ > + if (!get_page_unless_zero(vmf->page)) > + return 0; > + > + if (!folio_lock_or_retry(folio, vma->vm_mm, vmf->flags)) { > + put_page(vmf->page); > return VM_FAULT_RETRY; > + } > mmu_notifier_range_init_owner(&range, MMU_NOTIFY_EXCLUSIVE, 0, vma, > vma->vm_mm, vmf->address & PAGE_MASK, > (vmf->address & PAGE_MASK) + PAGE_SIZE, NULL); > @@ -3637,6 +3648,7 @@ static vm_fault_t remove_device_exclusive_entry(struct vm_fau...

...+ vm_fault_t ret; > + > + ret = ttm_bo_vm_reserve(bo, vmf); > + if (ret) > + return ret; > + > + nouveau_bo_del_io_reserve_lru(bo); > + > + prot = vm_get_page_prot(vma->vm_flags); > + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT); > + if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) > + return ret; > + > + nouveau_bo_add_io_reserve_lru(bo); > + > + dma_resv_unlock(bo->base.resv); > + > + return ret; > +} > + > +static struct vm_operations_struct nouveau_ttm_vm_ops = { > + .fault = n...

...o_vm_reserve(bo, vmf); > + if (ret) > + return ret; > + > + nouveau_bo_del_io_reserve_lru(bo); > + > + prot = vm_get_page_prot(vma->vm_flags); > + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT); > + if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) > + return ret; > + > + nouveau_bo_add_io_reserve_lru(bo); > + > + dma_resv_unlock(bo->base.resv); > + > + return ret; > +} > + > +static struct vm_operations_struct nouveau_t...

Hi guys, I've already send this out in September last year, but only got a response from Daniel. Could you guys please test this and tell me what you think about it? Basically I'm trying to remove all driver specific features from TTM which don't need to be inside the framework. Thanks, Christian.

...t *bo = vma->vm_private_data; + pgprot_t prot; + vm_fault_t ret; + + ret = ttm_bo_vm_reserve(bo, vmf); + if (ret) + return ret; + + nouveau_bo_del_io_reserve_lru(bo); + + prot = vm_get_page_prot(vma->vm_flags); + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT); + if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) + return ret; + + nouveau_bo_add_io_reserve_lru(bo); + + dma_resv_unlock(bo->base.resv); + + return ret; +} + +static struct vm_operations_struct nouveau_ttm_vm_ops = { + .fault = nouveau_ttm_fault, + .open = ttm_bo_vm_open, + .close =...

...bo = vma->vm_private_data; + pgprot_t prot; + vm_fault_t ret; + + ret = ttm_bo_vm_reserve(bo, vmf); + if (ret) + return ret; + + nouveau_bo_del_io_reserve_lru(bo); + + prot = vm_get_page_prot(vma->vm_flags); + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1); + if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) + return ret; + + nouveau_bo_add_io_reserve_lru(bo); + + dma_resv_unlock(bo->base.resv); + + return ret; +} + +static struct vm_operations_struct nouveau_ttm_vm_ops = { + .fault = nouveau_ttm_fault, + .open = ttm_bo_vm_open, + .close =...

...out? Same problem really that ttm had (well still has I think for !amdgpu) with its other lru ... Or am I getting totally lost again here? -Daniel > + > + prot = vm_get_page_prot(vma->vm_flags); > + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT); > + if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) > + return ret; > + > + nouveau_bo_add_io_reserve_lru(bo); > + > + dma_resv_unlock(bo->base.resv); > + > + return ret; > +} > + > +static struct vm_operations_struct nouveau_ttm_vm_ops = { > + .fault = n...

...t *bo = vma->vm_private_data; + pgprot_t prot; + vm_fault_t ret; + + ret = ttm_bo_vm_reserve(bo, vmf); + if (ret) + return ret; + + nouveau_bo_del_io_reserve_lru(bo); + + prot = vm_get_page_prot(vma->vm_flags); + ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT); + if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT)) + return ret; + + nouveau_bo_add_io_reserve_lru(bo); + + dma_resv_unlock(bo->base.resv); + + return ret; +} + +static struct vm_operations_struct nouveau_ttm_vm_ops = { + .fault = nouveau_ttm_fault, + .open = ttm_bo_vm_open, + .close =...