xf86UnloadSubModule is still buggy

Matthieu Herrb matthieu.herrb at laas.fr
Sun Jun 3 13:39:37 PDT 2012


> commit 0d4bb5442ceb8e8e4a8de6cfc4203cae469eee72
> Author: Michal Suchanek <hramrach at centrum.cz>
> Date:   Sat Oct 8 14:13:33 2011 +0200
>    Unload submodules.
>    Signed-off-by: Michal Suchanek <hramrach at centrum.cz>
>    Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
> 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

This commit is like dropping a live hand grenade in the X server.

UnloadSubModule does not behave like you think it does.

If you look at the code, you'll see that it will unload a submodule
and then recursively unload all siblings (and childrens) of that

The problem is with siblings... generally it doesn't make sense to
unload siblings of the submodule you're unloading.

There is at least one practical case where this is not the expected
behaviour. It's a case dealing with an older driver, but I'm sure
other cases can be found. 

The issue is with the intel driver in non-kms mode that we use on

The driver loads several sub-modules, the first one is vgahw, then it
tries several output driver submodules: "sil164", "ch7xxx", "ivch",
"tfp410", "ch7017", unloading each of them if it's not useful. 

Since "vgahw" is the first loaded submoudle, it became a sibling of
sil164, and gets un-loaded when the code unloads the latter. But since
no-one intened to remove it, there are still tons of references to
that module.

This issue goes unoticed for 2 reasons: most of the submodules were
turned into built-ins recently, and Linux still doesn't overwrite
memory in free() so the X server can keep on using the unloaded
modules for a while.

I suggest to revert that commit untill a proper solution for unloading
only a sub-module is found.

Matthieu Herrb

More information about the xorg-devel mailing list