Re: [Updated PATCH v2] drm: modify drm_global_item_ref to avoid two times of writing ref->object

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 07.09.2016 um 07:24 schrieb Huang Rui:
In previous drm_global_item_ref, there are two times of writing
ref->object if item->refcount is 0. So this patch does a minor update
to put alloc and init ref firstly, and then to modify the item of glob
array. Use "else" to avoid two times of writing ref->object. It can
make the code logic more clearly.

Signed-off-by: Huang Rui <ray.huang@xxxxxxx>

Well when you update your patch, even when it's just fixing a small typo, please increase the version number.

That makes it much easier to track the different instances of a patch.

A few additional notes below.

---

Changes from V1 -> V2:
- Add kfree exceptional handle to avoid memory leak.
- Improve code style.

---
  drivers/gpu/drm/drm_global.c | 23 +++++++++++++----------
  1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_global.c b/drivers/gpu/drm/drm_global.c
index 3d2e91c..b181e81 100644
--- a/drivers/gpu/drm/drm_global.c
+++ b/drivers/gpu/drm/drm_global.c
@@ -65,30 +65,33 @@ void drm_global_release(void)
int drm_global_item_ref(struct drm_global_reference *ref)
  {
-	int ret;
+	int ret = 0;
  	struct drm_global_item *item = &glob[ref->global_type];
mutex_lock(&item->mutex);
  	if (item->refcount == 0) {
-		item->object = kzalloc(ref->size, GFP_KERNEL);
-		if (unlikely(item->object == NULL)) {
+		ref->object = kzalloc(ref->size, GFP_KERNEL);
+		if (unlikely(ref->object == NULL)) {
  			ret = -ENOMEM;
-			goto out_err;
+			goto out;
  		}
-
-		ref->object = item->object;
  		ret = ref->init(ref);
  		if (unlikely(ret != 0))
  			goto out_err;
+ item->object = ref->object;
+	} else {
+		ref->object = item->object;
  	}
+
  	++item->refcount;
-	ref->object = item->object;
-	mutex_unlock(&item->mutex);
-	return 0;
+	goto out;

A goto in the not error path is a bit unusual. Since out_err is only used once couldn't you just move the code into the if and then goto to out as well?

Alternative you could just duplicate the mutex release in the non-error path.

Regards,
Christian.

+
  out_err:
+	kfree(ref->object);
+	ref->object = NULL;
+out:
  	mutex_unlock(&item->mutex);
-	item->object = NULL;
  	return ret;
  }
  EXPORT_SYMBOL(drm_global_item_ref);


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux