On 10 October 2011 05:38, Peter Hutterer <[email protected]> wrote: > On Fri, Oct 07, 2011 at 02:58:35PM +0200, Michal Suchanek wrote: >> Hello, >> >> The X server "duplicates" modules by loading them again from the disk. >> >> Since this "duplication" happens on every input device hotplug, possibly >> weeks >> after the X server has been started a different module might have been >> installed by that time (eg. due to upgrading distribution packages) the X >> server should check the module every time it is loaded to prevent weird >> issues >> due to modules compiled for different ABI. >> >> I compiled an X server with the patch and it worked but I have no idea how to >> test the error paths. >> >> https://bugs.freedesktop.org/attachment.cgi?id=51585 >> https://bugs.freedesktop.org/show_bug.cgi?id=41182 > > fwiw, last time I tried this (about a year or so ago), the problem was > dlopen(). even if you replace the file, dlopen() will simply re-load the > original file. Not sure if this has been fixed yet but that's what you need > to check first (would be simple to verify with a test program) >
If it reloaded the original file no problem would occur. There was a reference counting layer in the loader which was removed supposedly because dlopen does just that. Anyway, here is much simpler way to fix it. Instead of the previous patch apply 2-5. Applied to 1.11 branch X server and tested that unplugging and replugging a wacom tablet works. That's the only driver I know of for which all devices can be removed. Thanks Michal
From 15e191dd91e09549ea2f96495f9f89ea0fd959ec Mon Sep 17 00:00:00 2001 From: Michal Suchanek <[email protected]> Date: Sat, 8 Oct 2011 14:12:59 +0200 Subject: [PATCH 2/5] Document DuplicateModule function. --- hw/xfree86/doc/ddxDesign.xml | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-) diff --git a/hw/xfree86/doc/ddxDesign.xml b/hw/xfree86/doc/ddxDesign.xml index a4baad5..6afdc56 100644 --- a/hw/xfree86/doc/ddxDesign.xml +++ b/hw/xfree86/doc/ddxDesign.xml @@ -5495,6 +5495,18 @@ typedef struct { <blockquote><para> <programlisting> + pointer DuplicateModule(pointer mod); + </programlisting> + <blockquote><para> + This function creates a copy of the module description. All children are + also copied recursively. When you duplicate a module using this function + you are responsible for ensuring that no duplicated modules are used once + the master copy created with LoadModule or LoadSubModule is unloaded. + </para> + </blockquote></para></blockquote> + + <blockquote><para> + <programlisting> void UnloadModule(pointer mod); </programlisting> <blockquote><para> -- 1.7.6.3
From 4f26662dad12d5c1e20b941e8653b42be8985ef1 Mon Sep 17 00:00:00 2001 From: Michal Suchanek <[email protected]> Date: Sat, 8 Oct 2011 14:13:33 +0200 Subject: [PATCH 3/5] Unload submodules. --- hw/xfree86/common/xf86Helper.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c index a8aa316..4e9bcad 100644 --- a/hw/xfree86/common/xf86Helper.c +++ b/hw/xfree86/common/xf86Helper.c @@ -1643,13 +1643,7 @@ xf86LoadOneModule(char *name, pointer opt) void xf86UnloadSubModule(pointer mod) { - /* - * This is disabled for now. The loader isn't smart enough yet to undo - * relocations. - */ -#if 0 UnloadSubModule(mod); -#endif } Bool -- 1.7.6.3
From b9937bbf6c984f245ca5c980a41e7f39dd5a8d82 Mon Sep 17 00:00:00 2001 From: Michal Suchanek <[email protected]> Date: Sat, 8 Oct 2011 14:19:34 +0200 Subject: [PATCH 4/5] Use UnloadModuleOrDriver for UnloadSubModule. --- hw/xfree86/loader/loadmod.c | 19 +++---------------- 1 files changed, 3 insertions(+), 16 deletions(-) diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c index 9f82099..119d2f4 100644 --- a/hw/xfree86/loader/loadmod.c +++ b/hw/xfree86/loader/loadmod.c @@ -1092,23 +1092,8 @@ UnloadSubModule(pointer _mod) { ModuleDescPtr mod = (ModuleDescPtr)_mod; - if (mod == NULL || mod->name == NULL) - return; - - xf86MsgVerb(X_INFO, 3, "UnloadSubModule: \"%s\"\n", mod->name); - - if ((mod->TearDownProc) && (mod->TearDownData)) - mod->TearDownProc(mod->TearDownData); - LoaderUnload(mod->name, mod->handle); - RemoveChild(mod); - - if (mod->child) - UnloadModuleOrDriver(mod->child); - - free(mod->path); - free(mod->name); - free(mod); + UnloadModuleOrDriver(mod); } static void @@ -1135,6 +1120,8 @@ RemoveChild(ModuleDescPtr child) } if (mdp == child) prevsib->sib = child->sib; + child->parent = NULL; + child->sib = NULL; return; } -- 1.7.6.3
From d24dfb2dea14d1e869f52cf46204e8a869ea336a Mon Sep 17 00:00:00 2001 From: Michal Suchanek <[email protected]> Date: Sat, 8 Oct 2011 14:26:24 +0200 Subject: [PATCH 5/5] Do not uselessly reload modules in DuplicateModule The function does not initialize the module so it has no business loading it. If some user of DuplicateModule expects a module actually loaded they should use LoadModule. --- hw/xfree86/loader/loadmod.c | 20 ++++++++++---------- 1 files changed, 10 insertions(+), 10 deletions(-) diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c index 119d2f4..e6ba729 100644 --- a/hw/xfree86/loader/loadmod.c +++ b/hw/xfree86/loader/loadmod.c @@ -94,6 +94,8 @@ const ModuleVersions LoaderVersionInfo = { ABI_FONT_VERSION }; +static int ModuleDuplicated[] = {}; + static void FreeStringList(char **paths) { @@ -785,7 +787,6 @@ ModuleDescPtr DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent) { ModuleDescPtr ret; - int errmaj, errmin; if (!mod) return NULL; @@ -794,14 +795,11 @@ DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent) if (ret == NULL) return NULL; - if (!(ret->handle = LoaderOpen(mod->path, &errmaj, &errmin))) { - free(ret); - return NULL; - } + ret->handle = mod->handle; ret->SetupProc = mod->SetupProc; ret->TearDownProc = mod->TearDownProc; - ret->TearDownData = NULL; + ret->TearDownData = ModuleDuplicated; ret->child = DuplicateModule(mod->child, ret); ret->sib = DuplicateModule(mod->sib, parent); ret->parent = parent; @@ -1066,7 +1064,7 @@ UnloadModule(pointer mod) static void UnloadModuleOrDriver(ModuleDescPtr mod) { - if (mod == (ModuleDescPtr) 1) + if (mod == (ModuleDescPtr) 1) /* WTF is this? */ return; if (mod == NULL || mod->name == NULL) @@ -1074,9 +1072,11 @@ UnloadModuleOrDriver(ModuleDescPtr mod) xf86MsgVerb(X_INFO, 3, "UnloadModule: \"%s\"\n", mod->name); - if ((mod->TearDownProc) && (mod->TearDownData)) - mod->TearDownProc(mod->TearDownData); - LoaderUnload(mod->name, mod->handle); + if (mod->TearDownData != ModuleDuplicated) { + if ((mod->TearDownProc) && (mod->TearDownData)) + mod->TearDownProc(mod->TearDownData); + LoaderUnload(mod->name, mod->handle); + } if (mod->child) UnloadModuleOrDriver(mod->child); -- 1.7.6.3
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
