Re: [PATCH] drm/amdkfd: fix svm_migrate_fini warning

[Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>]

In any case, once you converge on solution please include
the relevant ticket in the commit description  -

https://gitlab.freedesktop.org/drm/amd/-/issues/1718

Andrey

On 2021-09-20 10:20 p.m., Felix Kuehling wrote:
> Am 2021-09-20 um 5:55 p.m. schrieb Philip Yang:
> > Don't use devm_request_free_mem_region to alloc VRAM region for pgmap,
> devm_... refers to a device manager that automatically releases
> device-specific resources when a driver disconnects from a device. So
> maybe that just means, our devm_memunmap_pages and
> devm_release_mem_region calls in svm_migrate_fini are redundant, and the
> best solution is to remove those calls.
>
> See Documentation/driver-api/driver-model/devres.rst
>
> Regards,
>    Felix
>
>
> > because driver__detach releases all device resource region, then calls
> > amdgpu_device_fini_sw, which calls devm_memunmap_pages generating below
> > warning trace:
> >
> > WARNING: CPU: 1 PID: 3646 at drivers/base/devres.c:795
> > devm_release_action+0x51/0x60
> > Call Trace:
> >      ? memunmap_pages+0x360/0x360
> >      svm_migrate_fini+0x2d/0x60 [amdgpu]
> >      kgd2kfd_device_exit+0x23/0xa0 [amdgpu]
> >      amdgpu_amdkfd_device_fini_sw+0x1d/0x30 [amdgpu]
> >      amdgpu_device_fini_sw+0x45/0x290 [amdgpu]
> >      amdgpu_driver_release_kms+0x12/0x30 [amdgpu]
> >      drm_dev_release+0x20/0x40 [drm]
> >      release_nodes+0x196/0x1e0
> >      device_release_driver_internal+0x104/0x1d0
> >      driver_detach+0x47/0x90
> >      bus_remove_driver+0x7a/0xd0
> >      pci_unregister_driver+0x3d/0x90
> >      amdgpu_exit+0x11/0x20 [amdgpu]
> > Trying to free nonexistent resource <000007fc00000000-000007fdffffffff>
> >
> > Signed-off-by: Philip Yang <Philip.Yang@xxxxxxx>
> > ---
> >   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 13 ++++++-------
> >   1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> > index 21f745e0b86c..aa96767920a9 100644
> > --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> > @@ -880,7 +880,7 @@ int svm_migrate_init(struct amdgpu_device *adev)
> >   	 * should remove reserved size
> >   	 */
> >   	size = ALIGN(adev->gmc.real_vram_size, 2ULL << 20);
> > -	res = devm_request_free_mem_region(adev->dev, &iomem_resource, size);
> > +	res = request_free_mem_region(&iomem_resource, size, "amdgpu_vram");
> >   	if (IS_ERR(res))
> >   		return -ENOMEM;
> >   
> > @@ -891,14 +891,13 @@ int svm_migrate_init(struct amdgpu_device *adev)
> >   	pgmap->ops = &svm_migrate_pgmap_ops;
> >   	pgmap->owner = SVM_ADEV_PGMAP_OWNER(adev);
> >   	pgmap->flags = MIGRATE_VMA_SELECT_DEVICE_PRIVATE;
> > -	r = devm_memremap_pages(adev->dev, pgmap);
> > +	r = memremap_pages(pgmap, dev_to_node(adev->dev));
> >   	if (IS_ERR(r)) {
> >   		pr_err("failed to register HMM device memory\n");
> >   
> >   		/* Disable SVM support capability */
> >   		pgmap->type = 0;
> > -		devm_release_mem_region(adev->dev, res->start,
> > -					res->end - res->start + 1);
> > +		release_mem_region(res->start, res->end - res->start + 1);
> >   		return PTR_ERR(r);
> >   	}
> >   
> > @@ -919,7 +918,7 @@ void svm_migrate_fini(struct amdgpu_device *adev)
> >   	if (!KFD_IS_SVM_API_SUPPORTED(adev->kfd.dev))
> >   		return;
> >   
> > -	devm_memunmap_pages(adev->dev, pgmap);
> > -	devm_release_mem_region(adev->dev, pgmap->range.start,
> > -				pgmap->range.end - pgmap->range.start + 1);
> > +	memunmap_pages(pgmap);
> > +	release_mem_region(pgmap->range.start,
> > +			   pgmap->range.end - pgmap->range.start + 1);
> >   }