Discussion:
[PATCH] drm/lease: fix WARNING in idr_destroy
(too old to reply)
Daniel Vetter
2020-03-17 16:57:29 UTC
Permalink
drm_master_put
->drm_master_destroy
->idr_destroy
so the "out_lessee" needn't to call idr_destroy again.
---
drivers/gpu/drm/drm_lease.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index b481caf..54506c2 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -577,11 +577,14 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
drm_master_put(&lessee);
+ goto err_exit;
I think this is correct, but also confusioning and inconsistent with the
existing style. Most error cases before drm_lease_create explicit destroy
the idr, except the one for drm_lease_create.

Instead of your patch I'd
- remove the idr_destry from the error unrolling here
- add it the to drm_lease_create error case
- add a comment above the call to drm_lease_crete that it takes ownership
of the lease idr

Can you pls respin and retest to make sure that patch is still correct?

Thanks, Daniel
- put_unused_fd(fd);
idr_destroy(&leases);
+ put_unused_fd(fd);
+
DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret);
return ret;
}
--
1.8.3.1
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Qiujun Huang
2020-03-17 22:29:02 UTC
Permalink
Post by Daniel Vetter
drm_master_put
->drm_master_destroy
->idr_destroy
so the "out_lessee" needn't to call idr_destroy again.
---
drivers/gpu/drm/drm_lease.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index b481caf..54506c2 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -577,11 +577,14 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
drm_master_put(&lessee);
+ goto err_exit;
I think this is correct, but also confusioning and inconsistent with the
existing style. Most error cases before drm_lease_create explicit destroy
the idr, except the one for drm_lease_create.
Yeah, it is.
Post by Daniel Vetter
Instead of your patch I'd
- remove the idr_destry from the error unrolling here
- add it the to drm_lease_create error case
- add a comment above the call to drm_lease_crete that it takes ownership
of the lease idr
Can you pls respin and retest to make sure that patch is still correct?
I get that, thanks.
Post by Daniel Vetter
Thanks, Daniel
- put_unused_fd(fd);
idr_destroy(&leases);
+ put_unused_fd(fd);
+
DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl failed: %d\n", ret);
return ret;
}
--
1.8.3.1
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Continue reading on narkive:
Loading...