Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Permanent CMA capacity leak due to clearing dev->cma_area before GEM objects are freed during device unbind. -- commit a1f947cbe50ede0e9b650dcb9e31cc36cb85952d Author: Mukesh Ojha <[email protected]> drm: malidp: Use devm_of_reserved_mem_device_init() This commit simplifies the driver by transitioning to the devres-managed devm_of_reserved_mem_device_init(), removing manual cleanup calls during bind failures and unbind. > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > b/drivers/gpu/drm/arm/malidp_drv.c > index 23fa942ae4bbe..3e17513c31147 100644 > --- a/drivers/gpu/drm/arm/malidp_drv.c > +++ b/drivers/gpu/drm/arm/malidp_drv.c > @@ -758,7 +758,7 @@ static int malidp_bind(struct device *dev) > return PTR_ERR(hwdev->pxlclk); > > /* Get the optional framebuffer memory resource */ > - ret = of_reserved_mem_device_init(dev); > + ret = devm_of_reserved_mem_device_init(dev); [Severity: High] This isn't a bug introduced by this patch, but does this devres conversion preserve a pre-existing CMA memory capacity leak during device unbind? In malidp_bind(), devm_drm_dev_alloc() is called before this initialization function. Because devres actions execute in reverse order of allocation, the cleanup action registered here will execute before the drm_dev_put() action. When malidp_unbind() returns, the component framework immediately releases the component's devres group, clearing dev->cma_area. Later, when the DRM device is freed (either immediately or when userspace eventually closes its file descriptors), drm_gem_object_free() calls dma_free_contiguous(). Because dev->cma_area is already NULL at that point, it bypasses cma_release() and falls back to the buddy allocator, permanently leaking the CMA region's bitmap capacity. The previous code suffered from this exact same ordering issue since the manual of_reserved_mem_device_release() call happened before drm_dev_put() ran. Is there a way to adjust the cleanup order so the CMA area outlives the GEM objects? > if (ret && ret != -ENODEV) > return ret; > [ ... ] > @@ -913,7 +912,6 @@ static void malidp_unbind(struct device *dev) > else > malidp_runtime_pm_suspend(dev); > dev_set_drvdata(dev, NULL); > - of_reserved_mem_device_release(dev); > } > > static const struct component_master_ops malidp_master_ops = { -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
