Re: [PATCH v3] 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 08.09.2016 um 09:35 schrieb Chris Wilson:
On Thu, Sep 08, 2016 at 03:22:48PM +0800, Huang Rui wrote:
On Thu, Sep 08, 2016 at 02:36:06PM +0800, Chris Wilson wrote:
On Wed, Sep 07, 2016 at 10:07:57PM -0400, Huang Rui wrote:
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>
---

Changes from V2 -> V3:
- Use duplicate mutex release to avoid "goto" in non-error patch.
- Rename error label

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

diff --git a/drivers/gpu/drm/drm_global.c b/drivers/gpu/drm/drm_global.c
index 3d2e91c..b404287 100644
--- a/drivers/gpu/drm/drm_global.c
+++ b/drivers/gpu/drm/drm_global.c
@@ -65,30 +65,34 @@ 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);
So the item refcount is 0, we operate on ref, whereas previous we
inspected item and operated on item. Not an improvement.
Hmm, when item refcount is 0, we operate on ref to create object and init
it for item, so although item->object and ref->object here should point the
same thing, but we should alloc and init ref firstly and pass the
ref->object address to item (item actually points a static area). This make
the code logic more clearly and readable. So I updated it to "ref" here.
:-)
The object is owned by item. ref is just a pointer to it.

Yeah, so what? We initialized ref->item when the refcount was 0 before as well.

Why not create the reference first and initialize the item later on?

Regards,
Christian.

-Chris


_______________________________________________
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