On Wed, Aug 26, 2026 at 04:56:04PM +0200, Boris Brezillon wrote: > There's no clue that we'll ever have an SoC with multiple Mali GPUs, > and even then, the gains of sharing the PT caches and cleanup workqueue > remains unclear. > > On the other hand, binding those resources to the device allows us to > detect leaks or UAF at drm_device removal time instead of when the > module is removed. > > Signed-off-by: Boris Brezillon <[email protected]>
Reviewed-by: Liviu Dudau <[email protected]> Best regards, Liviu > --- > drivers/gpu/drm/panthor/panthor_device.c | 17 ++++--- > drivers/gpu/drm/panthor/panthor_device.h | 8 ++++ > drivers/gpu/drm/panthor/panthor_drv.c | 47 +------------------ > drivers/gpu/drm/panthor/panthor_mmu.c | 78 > +++++++++++++++----------------- > drivers/gpu/drm/panthor/panthor_mmu.h | 3 -- > drivers/gpu/drm/panthor/panthor_sched.c | 4 +- > 6 files changed, 58 insertions(+), 99 deletions(-) > > diff --git a/drivers/gpu/drm/panthor/panthor_device.c > b/drivers/gpu/drm/panthor/panthor_device.c > index 133e3895cd0a..08bb4d410941 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.c > +++ b/drivers/gpu/drm/panthor/panthor_device.c > @@ -131,12 +131,9 @@ static void panthor_device_unplug_work(struct > work_struct *work) > panthor_device_unplug(ptdev); > } > > -static void panthor_device_reset_cleanup(struct drm_device *ddev, void *data) > +static void destroy_wq(struct drm_device *ddev, void *wq) > { > - struct panthor_device *ptdev = container_of(ddev, struct > panthor_device, base); > - > - disable_work_sync(&ptdev->reset.work); > - destroy_workqueue(ptdev->reset.wq); > + destroy_workqueue(wq); > } > > static void panthor_device_reset_work(struct work_struct *work) > @@ -242,7 +239,15 @@ int panthor_device_init(struct panthor_device *ptdev) > if (!ptdev->reset.wq) > return -ENOMEM; > > - ret = drmm_add_action_or_reset(&ptdev->base, > panthor_device_reset_cleanup, NULL); > + ret = drmm_add_action_or_reset(&ptdev->base, destroy_wq, > ptdev->reset.wq); > + if (ret) > + return ret; > + > + ptdev->cleanup_wq = alloc_workqueue("panthor-cleanup", WQ_UNBOUND, 0); > + if (!ptdev->cleanup_wq) > + return -ENOMEM; > + > + ret = drmm_add_action_or_reset(&ptdev->base, destroy_wq, > ptdev->cleanup_wq); > if (ret) > return ret; > > diff --git a/drivers/gpu/drm/panthor/panthor_device.h > b/drivers/gpu/drm/panthor/panthor_device.h > index 217eec811bdb..a7ea475fb407 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.h > +++ b/drivers/gpu/drm/panthor/panthor_device.h > @@ -327,6 +327,14 @@ struct panthor_device { > atomic_t recovery_needed; > } pm; > > + /** > + * @cleanup_wq: Workqueue used for cleanup operations. > + * > + * We create a dedicated workqueue so we can flush on unplug and > + * make sure all resources are freed before we finish the unplug. > + */ > + struct workqueue_struct *cleanup_wq; > + > /** @profile_mask: User-set profiling flags for job accounting. */ > u32 profile_mask; > > diff --git a/drivers/gpu/drm/panthor/panthor_drv.c > b/drivers/gpu/drm/panthor/panthor_drv.c > index 46a3080b0b20..67db6701699b 100644 > --- a/drivers/gpu/drm/panthor/panthor_drv.c > +++ b/drivers/gpu/drm/panthor/panthor_drv.c > @@ -1904,52 +1904,7 @@ static struct platform_driver panthor_driver = { > .dev_groups = panthor_groups, > }, > }; > - > -/* > - * Workqueue used to cleanup stuff. > - * > - * We create a dedicated workqueue so we can drain on unplug and > - * make sure all resources are freed before the module is unloaded. > - */ > -struct workqueue_struct *panthor_cleanup_wq; > - > -static int __init panthor_init(void) > -{ > - int ret; > - > - ret = panthor_mmu_pt_cache_init(); > - if (ret) > - return ret; > - > - panthor_cleanup_wq = alloc_workqueue("panthor-cleanup", WQ_UNBOUND, 0); > - if (!panthor_cleanup_wq) { > - pr_err("panthor: Failed to allocate the workqueues"); > - ret = -ENOMEM; > - goto err_mmu_pt_cache_fini; > - } > - > - ret = platform_driver_register(&panthor_driver); > - if (ret) > - goto err_destroy_cleanup_wq; > - > - return 0; > - > -err_destroy_cleanup_wq: > - destroy_workqueue(panthor_cleanup_wq); > - > -err_mmu_pt_cache_fini: > - panthor_mmu_pt_cache_fini(); > - return ret; > -} > -module_init(panthor_init); > - > -static void __exit panthor_exit(void) > -{ > - platform_driver_unregister(&panthor_driver); > - destroy_workqueue(panthor_cleanup_wq); > - panthor_mmu_pt_cache_fini(); > -} > -module_exit(panthor_exit); > +module_platform_driver(panthor_driver); > > MODULE_AUTHOR("Panthor Project Developers"); > MODULE_DESCRIPTION("Panthor DRM Driver"); > diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c > b/drivers/gpu/drm/panthor/panthor_mmu.c > index 1385ee28bee5..b4e41556247a 100644 > --- a/drivers/gpu/drm/panthor/panthor_mmu.c > +++ b/drivers/gpu/drm/panthor/panthor_mmu.c > @@ -61,6 +61,16 @@ struct panthor_mmu { > /** @irq: The MMU irq. */ > struct panthor_irq irq; > > + /** > + * @pt_cache: Cache used to allocate MMU page tables. > + * > + * The pre-allocation pattern forces us to over-allocate to plan for > + * the worst case scenario, and return the pages we didn't use. > + * > + * Having a kmem_cache allows us to speed allocations. > + */ > + struct kmem_cache *pt_cache; > + > /** > * @as: Address space related fields. > * > @@ -440,16 +450,6 @@ struct panthor_vm_bind_job { > struct panthor_vm_op_ctx ctx; > }; > > -/* > - * @pt_cache: Cache used to allocate MMU page tables. > - * > - * The pre-allocation pattern forces us to over-allocate to plan for > - * the worst case scenario, and return the pages we didn't use. > - * > - * Having a kmem_cache allows us to speed allocations. > - */ > -static struct kmem_cache *pt_cache; > - > /** > * alloc_pt() - Custom page table allocator > * @cookie: Cookie passed at page table allocation time. > @@ -530,7 +530,7 @@ static void free_pt(void *cookie, void *data, size_t size) > return; > > /* Return the page to the pt_cache. */ > - kmem_cache_free(pt_cache, data); > + kmem_cache_free(vm->ptdev->mmu->pt_cache, data); > } > > static int wait_ready(struct panthor_device *ptdev, u32 as_nr) > @@ -1179,7 +1179,8 @@ static void panthor_vm_cleanup_op_ctx(struct > panthor_vm_op_ctx *op_ctx, > !op_ctx->map.bo; > > if (remaining_pt_count) { > - kmem_cache_free_bulk(pt_cache, remaining_pt_count, > + kmem_cache_free_bulk(vm->ptdev->mmu->pt_cache, > + remaining_pt_count, > op_ctx->rsvd_page_tables.pages + > op_ctx->rsvd_page_tables.ptr); > } > @@ -1279,7 +1280,8 @@ static void panthor_vm_init_op_ctx(struct > panthor_vm_op_ctx *op_ctx, > op_ctx->va.addr = va; > } > > -static int panthor_vm_op_ctx_prealloc_pts(struct panthor_vm_op_ctx *op_ctx) > +static int panthor_vm_op_ctx_prealloc_pts(struct panthor_device *ptdev, > + struct panthor_vm_op_ctx *op_ctx) > { > u64 size = op_ctx->va.range; > u64 va = op_ctx->va.addr; > @@ -1298,7 +1300,7 @@ static int panthor_vm_op_ctx_prealloc_pts(struct > panthor_vm_op_ctx *op_ctx) > if (!op_ctx->rsvd_page_tables.pages) > return -ENOMEM; > > - if (!kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count, > + if (!kmem_cache_alloc_bulk(ptdev->mmu->pt_cache, GFP_KERNEL, pt_count, > op_ctx->rsvd_page_tables.pages)) { > op_ctx->rsvd_page_tables.count = 0; > return -ENOMEM; > @@ -1385,7 +1387,7 @@ static int panthor_vm_prepare_map_op_ctx(struct > panthor_vm_op_ctx *op_ctx, > op_ctx->map.vm_bo = drm_gpuvm_bo_obtain_prealloc(preallocated_vm_bo); > op_ctx->map.bo_offset = op->bo_offset; > > - ret = panthor_vm_op_ctx_prealloc_pts(op_ctx); > + ret = panthor_vm_op_ctx_prealloc_pts(vm->ptdev, op_ctx); > if (ret) > goto err_cleanup; > > @@ -1444,8 +1446,9 @@ static int panthor_vm_prepare_unmap_op_ctx(struct > panthor_vm_op_ctx *op_ctx, > goto err_cleanup; > } > > - if (!kmem_cache_alloc_bulk(pt_cache, GFP_KERNEL, pt_count, > - op_ctx->rsvd_page_tables.pages)) { > + if (!kmem_cache_alloc_bulk(vm->ptdev->mmu->pt_cache, > + GFP_KERNEL, pt_count, > + op_ctx->rsvd_page_tables.pages)) { > ret = -ENOMEM; > goto err_cleanup; > } > @@ -2580,7 +2583,7 @@ static int remap_evicted_vma(struct drm_gpuvm_bo *vm_bo, > bool found = false; > int ret; > > - ret = panthor_vm_op_ctx_prealloc_pts(op_ctx); > + ret = panthor_vm_op_ctx_prealloc_pts(vm->ptdev, op_ctx); > if (ret) > goto out_cleanup; > > @@ -2821,7 +2824,7 @@ panthor_vm_bind_free_job(struct drm_sched_job > *sched_job) > /* Do the heavy cleanups asynchronously, so we're out of the > * dma-signaling path and can acquire dma-resv locks safely. > */ > - queue_work(panthor_cleanup_wq, &job->cleanup_op_ctx_work); > + queue_work(job->vm->ptdev->cleanup_wq, &job->cleanup_op_ctx_work); > } > > static enum drm_gpu_sched_stat > @@ -3372,7 +3375,7 @@ void panthor_mmu_unplug(struct panthor_device *ptdev) > * otherwise those might access objects that are gone if the work is > * executed after other components are unplugged. > */ > - flush_workqueue(panthor_cleanup_wq); > + flush_workqueue(ptdev->cleanup_wq); > } > > static void panthor_mmu_release_wq(struct drm_device *ddev, void *res) > @@ -3385,6 +3388,11 @@ static void panthor_mmu_info_init(struct > panthor_device *ptdev) > ptdev->mmu_info.page_size_bitmap = SZ_4K | SZ_2M; > } > > +static void free_pt_cache(struct drm_device *, void *pt_cache) > +{ > + kmem_cache_destroy(pt_cache); > +} > + > /** > * panthor_mmu_init() - Initialize the MMU logic. > * @ptdev: Device. > @@ -3414,6 +3422,14 @@ int panthor_mmu_init(struct panthor_device *ptdev) > if (ret) > return ret; > > + mmu->pt_cache = kmem_cache_create("panthor-mmu-pt", SZ_4K, SZ_4K, 0, > NULL); > + if (!mmu->pt_cache) > + return -ENOMEM; > + > + ret = drmm_add_action_or_reset(&ptdev->base, free_pt_cache, > mmu->pt_cache); > + if (ret) > + return ret; > + > mmu->iomem = ptdev->iomem + MMU_AS_BASE; > ptdev->mmu = mmu; > > @@ -3498,25 +3514,3 @@ void panthor_mmu_debugfs_init(struct drm_minor *minor) > minor->debugfs_root, minor); > } > #endif /* CONFIG_DEBUG_FS */ > - > -/** > - * panthor_mmu_pt_cache_init() - Initialize the page table cache. > - * > - * Return: 0 on success, a negative error code otherwise. > - */ > -int panthor_mmu_pt_cache_init(void) > -{ > - pt_cache = kmem_cache_create("panthor-mmu-pt", SZ_4K, SZ_4K, 0, NULL); > - if (!pt_cache) > - return -ENOMEM; > - > - return 0; > -} > - > -/** > - * panthor_mmu_pt_cache_fini() - Destroy the page table cache. > - */ > -void panthor_mmu_pt_cache_fini(void) > -{ > - kmem_cache_destroy(pt_cache); > -} > diff --git a/drivers/gpu/drm/panthor/panthor_mmu.h > b/drivers/gpu/drm/panthor/panthor_mmu.h > index 3522fbbce369..de6b4ee4e41a 100644 > --- a/drivers/gpu/drm/panthor/panthor_mmu.h > +++ b/drivers/gpu/drm/panthor/panthor_mmu.h > @@ -104,9 +104,6 @@ void panthor_vm_update_resvs(struct panthor_vm *vm, > struct drm_exec *exec, > enum dma_resv_usage private_usage, > enum dma_resv_usage extobj_usage); > > -int panthor_mmu_pt_cache_init(void); > -void panthor_mmu_pt_cache_fini(void); > - > #ifdef CONFIG_DEBUG_FS > void panthor_mmu_debugfs_init(struct drm_minor *minor); > #endif > diff --git a/drivers/gpu/drm/panthor/panthor_sched.c > b/drivers/gpu/drm/panthor/panthor_sched.c > index a6d57dc1b43e..4ea16b40d6b9 100644 > --- a/drivers/gpu/drm/panthor/panthor_sched.c > +++ b/drivers/gpu/drm/panthor/panthor_sched.c > @@ -972,7 +972,7 @@ static void group_release(struct kref *kref) > drm_WARN_ON(&ptdev->base, !list_empty(&group->run_node)); > drm_WARN_ON(&ptdev->base, !list_empty(&group->wait_node)); > > - queue_work(panthor_cleanup_wq, &group->release_work); > + queue_work(ptdev->cleanup_wq, &group->release_work); > } > > static void group_put(struct panthor_group *group) > @@ -4054,7 +4054,7 @@ void panthor_sched_unplug(struct panthor_device *ptdev) > * otherwise those might access objects that are gone if the work is > * executed after other components are unplugged. > */ > - flush_workqueue(panthor_cleanup_wq); > + flush_workqueue(ptdev->cleanup_wq); > } > > static void panthor_sched_fini(struct drm_device *ddev, void *res) > > -- > 2.55.0 > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯
