Hi Christian, Thanks for your review. BTW, Have you pushed the previous patches in all-open branch? I didn't find them. Or any concern? So we may use these patch on topic branch only for now. Regards, Jerry (Junwei Zhang) Linux Base Graphics SRDC Software Development _____________________________________ > -----Original Message----- > From: Christian König [mailto:deathsimple at vodafone.de] > Sent: Thursday, March 16, 2017 17:11 > To: Zhang, Jerry; amd-gfx at lists.freedesktop.org > Subject: Re: [PATCH v2] drm/amdgpu: fix before and after mapping judgement > for replace mapping > > Am 16.03.2017 um 09:13 schrieb Junwei Zhang: > > If the before mapping is 1 page size, so its start and last will be same. > > Thus below condition will become false, then to free the before mapping. > > > if (before->it.start != before->it.last) But in this case, we > > need the before mapping of 1 page size. > > So does after mapping. > > > > Signed-off-by: Junwei Zhang <Jerry.Zhang at amd.com> > > Reviewed-by: Christian König <christian.koenig at amd.com> > > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 10 ++++++---- > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > index f7c02a9..e95adec 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > @@ -1748,12 +1748,14 @@ int amdgpu_vm_bo_clear_mappings(struct > amdgpu_device *adev, > > before = kzalloc(sizeof(*before), GFP_KERNEL); > > if (!before) > > return -ENOMEM; > > + INIT_LIST_HEAD(&before->list); > > > > after = kzalloc(sizeof(*after), GFP_KERNEL); > > if (!after) { > > kfree(before); > > return -ENOMEM; > > } > > + INIT_LIST_HEAD(&after->list); > > > > /* Now gather all removed mappings */ > > it = interval_tree_iter_first(&vm->va, saddr, eaddr); @@ -1763,7 > > +1765,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev, > > > > /* Remember mapping split at the start */ > > if (tmp->it.start < saddr) { > > - before->it.start = tmp->it.start;; > > + before->it.start = tmp->it.start; > > before->it.last = saddr - 1; > > before->offset = tmp->offset; > > before->flags = tmp->flags; > > @@ -1799,8 +1801,8 @@ int amdgpu_vm_bo_clear_mappings(struct > amdgpu_device *adev, > > trace_amdgpu_vm_bo_unmap(NULL, tmp); > > } > > > > - /* Insert partial mapping before the range*/ > > - if (before->it.start != before->it.last) { > > + /* Insert partial mapping before the range */ > > + if (!list_empty(&before->list)) { > > interval_tree_insert(&before->it, &vm->va); > > if (before->flags & AMDGPU_PTE_PRT) > > amdgpu_vm_prt_get(adev); > > @@ -1809,7 +1811,7 @@ int amdgpu_vm_bo_clear_mappings(struct > amdgpu_device *adev, > > } > > > > /* Insert partial mapping after the range */ > > - if (after->it.start != after->it.last) { > > + if (!list_empty(&after->list)) { > > interval_tree_insert(&after->it, &vm->va); > > if (after->flags & AMDGPU_PTE_PRT) > > amdgpu_vm_prt_get(adev); >
