On 06/05/2015 03:50 PM, Ilia Mirkin wrote:
This scheme is copied from radeon, does it need a similar fix? I'm away from computers for another week or so, will be able to look then.
For some reason, no. Testing on Radeon multi-x-screen ZaphodHeads never showed such problems over multiple years and many mesa versions. Apparently it does something slightly different, although i saw the hashing logic with fstat() is the same. Maybe correct by slightly different design, or maybe we just get lucky because the same fd's aren't recycled on successive glXQueryVersion() calls?
thanks, -mario
On Jun 5, 2015 4:37 PM, "Mario Kleiner" <mario.kleiner...@gmail.com <mailto:mario.kleiner...@gmail.com>> wrote: The dup'ed fd owned by the nouveau_screen for a device node must also be used as key for the winsys hash table, instead of using the original fd passed in for a screen, to make multi-x-screen ZaphodHeads configurations work on nouveau. This prevents the following crash scenario that was observed when a dynamically loaded rendering plugin used OpenGL on a ZaphodHeads setup, e.g., on a dual x-screen setup. At first load the plugin worked, but after unloading and reloading it, the next rendering operation crashed: 1. Client, e.g., a plugin, calls glXQueryVersion. 2. DRI screens for the x-screens 0 and 1 are created, one shared nouveau_screen is created for the shared device node of both screens, but the original fd of x-screen 0 is used as identifying key in the hash table, instead of the dup()ed fd of x-screen 0 which is owned by the nouveau_screen. nouveau_screen's refcount is now 2. 3. Regular rendering happens by the client plugin, then the plugin gets unloaded. 4. XCloseDisplay(). x-screen 0 gets its DRI screen destroyed, nouveau_drm_screen_unref() drops the refcount to 1, calling mesa code then closes the fd of x-screen 0, so now the fd which is used as key in the hash table is invalid. x-screen 1 gets destroyed, nouveau_drm_screen_unref() drops the refcount to 0, the nouveau_screen gets destroyed, but removal of its entry in the hash table fails, because the invalid fd in the hash table no longer matches anything (fstat() on the fd is used for hashing and key comparison, but fstat() on an already closed fd fails and returns bogus results). x-screen 1 closes its fd. Now all fd's are closed, the nouveau_screen destroyed, but there is a dangling reference to the nouveau_screen in the hash table. 5. Some OpenGL client plugin gets loaded again and calls glXQueryVersion. Step 2 above repeats, but because a dangling reference with a matching fd is found in the winsys hash table, no new nouveau_screen is created this time. Instead the invalid pointer to the old nouveau_screen is recycled, which points to nirvana -> Crash. This problem is avoided by use of the dup()ed fd which is owned by the nouveau_screen and has the same lifetime as the nouveau_screen itself. Cc: "10.3 10.4 10.5 10.6" <mesa-sta...@lists.freedesktop.org <mailto:mesa-sta...@lists.freedesktop.org>> Signed-off-by: Mario Kleiner <mario.kleiner...@gmail.com <mailto:mario.kleiner...@gmail.com>> Cc: Ilia Mirkin <imir...@alum.mit.edu <mailto:imir...@alum.mit.edu>> --- src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c index 0635246..dbc3cae 100644 --- a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c +++ b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c @@ -120,7 +120,8 @@ nouveau_drm_screen_create(int fd) if (!screen) goto err; - util_hash_table_set(fd_tab, intptr_to_pointer(fd), screen); + /* Use dupfd in hash table, to avoid crashes in ZaphodHeads configs */ + util_hash_table_set(fd_tab, intptr_to_pointer(dupfd), screen); screen->refcount = 1; pipe_mutex_unlock(nouveau_screen_mutex); return &screen->base; -- 2.1.4
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
