As before, you need to keep the dupfd. There's even a long comment there to explain why. <div class="gmail_quote">On Jun 23, 2016 7:58 PM, "Rob Herring" <<a href="mailto:robh@kernel.org">robh@kernel.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Use the common pipe_screen ref counting and fd hashing functions. The mutex can be dropped as the pipe loader protects the create_screen() calls. Signed-off-by: Rob Herring <<a href="mailto:robh@kernel.org">robh@kernel.org</a>> Cc: Alexandre Courbot <<a href="mailto:acourbot@nvidia.com">acourbot@nvidia.com</a>> --- src/gallium/drivers/nouveau/nouveau_screen.c | 6 -- src/gallium/drivers/nouveau/nouveau_screen.h | 4 - src/gallium/drivers/nouveau/nv30/nv30_screen.c | 3 - src/gallium/drivers/nouveau/nv50/nv50_screen.c | 3 - src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 3 - .../winsys/nouveau/drm/nouveau_drm_winsys.c | 89 ++-------------------- 6 files changed, 7 insertions(+), 101 deletions(-) diff --git a/src/gallium/drivers/nouveau/nouveau_screen.c b/src/gallium/drivers/nouveau/nouveau_screen.c index 2c421cc..41d4bef 100644 --- a/src/gallium/drivers/nouveau/nouveau_screen.c +++ b/src/gallium/drivers/nouveau/nouveau_screen.c @@ -159,12 +159,6 @@ nouveau_screen_init(struct nouveau_screen *screen, struct nouveau_device *dev) screen->drm = nouveau_drm(&dev->object); screen->device = dev; - /* - * this is initialized to 1 in nouveau_drm_screen_create after screen - * is fully constructed and added to the global screen list. - */ - screen->refcount = -1; - if (dev->chipset < 0xc0) { data = &nv04_data; size = sizeof(nv04_data); diff --git a/src/gallium/drivers/nouveau/nouveau_screen.h b/src/gallium/drivers/nouveau/nouveau_screen.h index 28c4760..55156c3 100644 --- a/src/gallium/drivers/nouveau/nouveau_screen.h +++ b/src/gallium/drivers/nouveau/nouveau_screen.h @@ -23,8 +23,6 @@ struct nouveau_screen { struct nouveau_client *client; struct nouveau_pushbuf *pushbuf; - int refcount; - unsigned vidmem_bindings; /* PIPE_BIND_* where VRAM placement is desired */ unsigned sysmem_bindings; /* PIPE_BIND_* where GART placement is desired */ unsigned lowmem_bindings; /* PIPE_BIND_* that require an address < 4 GiB */ @@ -119,8 +117,6 @@ nouveau_screen(struct pipe_screen *pscreen) return (struct nouveau_screen *)pscreen; } -bool nouveau_drm_screen_unref(struct nouveau_screen *screen); - bool nouveau_screen_bo_get_handle(struct pipe_screen *pscreen, struct nouveau_bo *bo, diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c b/src/gallium/drivers/nouveau/nv30/nv30_screen.c index de798cf..08194fd 100644 --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c @@ -406,9 +406,6 @@ nv30_screen_destroy(struct pipe_screen *pscreen) { struct nv30_screen *screen = nv30_screen(pscreen); - if (!nouveau_drm_screen_unref(&screen->base)) - return; - if (screen->base.fence.current) { struct nouveau_fence *current = NULL; diff --git a/src/gallium/drivers/nouveau/nv50/nv50_screen.c b/src/gallium/drivers/nouveau/nv50/nv50_screen.c index 03f4591..1654d0e 100644 --- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c +++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c @@ -425,9 +425,6 @@ nv50_screen_destroy(struct pipe_screen *pscreen) { struct nv50_screen *screen = nv50_screen(pscreen); - if (!nouveau_drm_screen_unref(&screen->base)) - return; - if (screen->base.fence.current) { struct nouveau_fence *current = NULL; diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c index b9437b2..712293a 100644 --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c @@ -481,9 +481,6 @@ nvc0_screen_destroy(struct pipe_screen *pscreen) { struct nvc0_screen *screen = nvc0_screen(pscreen); - if (!nouveau_drm_screen_unref(&screen->base)) - return; - if (screen->base.fence.current) { struct nouveau_fence *current = NULL; diff --git a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c index 598ffcb..b5f4247 100644 --- a/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c +++ b/src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c @@ -1,12 +1,9 @@ -#include <sys/stat.h> #include <unistd.h> #include "pipe/p_context.h" #include "pipe/p_state.h" #include "util/u_format.h" #include "util/u_memory.h" -#include "util/u_inlines.h" -#include "util/u_hash_table.h" -#include "os/os_thread.h" +#include "util/u_screen.h" #include "nouveau_drm_public.h" @@ -16,47 +13,6 @@ #include <nvif/class.h> #include <nvif/cl0080.h> -static struct util_hash_table *fd_tab = NULL; - -pipe_static_mutex(nouveau_screen_mutex); - -bool nouveau_drm_screen_unref(struct nouveau_screen *screen) -{ - int ret; - if (screen->refcount == -1) - return true; - - pipe_mutex_lock(nouveau_screen_mutex); - ret = --screen->refcount; - assert(ret >= 0); - if (ret == 0) - util_hash_table_remove(fd_tab, intptr_to_pointer(screen->drm->fd)); - pipe_mutex_unlock(nouveau_screen_mutex); - return ret == 0; -} - -static unsigned hash_fd(void *key) -{ - int fd = pointer_to_intptr(key); - struct stat stat; - fstat(fd, &stat); - - return stat.st_dev ^ stat.st_ino ^ stat.st_rdev; -} - -static int compare_fd(void *key1, void *key2) -{ - int fd1 = pointer_to_intptr(key1); - int fd2 = pointer_to_intptr(key2); - struct stat stat1, stat2; - fstat(fd1, &stat1); - fstat(fd2, &stat2); - - return stat1.st_dev != stat2.st_dev || - stat1.st_ino != stat2.st_ino || - stat1.st_rdev != stat2.st_rdev; -} - PUBLIC struct pipe_screen * nouveau_drm_screen_create(int fd) { @@ -64,36 +20,13 @@ nouveau_drm_screen_create(int fd) struct nouveau_device *dev = NULL; struct nouveau_screen *(*init)(struct nouveau_device *); struct nouveau_screen *screen = NULL; - int ret, dupfd; - - pipe_mutex_lock(nouveau_screen_mutex); - if (!fd_tab) { - fd_tab = util_hash_table_create(hash_fd, compare_fd); - if (!fd_tab) { - pipe_mutex_unlock(nouveau_screen_mutex); - return NULL; - } - } - - screen = util_hash_table_get(fd_tab, intptr_to_pointer(fd)); - if (screen) { - screen->refcount++; - pipe_mutex_unlock(nouveau_screen_mutex); - return &screen->base; - } + struct pipe_screen *pscreen = pipe_screen_reference(fd); + int ret; - /* Since the screen re-use is based on the device node and not the fd, - * create a copy of the fd to be owned by the device. Otherwise a - * scenario could occur where two screens are created, and the first - * one is shut down, along with the fd being closed. The second - * (identical) screen would now have a reference to the closed fd. We - * avoid this by duplicating the original fd. Note that - * nouveau_device_wrap does not close the fd in case of a device - * creation error. - */ - dupfd = dup(fd); + if (pscreen) + return pscreen; - ret = nouveau_drm_new(dupfd, &drm); + ret = nouveau_drm_new(fd, &drm); if (ret) goto err; @@ -135,13 +68,7 @@ nouveau_drm_screen_create(int fd) if (!screen || !screen->base.context_create) goto err; - /* Use dupfd in hash table, to avoid errors if the original fd gets - * closed by its owner. The hash key needs to live at least as long as - * the screen. - */ - util_hash_table_set(fd_tab, intptr_to_pointer(dupfd), screen); - screen->refcount = 1; - pipe_mutex_unlock(nouveau_screen_mutex); + pipe_screen_reference_init(&screen->base, fd); return &screen->base; err: @@ -150,8 +77,6 @@ err: } else { nouveau_device_del(&dev); nouveau_drm_del(&drm); - close(dupfd); } - pipe_mutex_unlock(nouveau_screen_mutex); return NULL; } -- 2.9.0 </blockquote></div>