[PATH] drm/loongson: add cleanup path for lsdc_ttm_init

[PATH] drm/loongson: add cleanup path for lsdc_ttm_init

[yaolu]

From: Lu Yao <yaolu@kylinos.cn>

There has resource leak when TTM_PL_VRAM or TTM_PL_TT man range init
failed.

Signed-off-by: Lu Yao <yaolu@kylinos.cn>
---
 drivers/gpu/drm/loongson/lsdc_ttm.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/loongson/lsdc_ttm.c b/drivers/gpu/drm/loongson/lsdc_ttm.c
index d7441d96a0dc..c84bc582f77c 100644
--- a/drivers/gpu/drm/loongson/lsdc_ttm.c
+++ b/drivers/gpu/drm/loongson/lsdc_ttm.c
@@ -554,7 +554,7 @@ int lsdc_ttm_init(struct lsdc_device *ldev)
 
 	ret = ttm_range_man_init(&ldev->bdev, TTM_PL_VRAM, false, num_vram_pages);
 	if (unlikely(ret))
-		return ret;
+		goto err_vram;
 
 	drm_info(ddev, "VRAM: %lu pages ready\n", num_vram_pages);
 
@@ -565,11 +565,17 @@ int lsdc_ttm_init(struct lsdc_device *ldev)
 
 	ret = ttm_range_man_init(&ldev->bdev, TTM_PL_TT, true, num_gtt_pages);
 	if (unlikely(ret))
-		return ret;
+		goto err_gtt;
 
 	drm_info(ddev, "GTT: %lu pages ready\n", num_gtt_pages);
 
 	return drmm_add_action_or_reset(ddev, lsdc_ttm_fini, ldev);
+
+err_gtt:
+	ttm_range_man_fini(&ldev->bdev, TTM_PL_VRAM);
+err_vram:
+	ttm_device_fini(&ldev->bdev);
+	return ret;
 }
 
 void lsdc_ttm_debugfs_init(struct lsdc_device *ldev)
-- 
2.25.1

Re: [PATH] drm/loongson: add cleanup path for lsdc_ttm_init

[Xi Ruoyao]

On Tue, 2026-05-19 at 15:41 +0800, yaolu@kylinos.cn wrote:
> From: Lu Yao <yaolu@kylinos.cn>
> 
> There has resource leak when TTM_PL_VRAM or TTM_PL_TT man range init
> failed.
> 
> Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> ---
>  drivers/gpu/drm/loongson/lsdc_ttm.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/loongson/lsdc_ttm.c b/drivers/gpu/drm/loongson/lsdc_ttm.c
> index d7441d96a0dc..c84bc582f77c 100644
> --- a/drivers/gpu/drm/loongson/lsdc_ttm.c
> +++ b/drivers/gpu/drm/loongson/lsdc_ttm.c
> @@ -554,7 +554,7 @@ int lsdc_ttm_init(struct lsdc_device *ldev)
>  
>  	ret = ttm_range_man_init(&ldev->bdev, TTM_PL_VRAM, false, num_vram_pages);
>  	if (unlikely(ret))
> -		return ret;
> +		goto err_vram;
>  
>  	drm_info(ddev, "VRAM: %lu pages ready\n", num_vram_pages);
>  
> @@ -565,11 +565,17 @@ int lsdc_ttm_init(struct lsdc_device *ldev)
>  
>  	ret = ttm_range_man_init(&ldev->bdev, TTM_PL_TT, true, num_gtt_pages);
>  	if (unlikely(ret))
> -		return ret;
> +		goto err_gtt;
>  
>  	drm_info(ddev, "GTT: %lu pages ready\n", num_gtt_pages);
>  
>  	return drmm_add_action_or_reset(ddev, lsdc_ttm_fini, ldev);

Isn't the error handling path also needed in case
drmm_add_action_or_reset fails?

> +
> +err_gtt:
> +	ttm_range_man_fini(&ldev->bdev, TTM_PL_VRAM);
> +err_vram:
> +	ttm_device_fini(&ldev->bdev);
> +	return ret;
>  }
>  
>  void lsdc_ttm_debugfs_init(struct lsdc_device *ldev)

-- 
Xi Ruoyao <xry111@xry111.site>

Re: [PATH] drm/loongson: add cleanup path for lsdc_ttm_init

[Icenowy Zheng]

在 2026-05-19二的 15:43 +0800，Xi Ruoyao写道：
> On Tue, 2026-05-19 at 15:41 +0800, yaolu@kylinos.cn wrote:
> > From: Lu Yao <yaolu@kylinos.cn>
> > 
> > There has resource leak when TTM_PL_VRAM or TTM_PL_TT man range
> > init
> > failed.
> > 
> > Signed-off-by: Lu Yao <yaolu@kylinos.cn>
> > ---
> >  drivers/gpu/drm/loongson/lsdc_ttm.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/loongson/lsdc_ttm.c
> > b/drivers/gpu/drm/loongson/lsdc_ttm.c
> > index d7441d96a0dc..c84bc582f77c 100644
> > --- a/drivers/gpu/drm/loongson/lsdc_ttm.c
> > +++ b/drivers/gpu/drm/loongson/lsdc_ttm.c
> > @@ -554,7 +554,7 @@ int lsdc_ttm_init(struct lsdc_device *ldev)
> >  
> >  	ret = ttm_range_man_init(&ldev->bdev, TTM_PL_VRAM, false,
> > num_vram_pages);
> >  	if (unlikely(ret))
> > -		return ret;
> > +		goto err_vram;
> >  
> >  	drm_info(ddev, "VRAM: %lu pages ready\n", num_vram_pages);
> >  
> > @@ -565,11 +565,17 @@ int lsdc_ttm_init(struct lsdc_device *ldev)
> >  
> >  	ret = ttm_range_man_init(&ldev->bdev, TTM_PL_TT, true,
> > num_gtt_pages);
> >  	if (unlikely(ret))
> > -		return ret;
> > +		goto err_gtt;
> >  
> >  	drm_info(ddev, "GTT: %lu pages ready\n", num_gtt_pages);
> >  
> >  	return drmm_add_action_or_reset(ddev, lsdc_ttm_fini,
> > ldev);
> 
> Isn't the error handling path also needed in case
> drmm_add_action_or_reset fails?

drmm_add_action_or_reset adds an action for teardown, and if it fails,
the teardown will immediately happen (this is what `or_reset` means).

The `lsdc_ttm_fini()` function will properly finalize both ttm range
managers and the ttm device, so it's not a problem here.

This patch is quite valid for handling the situation that some init
operations succeeded and some failed in this function.

Thanks,
Icenowy

> 
> > +
> > +err_gtt:
> > +	ttm_range_man_fini(&ldev->bdev, TTM_PL_VRAM);
> > +err_vram:
> > +	ttm_device_fini(&ldev->bdev);
> > +	return ret;
> >  }
> >  
> >  void lsdc_ttm_debugfs_init(struct lsdc_device *ldev)