Re: 答复: [PATCH] drm/amdkfd: use vma_lookup() instead of find_vma()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Am 2022-10-17 um 20:47 schrieb tomorrow Wang (王德明):
Hi,
The function vma_lookup show below.  Vma valid check is included in it. Or, What other questions do you have?

My question is, why did you leave the find_vma call in svm_range_is_valid unchanged? I don't see a technical reason, but maybe I'm missing something. If there is a reason, please explain. If there is no reason, please fix that place as well for consistency.

Thanks,
  Felix



static inline
struct vm_area_struct *vma_lookup(struct mm_struct *mm, unsigned long addr)
  {
          struct vm_area_struct *vma = find_vma(mm, addr);

          if (vma && addr < vma->vm_start)
                  vma = NULL;

          return vma;
  }


from: Felix Kuehling <felix.kuehling@xxxxxxx>
time: 2022年10月18日 3:35
to: tomorrow Wang (王德明) <wangdeming@xxxxxxxxxx>;
airlied@xxxxxxxxx; daniel@xxxxxxxx; alexander.deucher@xxxxxxx;
christian.koenig@xxxxxxx; Xinhui.Pan@xxxxxxx
linux-kernel@xxxxxxxxxxxxxxx
sub: Re: [PATCH] drm/amdkfd: use vma_lookup() instead of find_vma()


On 2022-10-06 22:48, Deming Wang wrote:
Using vma_lookup() verifies the start address is contained in the
found vma.  This results in easier to read the code.
Thank you for the patches. This and your other patch look good to me.
However, you missed one use of find_vma in svm_range_is_valid. Is that an
oversight or is there a reason why we need to use find_vma there?

If you're going to respin it, you may also squash the two patches into one.

Thanks,
    Felix


Signed-off-by: Deming Wang <wangdeming@xxxxxxxxxx>
---
   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 12 ++++++------
   1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 64fdf63093a0..cabcc2ca3c23 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -1586,8 +1586,8 @@ static int svm_range_validate_and_map(struct
mm_struct *mm,
   		unsigned long npages;
   		bool readonly;

-		vma = find_vma(mm, addr);
-		if (!vma || addr < vma->vm_start) {
+		vma = vma_lookup(mm, addr);
+		if (!vma) {
   			r = -EFAULT;
   			goto unreserve_out;
   		}
@@ -2542,8 +2542,8 @@ svm_range_get_range_boundaries(struct
kfd_process *p, int64_t addr,
   	struct interval_tree_node *node;
   	unsigned long start_limit, end_limit;

-	vma = find_vma(p->mm, addr << PAGE_SHIFT);
-	if (!vma || (addr << PAGE_SHIFT) < vma->vm_start) {
+	vma = vma_lookup(p->mm, addr << PAGE_SHIFT);
+	if (!vma) {
   		pr_debug("VMA does not exist in address [0x%llx]\n", addr);
   		return -EFAULT;
   	}
@@ -2871,8 +2871,8 @@ svm_range_restore_pages(struct amdgpu_device
*adev, unsigned int pasid,
   	/* __do_munmap removed VMA, return success as we are handling stale
   	 * retry fault.
   	 */
-	vma = find_vma(mm, addr << PAGE_SHIFT);
-	if (!vma || (addr << PAGE_SHIFT) < vma->vm_start) {
+	vma = vma_lookup(mm, addr << PAGE_SHIFT);
+	if (!vma) {
   		pr_debug("address 0x%llx VMA is removed\n", addr);
   		r = 0;
   		goto out_unlock_range;



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux