From mboxrd@z Thu Jan  1 00:00:00 1970
From: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Subject: Re: [Linaro-mm-sig] thoughts of looking at android fences
Date: Fri, 08 Nov 2013 11:43:41 +0100
Message-ID: <527CC05D.9010400@canonical.com>
References: <524BCCD0.90002@canonical.com>	<CACSP8ShRk_m6VOvooaQ+T3aq=zf=LFz=u5rSF0mWXLfj6K+hQw@mail.gmail.com>	<CALAqxLUWLCQ67NHvpBoQUjP+s32YHh326uUgMH9Av5+Ma8stcg@mail.gmail.com>	<52556ABE.2090201@canonical.com>	<52690EEC.5000501@canonical.com>	<5270F8D7.4040406@canonical.com>	<CAMbhsRQ67B+MLLp-YhYhegDrOWp9AYN8eAaZnjdTc-wuwnM9vA@mail.gmail.com>	<5277777E.5060904@canonical.com>
	<CAABpnA-85QdwoYO5Ldz52bxw1_dWaQz541MjgEGZhwdu+Bhtvw@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <dri-devel-bounces@lists.freedesktop.org>
Received: from youngberry.canonical.com (youngberry.canonical.com
	[91.189.89.112])
	by gabe.freedesktop.org (Postfix) with ESMTP id 86307EE745
	for <dri-devel@lists.freedesktop.org>;
	Fri,  8 Nov 2013 02:43:45 -0800 (PST)
In-Reply-To: <CAABpnA-85QdwoYO5Ldz52bxw1_dWaQz541MjgEGZhwdu+Bhtvw@mail.gmail.com>
List-Unsubscribe: <http://lists.freedesktop.org/mailman/options/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <http://lists.freedesktop.org/archives/dri-devel>
List-Post: <mailto:dri-devel@lists.freedesktop.org>
List-Help: <mailto:dri-devel-request@lists.freedesktop.org?subject=help>
List-Subscribe: <http://lists.freedesktop.org/mailman/listinfo/dri-devel>,
	<mailto:dri-devel-request@lists.freedesktop.org?subject=subscribe>
Sender: dri-devel-bounces@lists.freedesktop.org
Errors-To: dri-devel-bounces@lists.freedesktop.org
To: Rom Lemarchand <romlem@google.com>
Cc: "linaro-mm-sig@lists.linaro.org" <linaro-mm-sig@lists.linaro.org>, Android Kernel Team <kernel-team@android.com>, "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>, Colin Cross <ccross@google.com>
List-Id: dri-devel@lists.freedesktop.org

op 07-11-13 22:11, Rom Lemarchand schreef:
> Hi Maarten, I tested your changes and needed the attached patch: behavior
> now seems equivalent as android sync. I haven't tested performance.
>
> The issue resolved by this patch happens when i_b < b->num_fences and
> i_a >= a->num_fences (or vice versa). Then, pt_a is invalid and so
> dereferencing pt_a->context causes a crash.

Oops, thinko. :) Originally I had it correct by doing this:

+       /*
+        * Assume sync_fence a and b are both ordered and have no
+        * duplicates with the same context.
+        *
+        * If a sync_fence can only be created with sync_fence_merge
+        * and sync_fence_create, this is a reasonable assumption.
+        */
+       for (i = i_a = i_b = 0; i_a < a->num_fences && i_b < b->num_fences; ) {
+               struct fence *pt_a = a->cbs[i_a].sync_pt;
+               struct fence *pt_b = b->cbs[i_b].sync_pt;
+
+               if (pt_a->context < pt_b->context) {
+                       sync_fence_add_pt(fence, &i, pt_a);
+
+                       i_a++;
+               } else if (pt_a->context > pt_b->context) {
+                       sync_fence_add_pt(fence, &i, pt_b);
+
+                       i_b++;
+               } else {
+                       if (pt_a->seqno - pt_b->seqno <= INT_MAX)
+                               sync_fence_add_pt(fence, &i, pt_a);
+                       else
+                               sync_fence_add_pt(fence, &i, pt_b);
+
+                       i_a++;
+                       i_b++;
+               }
+        }
+
+        /* Add remaining fences from a or b*/
+        for (; i_a < a->num_fences; i_a++)
+               sync_fence_add_pt(fence, &i, a->cbs[i_a].sync_pt);
+
+        for (; i_b < b->num_fences; i_b++)
+               sync_fence_add_pt(fence, &i, b->cbs[i_b].sync_pt);

Then I thought I could clean it up by merging it, but that ended up being
more unreadable and crashing... so I guess I'll revert back to this version. :)