[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#686152: xorg server 1.12.3: xf86UnloadSubModule() hangs, patch proposal




A patch proposal.

The unpatched code is (hw/xfree86/loader/loadmod.c):

static void
UnloadModuleOrDriver(ModuleDescPtr mod)
{
    if (mod == (ModuleDescPtr) 1)
        return;

    if (mod == NULL || mod->name == NULL)
        return;

    if (mod->parent)
        xf86MsgVerb(X_INFO, 3, "UnloadSubModule: \"%s\"\n", mod->name);
    else
        xf86MsgVerb(X_INFO, 3, "UnloadModule: \"%s\"\n", mod->name);

    if (mod->TearDownData != ModuleDuplicated) {
        if ((mod->TearDownProc) && (mod->TearDownData))
            mod->TearDownProc(mod->TearDownData);
        LoaderUnload(mod->name, mod->handle);
    }

    if (mod->child)
        UnloadModuleOrDriver(mod->child);
    if (mod->sib)
        UnloadModuleOrDriver(mod->sib);
    free(mod->path);
    free(mod->name);
    free(mod);
}

void
UnloadSubModule(pointer _mod)
{
    ModuleDescPtr mod = (ModuleDescPtr) _mod;

    /* Some drivers are calling us on built-in submodules, ignore them */
    if (mod == (ModuleDescPtr) 1)
        return;
    RemoveChild(mod);
    UnloadModuleOrDriver(mod);
}

static void
RemoveChild(ModuleDescPtr child)
{
    ModuleDescPtr mdp;
    ModuleDescPtr prevsib;
    ModuleDescPtr parent;

    if (!child->parent)
        return;

    parent = child->parent;
    if (parent->child == child) {
        parent->child = child->sib;
        return;
    }

    prevsib = parent->child;
    mdp = prevsib->sib;
    while (mdp && mdp != child) {
        prevsib = mdp;
        mdp = mdp->sib;
    }
    if (mdp == child)
        prevsib->sib = child->sib;
    child->sib = NULL;
    return;
}


The UnloadSubModule() function is called by xf86UnloadSubModule() as you can see in my first report. UnloadSubModule() first calls RemoveChild(), finally UnloadModuleOrDriver() for the submodule pointer to unload it.

As you can see, each ModuleDescPtr instance maintains its childs:
ptr->child is the first child,
ptr->child->sib is the second child,
prt->child->sib->sib is the third child
as long as ->child or ->sib isn't NULL.

The idea seems to be that UnloadSubModule() deals with modules that didn't unload their childs - in the TearDownProc that UnloadModuleOrDriver() calls. Thus, UnloadModuleOrDriver() has the code lines which unload all childs and grand childs etc. recursively:
    if (mod->child)
        UnloadModuleOrDriver(mod->child);
    if (mod->sib)
        UnloadModuleOrDriver(mod->sib);

Note that the call of UnloadModuleOrDriver(mod->sib) is needed for that recursive unload.

The top-level instance of UnloadModuleOrDriver() gets the module pointer which UnloadSubModule() had. The UnloadModuleOrDriver() function must not unload mod->sib for that top-level module pointer because it is a sibling of the module but not a child. The call of UnloadModuleOrDriver(mod->sib) can't be removed because it would break the mentioned recursive unload.

Thus, RemoveChild() sets ->sib to NULL for the module pointer that it gets.

When UnloadSubModule() calls RemoveChild(), then UnloadModuleOrDriver() for the submodule to unload, everything works fine, because UnloadModuleOrDriver() detects the NULL ->sib pointer and won't call UnloadModuleOrDriver() in recursive manner for it.


The real bug is in RemoveChild() - it doesn't set ->sib to NULL if the module is the first child of its parent. This causes wrong and multiple unload attempts for particular submodules what hangs up the xorg server.


The patch corrects the RemoveChild() function:

static void
RemoveChild(ModuleDescPtr child)
{
    ModuleDescPtr mdp;
    ModuleDescPtr prevsib;
    ModuleDescPtr parent;

    if (!child->parent)
        return;

    parent = child->parent;
    if (parent->child == child) {
        parent->child = child->sib;
    }
    else {
        prevsib = parent->child;
        mdp = prevsib->sib;
        while (mdp && mdp != child) {
            prevsib = mdp;
            mdp = mdp->sib;
        }
        if (mdp == child)
            prevsib->sib = child->sib;
    }
    child->sib = NULL;
}




I built the xorg-xserver package with the patch (and the ones of bug#685750 and 686153).

It works; the resulting Xorg.0.log is:

[    31.839]
X.Org X Server 1.12.3
Release Date: 2012-07-09
[    31.839] X Protocol Version 11, Revision 0
[    31.839] Build Operating System: Linux 3.2.0-3-mckinley ia64 Debian
[ 31.839] Current Operating System: Linux itanic 3.2.0-3-mckinley #1 SMP Sat Aug 18 20:50:43 UTC 2012 ia64 [ 31.839] Kernel command line: BOOT_IMAGE=scsi0:/EFI/debian/vmlinuz root=/dev/sdb5 ro
[    31.839] Build Date: 05 September 2012  06:41:52PM
[    31.839] xorg-server 2:1.12.3-1 (Julien Cristau <jcristau@debian.org>)
[    31.839] Current version of pixman: 0.26.0
[    31.839] 	Before reporting problems, check http://wiki.x.org
	to make sure that you have the latest version.
[ 31.840] Markers: (--) probed, (**) from config file, (==) default setting,
	(++) from command line, (!!) notice, (II) informational,
	(WW) warning, (EE) error, (NI) not implemented, (??) unknown.
[ 31.840] (==) Log file: "/var/log/Xorg.0.log", Time: Wed Sep 5 19:24:48 2012
[    31.886] (==) Using system config directory "/usr/share/X11/xorg.conf.d"
[    31.914] (==) No Layout section.  Using the first Screen section.
[    31.914] (==) No screen section available. Using defaults.
[    31.914] (**) |-->Screen "Default Screen Section" (0)
[    31.914] (**) |   |-->Monitor "<default monitor>"
[    31.915] (==) No monitor specified for screen "Default Screen Section".
	Using a default monitor configuration.
[    31.915] (==) Automatically adding devices
[    31.915] (==) Automatically enabling devices
[ 31.955] (WW) The directory "/usr/share/fonts/X11/cyrillic" does not exist.
[    31.955] 	Entry deleted from font path.
[    31.955] (WW) The directory "/usr/share/fonts/X11/100dpi/" does not exist.
[    31.955] 	Entry deleted from font path.
[    31.955] (WW) The directory "/usr/share/fonts/X11/75dpi/" does not exist.
[    31.955] 	Entry deleted from font path.
[    31.955] (WW) The directory "/usr/share/fonts/X11/Type1" does not exist.
[    31.955] 	Entry deleted from font path.
[    31.955] (WW) The directory "/usr/share/fonts/X11/100dpi" does not exist.
[    31.955] 	Entry deleted from font path.
[    31.955] (WW) The directory "/usr/share/fonts/X11/75dpi" does not exist.
[    31.955] 	Entry deleted from font path.
[ 31.955] (WW) The directory "/var/lib/defoma/x-ttcidfont-conf.d/dirs/TrueType" does not exist.
[    31.955] 	Entry deleted from font path.
[    31.956] (==) FontPath set to:
	/usr/share/fonts/X11/misc,
	built-ins
[    31.956] (==) ModulePath set to "/usr/lib/xorg/modules"
[ 31.956] (II) The server relies on udev to provide the list of input devices.
	If no devices become available, reconfigure udev or disable AutoAddDevices.
[    31.956] (II) Loader magic: 0x20000008004321b0
[    31.956] (II) Module ABI versions:
[    31.956] 	X.Org ANSI C Emulation: 0.4
[    31.956] 	X.Org Video Driver: 12.0
[    31.956] 	X.Org XInput driver : 16.0
[    31.956] 	X.Org Server Extension : 6.0
[ 31.965] (--) PCI:*(0:1:1:0) 1002:4752:8086:3404 rev 39, Mem @ 0xfb000000/16777216, 0xfaff0000/4096, I/O @ 0x0000cc00/256, BIOS @ 0x????????/131072
[    31.966] (II) Open ACPI successful (/var/run/acpid.socket)
[    31.966] (II) LoadModule: "extmod"
[    31.994] (II) Loading /usr/lib/xorg/modules/extensions/libextmod.so
[    32.004] (II) Module extmod: vendor="X.Org Foundation"
[    32.004] 	compiled for 1.12.3, module version = 1.0.0
[    32.004] 	Module class: X.Org Server Extension
[    32.004] 	ABI class: X.Org Server Extension, version 6.0
[    32.004] (II) Loading extension SELinux
[    32.004] (II) Loading extension MIT-SCREEN-SAVER
[    32.004] (II) Loading extension XFree86-VidModeExtension
[    32.004] (II) Loading extension XFree86-DGA
[    32.004] (II) Loading extension DPMS
[    32.004] (II) Loading extension XVideo
[    32.004] (II) Loading extension XVideo-MotionCompensation
[    32.004] (II) Loading extension X-Resource
[    32.004] (II) LoadModule: "dbe"
[    32.004] (II) Loading /usr/lib/xorg/modules/extensions/libdbe.so
[    32.005] (II) Module dbe: vendor="X.Org Foundation"
[    32.005] 	compiled for 1.12.3, module version = 1.0.0
[    32.005] 	Module class: X.Org Server Extension
[    32.005] 	ABI class: X.Org Server Extension, version 6.0
[    32.005] (II) Loading extension DOUBLE-BUFFER
[    32.005] (II) LoadModule: "glx"
[    32.005] (II) Loading /usr/lib/xorg/modules/extensions/libglx.so
[    32.040] (II) Module glx: vendor="X.Org Foundation"
[    32.040] 	compiled for 1.12.3, module version = 1.0.0
[    32.040] 	ABI class: X.Org Server Extension, version 6.0
[    32.040] (==) AIGLX enabled
[    32.040] (II) Loading extension GLX
[    32.040] (II) LoadModule: "record"
[    32.041] (II) Loading /usr/lib/xorg/modules/extensions/librecord.so
[    32.041] (II) Module record: vendor="X.Org Foundation"
[    32.041] 	compiled for 1.12.3, module version = 1.13.0
[    32.041] 	Module class: X.Org Server Extension
[    32.041] 	ABI class: X.Org Server Extension, version 6.0
[    32.041] (II) Loading extension RECORD
[    32.041] (II) LoadModule: "dri"
[    32.042] (II) Loading /usr/lib/xorg/modules/extensions/libdri.so
[    32.061] (II) Module dri: vendor="X.Org Foundation"
[    32.061] 	compiled for 1.12.3, module version = 1.0.0
[    32.061] 	ABI class: X.Org Server Extension, version 6.0
[    32.061] (II) Loading extension XFree86-DRI
[    32.061] (II) LoadModule: "dri2"
[    32.061] (II) Loading /usr/lib/xorg/modules/extensions/libdri2.so
[    32.069] (II) Module dri2: vendor="X.Org Foundation"
[    32.069] 	compiled for 1.12.3, module version = 1.2.0
[    32.069] 	ABI class: X.Org Server Extension, version 6.0
[    32.069] (II) Loading extension DRI2
[    32.070] (==) Matched ati as autoconfigured driver 0
[    32.070] (==) Matched fbdev as autoconfigured driver 1
[    32.070] (==) Assigned the driver to the xf86ConfigLayout
[    32.070] (II) LoadModule: "ati"
[    32.074] (II) Loading /usr/lib/xorg/modules/drivers/ati_drv.so
[    32.080] (II) Module ati: vendor="X.Org Foundation"
[    32.099] 	compiled for 1.12.1.902, module version = 6.14.4
[    32.099] 	Module class: X.Org Video Driver
[    32.099] 	ABI class: X.Org Video Driver, version 12.0
[    32.099] (II) LoadModule: "mach64"
[    32.100] (II) Loading /usr/lib/xorg/modules/drivers/mach64_drv.so
[    32.122] (II) Module mach64: vendor="X.Org Foundation"
[    32.122] 	compiled for 1.12.3, module version = 6.9.1
[    32.122] 	Module class: X.Org Video Driver
[    32.122] 	ABI class: X.Org Video Driver, version 12.0
[    32.128] (II) LoadModule: "fbdev"
[    32.128] (WW) Warning, couldn't open module fbdev
[    32.128] (II) UnloadModule: "fbdev"
[    32.128] (II) Unloading fbdev
[    32.128] (EE) Failed to load module "fbdev" (module does not exist, 0)
[    32.128] (II) MACH64: Driver for ATI Mach64 chipsets
[    32.128] (--) using VT number 7

[ 32.132] (II) MACH64(0): Creating default Display subsection in Screen section
	"Default Screen Section" for depth/fbbpp 24/32
[    32.132] (==) MACH64(0): Depth 24, (--) framebuffer bpp 32
[    32.132] (==) MACH64(0): Using XAA acceleration architecture
[    32.132] (II) MACH64: Mach64 in slot 1:1:0 detected.
[    32.132] (II) Loading sub module "int10"
[    32.132] (II) LoadModule: "int10"
[    32.132] (II) Loading /usr/lib/xorg/modules/libint10.so
[    32.173] (II) Module int10: vendor="X.Org Foundation"
[    32.173] 	compiled for 1.12.3, module version = 1.0.0
[    32.173] 	ABI class: X.Org Video Driver, version 12.0
[    32.177] (II) Loading sub module "ddc"
[    32.177] (II) LoadModule: "ddc"
[    32.177] (II) Module "ddc" already built-in
[    32.177] (II) Loading sub module "vbe"
[    32.177] (II) LoadModule: "vbe"
[    32.177] (II) Loading /usr/lib/xorg/modules/libvbe.so
[    32.185] (II) Module vbe: vendor="X.Org Foundation"
[    32.185] 	compiled for 1.12.3, module version = 1.1.0
[    32.185] 	ABI class: X.Org Video Driver, version 12.0
[    32.185] (II) MACH64(0): VESA BIOS not detected
[    32.185] (II) UnloadSubModule: "vbe"
[    32.185] (II) Unloading vbe
[    32.185] (II) UnloadSubModule: "int10"
[    32.185] (II) Unloading int10
[    32.185] (II) MACH64(0): BIOS Data:  BIOSSize=0x8000, ROMTable=0x0114.
[ 32.185] (II) MACH64(0): BIOS Data: ClockTable=0x097C, FrequencyTable=0x0000.
[    32.185] (II) MACH64(0): BIOS Data:  LCDTable=0x0000.
[ 32.185] (II) MACH64(0): BIOS Data: VideoTable=0x0000, HardwareTable=0x015E. [ 32.185] (II) MACH64(0): BIOS Data: I2CType=0x0F, Tuner=0x00, Decoder=0x00, Audio=0x0F. [ 32.185] (--) MACH64(0): ATI 3D Rage XL or XC graphics controller detected. [ 32.185] (--) MACH64(0): Chip type 4752 "GR", version 7, foundry TSMC, class 0, revision 0x00. [ 32.185] (--) MACH64(0): PCI bus interface detected; block I/O base is 0xCC00.
[    32.185] (--) MACH64(0): ATI Mach64 adapter detected.
[ 32.185] (!!) MACH64(0): For information on using the multimedia capabilities
	of this adapter, please see http://gatos.sf.net.
[    32.185] (--) MACH64(0): Internal RAMDAC (subtype 1) detected.
[    32.185] (==) MACH64(0): RGB weight 888
[    32.185] (==) MACH64(0): Default visual is TrueColor
....



It includes successful submodule unloads now:
[    32.185] (II) UnloadSubModule: "vbe"
[    32.185] (II) Unloading vbe
[    32.185] (II) UnloadSubModule: "int10"
[    32.185] (II) Unloading int10



Kind regards
Stephan Schreiber



Attachment: unloadsubmodule.patch
Description: unloadsubmodule.patch


Reply to: