[PATCH 3/3] Reset only what the protocol requires, instead of full regeneration.

Aaron Plattner aplattner at nvidia.com
Mon Nov 28 09:24:45 PST 2011


Have you run this through XTS?

On 11/20/2011 03:48 AM, Jamey Sharp wrote:
> It's difficult to get regeneration correct in all the drivers,
> extensions, and everywhere else it's currently required--and there's no
> significant advantage to doing full regeneration. So instead, make DIX
> reset exactly the things required by the protocol specification, and set
> serverGeneration to a constant 1.
>
> Eliminating all the ugly code that's conditional on serverGeneration is
> left as a later exercise. This patch should simply improve reliability
> when the server resets.
>
> Signed-off-by: Jamey Sharp<jamey at minilop.net>
> ---
> This patch is generated with `diff -w`, so the patch's indentation is
> all wrong in places. I believe the real changes are easier to review
> this way, because I had to re-indent most of dix/main.c. If you want to
> review the version with correct indentation, please review it by cloning
> my public xserver repo or by browsing it in cgit:
>
> http://cgit.freedesktop.org/~jamey/xserver/commit/?h=no-regen&id=8fd86ed43310863d7c3321c007d9cf6419b8c120
>
>   dix/atom.c           |   30 +++++++++++++++++
>   dix/devices.c        |   22 ++++++++++++
>   dix/globals.c        |    1 -
>   dix/main.c           |   88 +++++++++++++++++++++++++++++++++-----------------
>   dix/property.c       |   21 ++++++++++++
>   dix/resource.c       |    3 +-
>   dix/window.c         |   13 +++++--
>   include/dix.h        |    7 ++++
>   include/misc.h       |    2 +-
>   include/property.h   |    2 +
>   include/propertyst.h |    1 +
>   include/window.h     |    3 ++
>   12 files changed, 156 insertions(+), 37 deletions(-)
>
> diff --git a/dix/atom.c b/dix/atom.c
> index 88b40db..5eb1d55 100644
> --- a/dix/atom.c
> +++ b/dix/atom.c
> @@ -68,6 +68,7 @@ typedef struct _Node {
>   } NodeRec, *NodePtr;
>
>   static Atom lastAtom = None;
> +static Atom lastServerAtom = None;
>   static NodePtr atomRoot = NULL;
>   static unsigned long tableLength;
>   static NodePtr *nodeTable;
> @@ -201,6 +202,7 @@ FreeAllAtoms(void)
>       free(nodeTable);
>       nodeTable = NULL;
>       lastAtom = None;
> +    lastServerAtom = None;
>   }
>
>   void
> @@ -216,3 +218,31 @@ InitAtoms(void)
>       if (lastAtom != XA_LAST_PREDEFINED)
>          AtomError();
>   }
> +
> +void
> +SaveServerAtoms(void)
> +{
> +    lastServerAtom = lastAtom;
> +}
> +
> +static void
> +ResetAtom(NodePtr *patom)
> +{
> +    if (!*patom)
> +       return;
> +    if ((*patom)->a>  lastServerAtom)
> +    {
> +       FreeAtom(*patom);
> +       *patom = NULL;
> +       return;
> +    }
> +    ResetAtom(&(*patom)->left);
> +    ResetAtom(&(*patom)->right);
> +}
> +
> +void
> +ResetAtoms(void)
> +{
> +    lastAtom = lastServerAtom;
> +    ResetAtom(&atomRoot);
> +}
> diff --git a/dix/devices.c b/dix/devices.c
> index da817a8..7d91416 100644
> --- a/dix/devices.c
> +++ b/dix/devices.c
> @@ -2645,3 +2645,25 @@ void valuator_set_mode(DeviceIntPtr dev, int axis, int mode)
>               dev->valuator->axes[i].mode = mode;
>       }
>   }
> +
> +void ResetInput(void)
> +{
> +    BITS32 mask = KBKeyClickPercent | KBBellPercent | KBBellPitch | KBBellDuration | KBAutoRepeatMode;
> +    XID attrs[] = { -1, -1, -1, -1, AutoRepeatModeDefault };
> +    DeviceIntPtr dev;
> +    for (dev = inputInfo.devices; dev; dev = dev->next)
> +    {
> +       /* TODO: reset keyboard and pointer mappings */
> +       if (dev->focus)
> +       {
> +           dev->focus->win = PointerRootWin;
> +           dev->focus->revert = None;
> +           dev->focus->time = currentTime;
> +           dev->focus->traceGood = 0;
> +       }
> +       if (dev->kbdfeed)
> +           DoChangeKeyboardControl(serverClient, dev, attrs, mask);
> +       if (dev->ptrfeed)
> +           dev->ptrfeed->ctrl = defaultPointerControl;
> +    }
> +}
> diff --git a/dix/globals.c b/dix/globals.c
> index 0a6b170..28f9dbd 100644
> --- a/dix/globals.c
> +++ b/dix/globals.c
> @@ -84,7 +84,6 @@ int  currentMaxClients;   /* current size of clients array */
>   long maxBigRequestSize = MAX_BIG_REQUEST_SIZE;
>
>   unsigned long globalSerialNumber = 0;
> -unsigned long serverGeneration = 0;
>
>   /* these next four are initialized in main.c */
>   CARD32 ScreenSaverTime;
> diff --git a/dix/main.c b/dix/main.c
> index 16575ce..745fe05 100644
> --- a/dix/main.c
> +++ b/dix/main.c
> @@ -149,9 +149,7 @@ int main(int argc, char *argv[], char *envp[])
>
>       alwaysCheckForInput[0] = 0;
>       alwaysCheckForInput[1] = 1;
> -    while(1)
> -    {
> -       serverGeneration++;
> +
>          ScreenSaverTime = defaultScreenSaverTime;
>          ScreenSaverInterval = defaultScreenSaverInterval;
>          ScreenSaverBlanking = defaultScreenSaverBlanking;
> @@ -164,18 +162,14 @@ int main(int argc, char *argv[], char *envp[])
>          InitBlockAndWakeupHandlers();
>          /* Perform any operating system dependent initializations you'd like */
>          OsInit();
> -       if(serverGeneration == 1)
> -       {
>              CreateWellKnownSockets();
> +
>              for (i=1; i<MAXCLIENTS; i++)
>                  clients[i] = NullClient;
>              serverClient = calloc(sizeof(ClientRec), 1);
>              if (!serverClient)
>                  FatalError("couldn't create server client");
>              InitClient(serverClient, 0, (pointer)NULL);
> -       }
> -       else
> -           ResetWellKnownSockets ();
>          clients[0] = serverClient;
>          currentMaxClients = 1;
>
> @@ -274,6 +268,8 @@ int main(int argc, char *argv[], char *envp[])
>              }
>          }
>
> +    SaveServerAtoms();
> +
>   #ifdef XQUARTZ
>          /* Let the other threads know the server is done with its init */
>          pthread_mutex_lock(&serverRunningMutex);
> @@ -284,8 +280,58 @@ int main(int argc, char *argv[], char *envp[])
>
>          NotifyParentProcess();
>
> +    while ((dispatchException&  DE_TERMINATE) == 0)
> +    {
>          Dispatch();
>
> +        /* As specified in "X Window System Protocol" chapter 10,
> +         * "Connection Close":
> +         *
> +         * At every transition to the state of having no connections as
> +         * a result of a connection closing with a Destroy close-down
> +         * mode, the server resets its state as if it had just been
> +         * started. */
> +
> +        /* - This starts by destroying all lingering resources from
> +         *   clients that have terminated in RetainPermanent or
> +         *   RetainTemporary mode. */
> +#ifdef PANORAMIX
> +        {
> +            Bool remember_it = noPanoramiXExtension;
> +            noPanoramiXExtension = TRUE;
> +            FreeAllResources();
> +            noPanoramiXExtension = remember_it;
> +        }
> +#else
> +        FreeAllResources();
> +#endif
> +
> +        /* - It additionally includes deleting all but the predefined
> +         *   atom identifiers, */
> +        ResetAtoms();
> +
> +        for (i = 0; i<  screenInfo.numScreens; i++)
> +        {
> +            /* - deleting all properties on all root windows, */
> +            ResetProperties(screenInfo.screens[i]->root);
> +
> +            /* - restoring the standard root tiles and cursors, */
> +            ResetRootWindow(screenInfo.screens[i]->root);
> +        }
> +
> +        /* - resetting the access control list, */
> +        ResetWellKnownSockets();
> +
> +        /* - restoring the default font path, */
> +        if (SetDefaultFontPath(defaultFontPath) != Success)
> +            ErrorF("[dix] failed to set default font path '%s'", defaultFontPath);
> +
> +        /* - resetting all device maps and attributes (key click, bell
> +         *   volume, acceleration),
> +         * - and restoring the input focus to state PointerRoot. */
> +        ResetInput();
> +    }
> +
>   #ifdef XQUARTZ
>          /* Let the other threads know the server is no longer running */
>          pthread_mutex_lock(&serverRunningMutex);
> @@ -301,16 +347,7 @@ int main(int argc, char *argv[], char *envp[])
>          FreeScreenSaverTimer();
>          CloseDownExtensions();
>
> -#ifdef PANORAMIX
> -       {
> -           Bool remember_it = noPanoramiXExtension;
> -           noPanoramiXExtension = TRUE;
> -           FreeAllResources();
> -           noPanoramiXExtension = remember_it;
> -       }
> -#else
> -       FreeAllResources();
> -#endif
> +    FreeClientResources(serverClient);
>
>           CloseInput();
>
> @@ -338,22 +375,13 @@ int main(int argc, char *argv[], char *envp[])
>
>          FreeAuditTimer();
>
> -       if (dispatchException&  DE_TERMINATE)
> -       {
>              CloseWellKnownConnections();
> -       }
> -
> -       OsCleanup((dispatchException&  DE_TERMINATE) != 0);
>
> -       if (dispatchException&  DE_TERMINATE)
> -       {
> -           ddxGiveUp(EXIT_NO_ERROR);
> -           break;
> -       }
> +    OsCleanup(TRUE);
>
>          free(ConnectionInfo);
> -       ConnectionInfo = NULL;
> -    }
> +
> +    ddxGiveUp(EXIT_NO_ERROR);
>       return 0;
>   }
>
> diff --git a/dix/property.c b/dix/property.c
> index a1ae530..87b8fe3 100644
> --- a/dix/property.c
> +++ b/dix/property.c
> @@ -284,6 +284,7 @@ dixChangeWindowProperty(ClientPtr pClient, WindowPtr pWin, Atom property,
>           pProp->format = format;
>           pProp->data = data;
>          pProp->size = len;
> +       pProp->retain = (pClient == serverClient);
>          rc = XaceHookPropertyAccess(pClient, pWin,&pProp,
>                                      DixCreateAccess|DixWriteAccess);
>          if (rc != Success) {
> @@ -429,6 +430,26 @@ DeleteAllWindowProperties(WindowPtr pWin)
>           pWin->optional->userProps = NULL;
>   }
>
> +void
> +ResetProperties(WindowPtr pWin)
> +{
> +    PropertyPtr tmp, *pProp;
> +    if (!pWin->optional)
> +       return;
> +
> +    for (pProp =&pWin->optional->userProps; (tmp = *pProp); )
> +    {
> +       if (tmp->retain&&  ValidAtom(tmp->propertyName)&&  ValidAtom(tmp->type))
> +       {
> +           pProp =&tmp->next;
> +           continue;
> +       }
> +       *pProp = tmp->next;
> +       free(tmp->data);
> +       dixFreeObjectWithPrivates(tmp, PRIVATE_PROPERTY);
> +    }
> +}
> +
>   static int
>   NullPropertyReply(
>       ClientPtr client,
> diff --git a/dix/resource.c b/dix/resource.c
> index eb9f049..efe1964 100644
> --- a/dix/resource.c
> +++ b/dix/resource.c
> @@ -863,7 +863,8 @@ FreeAllResources(void)
>   {
>       int        i;
>
> -    for (i = currentMaxClients; --i>= 0; )
> +    /* don't free serverClient's resources */
> +    for (i = currentMaxClients; --i>  0; )
>       {
>           if (clientTable[i].buckets)
>              FreeClientResources(clients[i]);
> diff --git a/dix/window.c b/dix/window.c
> index c87020d..987bb33 100644
> --- a/dix/window.c
> +++ b/dix/window.c
> @@ -543,16 +543,21 @@ void
>   InitRootWindow(WindowPtr pWin)
>   {
>       ScreenPtr pScreen = pWin->drawable.pScreen;
> -    int mask = CWBackPixmap | CWBackingStore | CWCursor;
> -    XID attrs[] = { None, defaultBackingStore, None };
> -
>       if (!(*pScreen->CreateWindow)(pWin))
>          return; /* XXX */
>       (*pScreen->PositionWindow)(pWin, 0, 0);
> -    ChangeWindowAttributes(pWin, mask, attrs, serverClient);
> +    ResetRootWindow(pWin);
>       MapWindow(pWin, serverClient);
>   }
>
> +void
> +ResetRootWindow(WindowPtr pWin)
> +{
> +    int mask = CWBackPixmap | CWBackingStore | CWCursor;
> +    XID attrs[] = { None, defaultBackingStore, None };
> +    ChangeWindowAttributes(pWin, mask, attrs, serverClient);
> +}
> +
>   /* Set the region to the intersection of the rectangle and the
>    * window's winSize.  The window is typically the parent of the
>    * window from which the region came.
> diff --git a/include/dix.h b/include/dix.h
> index 34661f3..7a931c3 100644
> --- a/include/dix.h
> +++ b/include/dix.h
> @@ -306,12 +306,19 @@ extern _X_EXPORT void FreeAllAtoms(void);
>
>   extern _X_EXPORT void InitAtoms(void);
>
> +extern void SaveServerAtoms(void);
> +extern void ResetAtoms(void);
> +
>   /* main.c */
>
>   extern _X_EXPORT void SetVendorRelease(int release);
>
>   extern _X_EXPORT void SetVendorString(char *string);
>
> +/* devices.c */
> +
> +extern void ResetInput(void);
> +
>   /* events.c */
>
>   extern void SetMaskForEvent(
> diff --git a/include/misc.h b/include/misc.h
> index dc03911..1097050 100644
> --- a/include/misc.h
> +++ b/include/misc.h
> @@ -357,6 +357,6 @@ typedef struct _CharInfo *CharInfoPtr; /* also in fonts/include/font.h */
>   #endif
>
>   extern _X_EXPORT unsigned long globalSerialNumber;
> -extern _X_EXPORT unsigned long serverGeneration;
> +#define serverGeneration 1
>
>   #endif /* MISC_H */
> diff --git a/include/property.h b/include/property.h
> index 075eb4a..01379ed 100644
> --- a/include/property.h
> +++ b/include/property.h
> @@ -88,4 +88,6 @@ extern _X_EXPORT int DeleteProperty(
>   extern _X_EXPORT void DeleteAllWindowProperties(
>       WindowPtr /*pWin*/);
>
> +extern void ResetProperties(WindowPtr /*pWin*/);
> +
>   #endif  /* PROPERTY_H */
> diff --git a/include/propertyst.h b/include/propertyst.h
> index 1edd11d..73ccce6 100644
> --- a/include/propertyst.h
> +++ b/include/propertyst.h
> @@ -61,6 +61,7 @@ typedef struct _Property {
>          uint32_t        format;     /* format of data for swapping - 8,16,32 */
>          uint32_t        size;       /* size of data in (format/8) bytes */
>          pointer         data;       /* private to client */
> +       unsigned int    retain : 1; /* should this property be retained across server reset? */
>          PrivateRec      *devPrivates;
>   } PropertyRec;
>
> diff --git a/include/window.h b/include/window.h
> index e13598b..1bb51e7 100644
> --- a/include/window.h
> +++ b/include/window.h
> @@ -93,6 +93,9 @@ extern _X_EXPORT Bool CreateRootWindow(
>   extern _X_EXPORT void InitRootWindow(
>       WindowPtr /*pWin*/);
>
> +extern void ResetRootWindow(
> +    WindowPtr /*pWin*/);
> +
>   typedef WindowPtr (* RealChildHeadProc) (WindowPtr pWin);
>
>   extern _X_EXPORT void RegisterRealChildHeadProc (RealChildHeadProc proc);
> --
> 1.7.5.4
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>



More information about the xorg-devel mailing list