Messages in this thread | | | Date | Tue, 2 Apr 2024 19:35:41 +0530 | Subject | Re: [PATCH v2] drm/panthor: Fix couple of NULL vs IS_ERR() bugs | From | Harshit Mogalapalli <> |
| |
Hi Boris, On 02/04/24 19:32, Boris Brezillon wrote: > On Tue, 2 Apr 2024 06:47:08 -0700 > Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> wrote: > >> 1. The devm_drm_dev_alloc() function returns error pointers. >> Update the error handling to check for error pointers instead of NULL. >> 2. Currently panthor_vm_get_heap_pool() returns both ERR_PTR() and >> NULL(when create is false and if there is no poool attached to the >> VM) >> - Change the function to return error pointers, when pool is >> NULL return -ENOENT >> - Also handle the callers to check for IS_ERR() on failure. >> >> Fixes: 4bdca1150792 ("drm/panthor: Add the driver frontend block") >> Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com> >> --- >> This is spotted by smatch and the patch is only compile tested >> >> v1->v2: Fix the function panthor_vm_get_heap_pool() to only return error >> pointers and handle the caller sites [Suggested by Boris Brezillon] >> - Also merge these IS_ERR() vs NULL bugs into same patch >> --- >> drivers/gpu/drm/panthor/panthor_drv.c | 6 +++--- >> drivers/gpu/drm/panthor/panthor_mmu.c | 2 ++ >> drivers/gpu/drm/panthor/panthor_sched.c | 2 +- >> 3 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c >> index 11b3ccd58f85..c8374cd4a30d 100644 >> --- a/drivers/gpu/drm/panthor/panthor_drv.c >> +++ b/drivers/gpu/drm/panthor/panthor_drv.c >> @@ -1090,8 +1090,8 @@ static int panthor_ioctl_tiler_heap_destroy(struct drm_device *ddev, void *data, >> return -EINVAL; >> >> pool = panthor_vm_get_heap_pool(vm, false); >> - if (!pool) { >> - ret = -EINVAL; >> + if (IS_ERR(pool)) { >> + ret = PTR_ERR(pool); >> goto out_put_vm; >> } >> >> @@ -1385,7 +1385,7 @@ static int panthor_probe(struct platform_device *pdev) >> >> ptdev = devm_drm_dev_alloc(&pdev->dev, &panthor_drm_driver, >> struct panthor_device, base); >> - if (!ptdev) >> + if (IS_ERR(ptdev)) >> return -ENOMEM; >> > > Sorry, that one deserves a separate patch. >
Ah okay, I was confused about the same. So I will send a V3 removing that part and could you please use the independent patch that I sent before ?
https://lore.kernel.org/all/20240402104041.1689951-1-harshit.m.mogalapalli@oracle.com/
Thanks, Harshit
>> platform_set_drvdata(pdev, ptdev); >> diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c >> index fdd35249169f..e1285cdb09ff 100644 >> --- a/drivers/gpu/drm/panthor/panthor_mmu.c >> +++ b/drivers/gpu/drm/panthor/panthor_mmu.c >> @@ -1893,6 +1893,8 @@ struct panthor_heap_pool *panthor_vm_get_heap_pool(struct panthor_vm *vm, bool c >> vm->heaps.pool = panthor_heap_pool_get(pool); >> } else { >> pool = panthor_heap_pool_get(vm->heaps.pool); >> + if (!pool) >> + pool = ERR_PTR(-ENOENT); >> } >> mutex_unlock(&vm->heaps.lock); >> >> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c >> index 5f7803b6fc48..617df2b980d0 100644 >> --- a/drivers/gpu/drm/panthor/panthor_sched.c >> +++ b/drivers/gpu/drm/panthor/panthor_sched.c >> @@ -1343,7 +1343,7 @@ static int group_process_tiler_oom(struct panthor_group *group, u32 cs_id) >> if (unlikely(csg_id < 0)) >> return 0; >> >> - if (!heaps || frag_end > vt_end || vt_end >= vt_start) { >> + if (IS_ERR(heaps) || frag_end > vt_end || vt_end >= vt_start) { >> ret = -EINVAL; >> } else { >> /* We do the allocation without holding the scheduler lock to avoid > >
| |