Attachment 281207 Details for Bug 472046

[patch] Fix.

0001-Fixed-SYNC-extension-trigger-BlockHandler-test.patch (text/plain), 10.39 KB, created by Egbert Eich on 2009-03-23 14:42:23 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Egbert Eich

Created: 2009-03-23 14:42:23 UTC

Size: 10.39 KB

patch

obsolete

>From 6dbae4f4841ddfb70d6f2a973431f34d2b3cc4c0 Mon Sep 17 00:00:00 2001
>From: Egbert Eich <eich@freedesktop.org>
>Date: Mon, 23 Mar 2009 12:58:28 +0100
>Subject: [PATCH] Fixed SYNC extension trigger BlockHandler test.
>
>* Reworked ComputeBracketValues():
>  + Removed 'startOver' flag: we always seem to want to start
>    over, there doesn't seem to be a reason why we should keep the old
>    bracket values, moreover when this flag was set the semantics of
>    setting the bracket values for the selected sync counter were flawed
>    anyhow:
>    the bracket whose value hadn't changed (if the other one did) was simply
>    deleted.
>  + Rewrote the bracket value calculation for Postitive/NegativeTransition:
>    the calulation didn't cover all possible cases and was flawed anyhow.
>* Reworked previous patch to IdleTimeBlockHandler()
>  (commit: 1f4fb022)
>  + Simplified code.
>  + Modified heuristics:
>    pIdleTimeValueLess does not exclude pIdleTimeValueGreater: if no wakeup
>    is scheduled for the former one check if there is one for the latter.
>  + If no immediate wakeup is scheduled at all find schedule a wakeup for
>    the next time an idle counter might trigger, do not wait for the latest
>    idle counter to trigger.
>    This fixes a problem introduced with commit 1f4fb022 where an idle counter
>    expires unnoticed.
>---
> Xext/sync.c |  179 +++++++++++++++++++++++++++++++----------------------------
> 1 files changed, 94 insertions(+), 85 deletions(-)
>
>diff --git a/Xext/sync.c b/Xext/sync.c
>index adb7324..4c03e06 100644
>--- a/Xext/sync.c
>+++ b/Xext/sync.c
>@@ -97,7 +97,7 @@ static SyncCounter **SysCounterList = NULL;
> #define XSyncCAAllTrigger \
>     (XSyncCACounter | XSyncCAValueType | XSyncCAValue | XSyncCATestType)
> 
>-static void SyncComputeBracketValues(SyncCounter *, Bool);
>+static void SyncComputeBracketValues(SyncCounter *);
> 
> static void SyncInitServerTime(void);
> 
>@@ -170,7 +170,7 @@ SyncDeleteTriggerFromCounter(SyncTrigger *pTrigger)
>     }
> 
>     if (IsSystemCounter(pTrigger->pCounter))
>-	SyncComputeBracketValues(pTrigger->pCounter, /*startOver*/ TRUE);
>+	SyncComputeBracketValues(pTrigger->pCounter);
> }
> 
> 
>@@ -197,7 +197,7 @@ SyncAddTriggerToCounter(SyncTrigger *pTrigger)
>     pTrigger->pCounter->pTriglist = pCur;
> 
>     if (IsSystemCounter(pTrigger->pCounter))
>-	SyncComputeBracketValues(pTrigger->pCounter, /*startOver*/ TRUE);
>+	SyncComputeBracketValues(pTrigger->pCounter);
> 
>     return Success;
> }
>@@ -354,7 +354,7 @@ SyncInitTrigger(ClientPtr client, SyncTrigger *pTrigger, XSyncCounter counter,
>     }
>     else if (IsSystemCounter(pCounter))
>     {
>-	SyncComputeBracketValues(pCounter, /*startOver*/ TRUE);
>+	SyncComputeBracketValues(pCounter);
>     }
> 
>     return Success;
>@@ -643,13 +643,14 @@ SyncChangeCounter(SyncCounter *pCounter, CARD64 newval)
>     for (ptl = pCounter->pTriglist; ptl; ptl = pnext)
>     {
> 	pnext = ptl->next;
>-	if ((*ptl->pTrigger->CheckTrigger)(ptl->pTrigger, oldval))
>+	if ((*ptl->pTrigger->CheckTrigger)(ptl->pTrigger, oldval)) {
> 	    (*ptl->pTrigger->TriggerFired)(ptl->pTrigger);
>+	}
>     }
> 
>     if (IsSystemCounter(pCounter))
>     {
>-	SyncComputeBracketValues(pCounter, /* startOver */ FALSE);
>+	SyncComputeBracketValues(pCounter);
>     }
> }
> 
>@@ -916,7 +917,7 @@ SyncDestroySystemCounter(pointer pSysCounter)
> }
> 
> static void
>-SyncComputeBracketValues(SyncCounter *pCounter, Bool startOver)
>+SyncComputeBracketValues(SyncCounter *pCounter)
> {
>     SyncTriggerList *pCur;
>     SyncTrigger *pTrigger;
>@@ -933,60 +934,51 @@ SyncComputeBracketValues(SyncCounter *pCounter, Bool startOver)
>     if (ct == XSyncCounterNeverChanges)
> 	return;
> 
>-    if (startOver)
>-    {
>-	XSyncMaxValue(&psci->bracket_greater);
>-	XSyncMinValue(&psci->bracket_less);
>-    }
>+    XSyncMaxValue(&psci->bracket_greater);
>+    XSyncMinValue(&psci->bracket_less);
>+
> 
>     for (pCur = pCounter->pTriglist; pCur; pCur = pCur->next)
>     {
> 	pTrigger = pCur->pTrigger;
>-	
>-        if (pTrigger->test_type == XSyncPositiveComparison &&
>-	    ct != XSyncCounterNeverIncreases)
>-	{
>-	    if (XSyncValueLessThan(pCounter->value, pTrigger->test_value) &&
>-		XSyncValueLessThan(pTrigger->test_value,
>-				   psci->bracket_greater))
>-	    {
>-		psci->bracket_greater = pTrigger->test_value;
>-		pnewgtval = &psci->bracket_greater;
>-	    }
>-	}
>-	else if (pTrigger->test_type == XSyncNegativeComparison &&
>-		 ct != XSyncCounterNeverDecreases)
>-	{
>-	    if (XSyncValueGreaterThan(pCounter->value, pTrigger->test_value) &&
>-		XSyncValueGreaterThan(pTrigger->test_value,
>-				      psci->bracket_less))
>-	    {
>-		psci->bracket_less = pTrigger->test_value;
>-		pnewltval = &psci->bracket_less;
>-	    }
>-	}
>-	else if ( (pTrigger->test_type == XSyncPositiveTransition &&
>-		   ct != XSyncCounterNeverIncreases)
>-		 ||
>-		 (pTrigger->test_type == XSyncNegativeTransition &&
>-		  ct != XSyncCounterNeverDecreases)
>+
>+        if ( ( (pTrigger->test_type == XSyncPositiveComparison)
>+	       && ( XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value)
>+		    || (ct != XSyncCounterNeverIncreases
>+			&& XSyncValueLessThan(pCounter->value, pTrigger->test_value))
>+		    )
>+	       )
>+	     || ((pTrigger->test_type == XSyncPositiveTransition)
>+		 && ( ( ct != XSyncCounterNeverDecreases
>+			&& XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value))
>+		      || ( ct != XSyncCounterNeverIncreases
>+			  && XSyncValueLessThan(pCounter->value, pTrigger->test_value))
>+		      )
> 		 )
>-	{
>-	    if (XSyncValueLessThan(pCounter->value, pTrigger->test_value))
>-	    {
>-		if (XSyncValueLessThan(pTrigger->test_value,
>-				       psci->bracket_greater))
>-		{
>-		    psci->bracket_greater = pTrigger->test_value;
>-		    pnewgtval = &psci->bracket_greater;
>-		}
>-		else
>-		if (XSyncValueGreaterThan(pTrigger->test_value,
>-					  psci->bracket_less))
>-		{
>-		    psci->bracket_less = pTrigger->test_value;
>-		    pnewltval = &psci->bracket_less;
>+	     ) {
>+	      if (XSyncValueLessThan(pTrigger->test_value,
>+				     psci->bracket_greater)) {
>+		  psci->bracket_greater = pTrigger->test_value;
>+		  pnewgtval = &psci->bracket_greater;
> 		}
>+	} else if ( ( (pTrigger->test_type == XSyncNegativeComparison)
>+		      && ( XSyncValueLessOrEqual(pCounter->value, pTrigger->test_value)
>+			   || (ct != XSyncCounterNeverDecreases
>+			       && XSyncValueGreaterThan(pCounter->value, pTrigger->test_value))
>+			   )
>+		      )
>+		    || ((pTrigger->test_type == XSyncNegativeTransition)
>+			&& ( ( ct != XSyncCounterNeverDecreases
>+			       && XSyncValueGreaterOrEqual(pCounter->value, pTrigger->test_value))
>+			     || ( ct != XSyncCounterNeverIncreases
>+				  && XSyncValueLessThan(pCounter->value, pTrigger->test_value))
>+			    )
>+			)
>+		    ) {
>+	    if (XSyncValueGreaterThan(pTrigger->test_value,
>+				      psci->bracket_less)) {
>+		psci->bracket_less = pTrigger->test_value;
>+		pnewltval = &psci->bracket_less;
> 	    }
> 	}
>     } /* end for each trigger */
>@@ -2291,11 +2283,17 @@ IdleTimeQueryValue (pointer pCounter, CARD64 *pValue_return)
> }
> 
> static void
>-IdleTimeBlockHandler(pointer env, struct timeval **wt, pointer LastSelectMask)
>-{
>-    XSyncValue idle, old_idle;
>-    SyncTriggerList *list = IdleTimeCounter->pTriglist;
>-    SyncTrigger *trig;
>+IdleTimeBlockHandler (pointer env,
>+                      struct timeval **wt,
>+                      pointer LastSelectMask)
>+{
>+  XSyncValue idle, old_idle;
>+  SyncTriggerList *list = IdleTimeCounter->pTriglist;
>+  SyncTrigger *trig;
>+  unsigned long timeout = -1;
>+  XSyncValue value;
>+  Bool overflow;
>+ 
> 
>     if (!pIdleTimeValueLess && !pIdleTimeValueGreater)
> 	return;
>@@ -2313,46 +2311,57 @@ IdleTimeBlockHandler(pointer env, struct timeval **wt, pointer LastSelectMask)
> 	 * want level or edge trigger.  Check the trigger list against the
> 	 * current idle time, and if any succeed, bomb out of select()
> 	 * immediately so we can reschedule.
>+	 * NOTE: we need to check trigger because the idle timer can go
>+	 * both ways (XSyncCounterUnrestricted) so that we need to set
>+	 * pIdleTimeValueLess in ComputeBracketValues() in the edge
>+	 * triggered case even if the idle timer is already less.
> 	 */
>-
>-	for (list = IdleTimeCounter->pTriglist; list; list = list->next) {
>+        for (list = IdleTimeCounter->pTriglist; list; list = list->next) {
> 	    trig = list->pTrigger;
> 	    if (trig->CheckTrigger(trig, old_idle)) {
>-		AdjustWaitForDelay(wt, 0);
>-		break;
>+	        AdjustWaitForDelay(wt, 0);
>+		IdleTimeCounter->value = old_idle; /* pop */
>+		return;
> 	    }
> 	}
>     }
>-    else if (pIdleTimeValueGreater)
>+    if (pIdleTimeValueGreater)
>     {
>-	/*
>-	 * There's a threshold in the positive direction.  If we've been
>-	 * idle less than it, schedule a wakeup for sometime in the future.
>-	 * If we've been idle more than it, and someone wants to know about
>-	 * that level-triggered, schedule an immediate wakeup.
>-	 */
>-	unsigned long timeout = -1;
>-
>-	if (XSyncValueLessThan (idle, *pIdleTimeValueGreater)) {
>-	    XSyncValue value;
>-	    Bool overflow;
>+      /*
>+       * There's a threshold in the positive direction.
>+       * If we've been idle more than it, and someone wants to know about
>+       * that level-triggered, schedule an immediate wakeup.
>+       * NOTE: we need to check trigger because the idle timer can go
>+       * both ways (XSyncCounterUnrestricted) so that we need to set
>+       * pIdleTimeValueGreater in ComputeBracketValues() in the edge
>+       * triggered case even if the idle timer is already greater.
>+       */
> 
>-	    XSyncValueSubtract (&value, *pIdleTimeValueGreater,
>-	                        idle, &overflow);
>-	    timeout = min(timeout, XSyncValueLow32 (value));
>-	} else {
>+	if (XSyncValueGreaterOrEqual (idle, *pIdleTimeValueGreater)) {
> 	    for (list = IdleTimeCounter->pTriglist; list; list = list->next) {
>-		trig = list->pTrigger;
>+	        trig = list->pTrigger;
> 		if (trig->CheckTrigger(trig, old_idle)) {
>-		    timeout = min(timeout, 0);
>-		    break;
>+		    AdjustWaitForDelay (wt, 0);
>+		    IdleTimeCounter->value = old_idle; /* pop */
>+		    return;
> 		}
> 	    }
> 	}
>-
>-	AdjustWaitForDelay (wt, timeout);
>     }
> 
>+    /*
>+     * If we don't have to wake up immediately we schedule a wakeup for the
>+     * next time a trigger expires.
>+     */
>+    for (list = IdleTimeCounter->pTriglist; list; list = list->next) {
>+        trig = list->pTrigger;
>+	if (XSyncValueLessThan (idle, trig->test_value)) {
>+	    XSyncValueSubtract (&value, trig->test_value,
>+				idle, &overflow);
>+	    timeout = min(timeout,XSyncValueLow32 (value));
>+	}
>+    }
>+    AdjustWaitForDelay (wt, timeout);
>     IdleTimeCounter->value = old_idle; /* pop */
> }
> 
>-- 
>1.5.2.4
>

Actions: View | Diff

Attachments on bug 472046: 281207