[PATCH v2] uload unused input modules
Michal Suchanek
hramrach at gmail.com
Tue Jul 3 03:45:21 PDT 2012
Hello,
since the patches got lost somewhere without even generating an error
sending as plain email.
Changes since v1:
- split out module 'query' functions
- rename some variables
- fix some whitespace and braces
Thanks
Michal
>From a86d41ee113f76776a9410205ef6f90c78e41d22 Mon Sep 17 00:00:00 2001
From: Michal Suchanek <hramrach at gmail.com>
Date: Mon, 4 Jun 2012 14:22:14 +0200
Subject: [PATCH 1/4] xfree86/loader: Do not unload sibling modules.
To: xorg-devel at lists.x.org
Ordering of sibling modules is internal to the loader implementation.
Unloading siblings following the requested module in the sibling list
leads to unexpected unloading of seemingly random modules.
Signed-off-by: Michal Suchanek <hramrach at gmail.com>
---
hw/xfree86/loader/loadmod.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 706b9b3..e94265a 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -1103,10 +1103,23 @@ UnloadModuleOrDriver(ModuleDescPtr mod)
LoaderUnload(mod->name, mod->handle);
}
- if (mod->child)
+ while (mod->child)
UnloadModuleOrDriver(mod->child);
- if (mod->sib)
- UnloadModuleOrDriver(mod->sib);
+
+ if (mod->parent) {
+ ModuleDescPtr sib_list = mod->parent->child;
+ if (sib_list == mod) {
+ mod->parent->child = mod->sib;
+ } else {
+ while (sib_list) {
+ if (sib_list->sib == mod) {
+ sib_list->sib = mod->sib;
+ break;
+ }
+ sib_list = sib_list->sib;
+ }
+ }
+ }
free(mod->path);
free(mod->name);
free(mod);
--
1.7.10
>From 04aee58b6b497613a9b0c2c9253cfbcbc3f7e249 Mon Sep 17 00:00:00 2001
From: Michal Suchanek <hramrach at gmail.com>
Date: Mon, 2 Jul 2012 10:54:53 +0200
Subject: [PATCH 2/4] xfree86/loader: Split off some loader code into separate
query functions.
To: xorg-devel at lists.x.org
* Add CanUnloadModule
Signed-off-by: Michal Suchanek <hramrach at gmail.com>
---
hw/xfree86/loader/loaderProcs.h | 5 +++++
hw/xfree86/loader/loadmod.c | 36 ++++++++++++++++++++++++++++++++++--
2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/hw/xfree86/loader/loaderProcs.h b/hw/xfree86/loader/loaderProcs.h
index a7b752b..c33039a 100644
--- a/hw/xfree86/loader/loaderProcs.h
+++ b/hw/xfree86/loader/loaderProcs.h
@@ -88,6 +88,11 @@ unsigned long LoaderGetModuleVersion(ModuleDescPtr mod);
void LoaderResetOptions(void);
void LoaderSetOptions(unsigned long);
+int IsBuiltinModule(ModuleDescPtr mod);
+int IsDuplicated(ModuleDescPtr mod);
+int IsDuplicateModule(ModuleDescPtr mod);
+int CanUnloadModule(ModuleDescPtr mod);
+
/* Options for LoaderSetOptions */
#define LDR_OPT_ABI_MISMATCH_NONFATAL 0x0001
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index e94265a..4844e18 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -803,6 +803,19 @@ NewModuleDesc(const char *name)
return mdp;
}
+/* An advisory function. When true is returned unloading the module should be
+ * safe. Uload can be attempted nonetheless. */
+int CanUnloadModule(ModuleDescPtr mod)
+{
+ if (IsBuiltinModule(mod))
+ return 0;
+ if (IsDuplicated(mod))
+ return 0;
+ /* Neither of the above can be true for modules for which IsDuplicateModule
+ * is true. */
+ return 1;
+}
+
ModuleDescPtr
DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent)
{
@@ -1083,10 +1096,29 @@ UnloadModule(pointer mod)
UnloadModuleOrDriver((ModuleDescPtr) mod);
}
+/* Is given module builtin? */
+int IsBuiltinModule(ModuleDescPtr mod)
+{
+ return (mod == (ModuleDescPtr) 1);
+}
+
+/* Is given module duplicate of another? */
+int IsDuplicateModule(ModuleDescPtr mod)
+{
+ return (mod->TearDownData == ModuleDuplicated);
+}
+
+/* Do duplicates of given module exist? */
+int IsDuplicated(ModuleDescPtr mod)
+{
+ /* cannot tell */
+ return 1;
+}
+
static void
UnloadModuleOrDriver(ModuleDescPtr mod)
{
- if (mod == (ModuleDescPtr) 1)
+ if (IsBuiltinModule(mod))
return;
if (mod == NULL || mod->name == NULL)
@@ -1097,7 +1129,7 @@ UnloadModuleOrDriver(ModuleDescPtr mod)
else
xf86MsgVerb(X_INFO, 3, "UnloadModule: \"%s\"\n", mod->name);
- if (mod->TearDownData != ModuleDuplicated) {
+ if (!IsDuplicateModule(mod)) {
if ((mod->TearDownProc) && (mod->TearDownData))
mod->TearDownProc(mod->TearDownData);
LoaderUnload(mod->name, mod->handle);
--
1.7.10
>From a2bdabbe9e5610b6c0ad80fb3659039da8800970 Mon Sep 17 00:00:00 2001
From: Michal Suchanek <hramrach at gmail.com>
Date: Mon, 4 Jun 2012 17:29:37 +0200
Subject: [PATCH 3/4] xfree86: unload unused input drivers
To: xorg-devel at lists.x.org
Signed-off-by: Michal Suchanek <hramrach at gmail.com>
---
hw/xfree86/common/xf86Helper.c | 22 +++++++++++++++++++++-
hw/xfree86/common/xf86Xinput.c | 4 ++++
hw/xfree86/common/xf86Xinput.h | 2 +-
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/hw/xfree86/common/xf86Helper.c b/hw/xfree86/common/xf86Helper.c
index 5ef1dab..7bf48bf 100644
--- a/hw/xfree86/common/xf86Helper.c
+++ b/hw/xfree86/common/xf86Helper.c
@@ -127,12 +127,32 @@ xf86AddInputDriver(InputDriverPtr driver,
pointer module, int flags)
}
void
-xf86DeleteInputDriver(int drvIndex)
+xf86DeleteInputDriver(InputDriverPtr drv)
{
+ int drvIndex;
+ for (drvIndex = 0; drvIndex < xf86NumInputDrivers; drvIndex ++)
+ if (xf86InputDriverList[drvIndex] == drv) break;
+ if (drvIndex >= xf86NumInputDrivers)
+ return; /* not found */
+
+ /* The loader does not return error from UnloadModule so check that the
+ * driver can be unloaded. */
+ if (xf86InputDriverList[drvIndex] && xf86InputDriverList[drvIndex]->module
+ &&
!CanUnloadModule((ModuleDescPtr)xf86InputDriverList[drvIndex]->module))
+ return; /* cannot unload (yet) */
+
+ /* Remove the driver */
if (xf86InputDriverList[drvIndex] && xf86InputDriverList[drvIndex]->module)
UnloadModule(xf86InputDriverList[drvIndex]->module);
free(xf86InputDriverList[drvIndex]);
xf86InputDriverList[drvIndex] = NULL;
+
+ /* Compact the list of input drivers */
+ if (drvIndex + 1 < xf86NumInputDrivers)
+ memmove(xf86InputDriverList[drvIndex], xf86InputDriverList[drvIndex+1],
+ sizeof(xf86InputDriverList[drvIndex]) *
(xf86NumInputDrivers - drvIndex - 1));
+ xf86NumInputDrivers--;
+ xf86InputDriverList[xf86NumInputDrivers] = NULL;
}
InputDriverPtr
diff --git a/hw/xfree86/common/xf86Xinput.c b/hw/xfree86/common/xf86Xinput.c
index bee407b..ff3b5e9 100644
--- a/hw/xfree86/common/xf86Xinput.c
+++ b/hw/xfree86/common/xf86Xinput.c
@@ -753,6 +753,10 @@ xf86DeleteInput(InputInfoPtr pInp, int flags)
if (pInp->module)
UnloadModule(pInp->module);
+ /* Attempt to unload the driver */
+ if (pInp->drv)
+ xf86DeleteInputDriver(pInp->drv);
+
/* This should *really* be handled in drv->UnInit(dev) call instead, but
* if the driver forgets about it make sure we free it or at least crash
* with flying colors */
diff --git a/hw/xfree86/common/xf86Xinput.h b/hw/xfree86/common/xf86Xinput.h
index 1d4363a..12ce0be 100644
--- a/hw/xfree86/common/xf86Xinput.h
+++ b/hw/xfree86/common/xf86Xinput.h
@@ -180,7 +180,7 @@ InputInfoPtr xf86AllocateInput(void);
/* xf86Helper.c */
extern _X_EXPORT void xf86AddInputDriver(InputDriverPtr driver, pointer module,
int flags);
-extern _X_EXPORT void xf86DeleteInputDriver(int drvIndex);
+extern _X_EXPORT void xf86DeleteInputDriver(InputDriverPtr drv);
extern _X_EXPORT InputDriverPtr xf86LookupInputDriver(const char *name);
extern _X_EXPORT InputInfoPtr xf86LookupInput(const char *name);
extern _X_EXPORT void xf86DeleteInput(InputInfoPtr pInp, int flags);
--
1.7.10
>From 818dd1317dcd356092b00749085ba1b43743e45a Mon Sep 17 00:00:00 2001
From: Michal Suchanek <hramrach at gmail.com>
Date: Mon, 4 Jun 2012 16:46:03 +0200
Subject: [PATCH 4/4] xfree86/loader: add back silly reference counting. (and
move it a layer higher)
To: xorg-devel at lists.x.org
ab7f057ce removes reference counting of loaded objects.
However, DuplicateModule is used extensively in X server to create
copies of modules. Without the reference counting layer DuplicateModule
cannot call LoaderOpen since at least glibc will happily realod
(possibly different) object from disk, and DuplicateModule does not
perform version checks nor module initialization.
To make it possible to unload module safely a duplicate count should be
tracked so that duplicated modules that are possibly still in use
somewhere are not uninitialized. Calling UnloadModule on a duplicated
module has no effect but no error is returned as the loader interface
does not allow for that.
Signed-off-by: Michal Suchanek <hramrach at gmail.com>
---
hw/xfree86/loader/loaderProcs.h | 2 ++
hw/xfree86/loader/loadmod.c | 30 ++++++++++++++++++------------
2 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/hw/xfree86/loader/loaderProcs.h b/hw/xfree86/loader/loaderProcs.h
index c33039a..bdd1328 100644
--- a/hw/xfree86/loader/loaderProcs.h
+++ b/hw/xfree86/loader/loaderProcs.h
@@ -66,6 +66,8 @@ typedef struct module_desc {
ModuleTearDownProc TearDownProc;
void *TearDownData; /* returned from SetupProc */
const XF86ModuleVersionInfo *VersionInfo;
+ struct module_desc *original;
+ int duplicate_count;
} ModuleDesc, *ModuleDescPtr;
/* External API for the loader */
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 4844e18..d3ab51b 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -93,8 +93,6 @@ const ModuleVersions LoaderVersionInfo = {
ABI_FONT_VERSION
};
-static int ModuleDuplicated[] = { };
-
static void
FreeStringList(char **paths)
{
@@ -803,8 +801,11 @@ NewModuleDesc(const char *name)
return mdp;
}
-/* An advisory function. When true is returned unloading the module should be
- * safe. Uload can be attempted nonetheless. */
+static ModuleDescPtr ModuleGetOriginal(ModuleDescPtr mod)
+{
+ return (mod->original ? mod->original : mod);
+}
+
int CanUnloadModule(ModuleDescPtr mod)
{
if (IsBuiltinModule(mod))
@@ -829,10 +830,11 @@ DuplicateModule(ModuleDescPtr mod, ModuleDescPtr parent)
return NULL;
ret->handle = mod->handle;
-
+ /* some code may check the procs are the same */
ret->SetupProc = mod->SetupProc;
ret->TearDownProc = mod->TearDownProc;
- ret->TearDownData = ModuleDuplicated;
+ ret->original = ModuleGetOriginal(mod);
+ ret->original->duplicate_count++;
ret->child = DuplicateModule(mod->child, ret);
ret->sib = DuplicateModule(mod->sib, parent);
ret->parent = parent;
@@ -1105,31 +1107,35 @@ int IsBuiltinModule(ModuleDescPtr mod)
/* Is given module duplicate of another? */
int IsDuplicateModule(ModuleDescPtr mod)
{
- return (mod->TearDownData == ModuleDuplicated);
+ return (mod->original != NULL);
}
/* Do duplicates of given module exist? */
int IsDuplicated(ModuleDescPtr mod)
{
- /* cannot tell */
- return 1;
+ return (mod->duplicate_count > 0);
}
static void
UnloadModuleOrDriver(ModuleDescPtr mod)
{
- if (IsBuiltinModule(mod))
+ if (mod == NULL || mod->name == NULL)
return;
- if (mod == NULL || mod->name == NULL)
+ if (!CanUnloadModule(mod)) {
+ if (IsDuplicated(mod))
+ xf86MsgVerb(X_WARNING, 3, "Cannot unload duplicated
module: \"%s\"\n", mod->name);
return;
+ }
if (mod->parent)
xf86MsgVerb(X_INFO, 3, "UnloadSubModule: \"%s\"\n", mod->name);
else
xf86MsgVerb(X_INFO, 3, "UnloadModule: \"%s\"\n", mod->name);
- if (!IsDuplicateModule(mod)) {
+ if (IsDuplicateModule(mod)) {
+ ModuleGetOriginal(mod)->duplicate_count--;
+ } else {
if ((mod->TearDownProc) && (mod->TearDownData))
mod->TearDownProc(mod->TearDownData);
LoaderUnload(mod->name, mod->handle);
--
1.7.10
More information about the xorg-devel
mailing list