On Wed, May 10, 2023 at 11:00 AM Felix Kuehling <felix.kuehling@xxxxxxx> wrote:
>
> Am 2023-05-09 um 18:17 schrieb Alex Deucher:
> > From: Xiaogang Chen <xiaogang.chen@xxxxxxx>
> >
> > mmu notifier does not always hold mm->sem during call back. That causes
> > a race condition between kfd userprt buffer mapping and mmu notifier
> > which leds to gpu shadder or SDMA access userptr buffer before it has been
> > mapped to gpu VM. Always map userptr buffer to avoid that though it may make
> > some userprt buffers mapped two times.
> >
> > Suggested-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>
> > Signed-off-by: Xiaogang Chen <xiaogang.chen@xxxxxxx>
> > Reviewed-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>
> > Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
>
> This patch is no longer needed and should not be applied. It was
> originally applied to amd-staging-drm-next as patch
> fcf00f8d29f2fc6bf00531a1447be28b99073cc3 in November 2022. This fixed a
> race condition due to incorrect assumptions about the mmap lock and MMU
> notifiers. This hunk was added back by my later patch f95f51a4c335
> ("drm/amdgpu: Add notifier lock for KFD userptrs") in December, using
> our own notifier lock that doesn't suffer from those races.
>
Thanks. Dropped.
Alex
> Regards,
> Felix
>
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 10 ----------
> > 1 file changed, 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > index 58a774647573..40078c0a5585 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> > @@ -1942,16 +1942,6 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
> > */
> > mutex_lock(&mem->process_info->lock);
> >
> > - /* Lock notifier lock. If we find an invalid userptr BO, we can be
> > - * sure that the MMU notifier is no longer running
> > - * concurrently and the queues are actually stopped
> > - */
> > - if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm)) {
> > - mutex_lock(&mem->process_info->notifier_lock);
> > - is_invalid_userptr = !!mem->invalid;
> > - mutex_unlock(&mem->process_info->notifier_lock);
> > - }
> > -
> > mutex_lock(&mem->lock);
> >
> > domain = mem->domain;
