On Wed, Jul 20, 2016 at 6:29 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
> On Wed, Jul 20, 2016 at 6:25 PM, Rob Herring <r...@kernel.org> wrote:
>> 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 <r...@kernel.org>
>> Cc: Alexandre Courbot <acour...@nvidia.com>
>> ---
>> 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 68d8317..591cf92 100644
>> --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>> +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c
>> @@ -408,9 +408,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 303ecf1..7dbf66f 100644
>> --- a/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_screen.c
>> @@ -428,9 +428,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 f681631..f789de4 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>> @@ -485,9 +485,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 f90572f..9ad3c57 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);
>
> I haven't looked at any other bits of the change, but this still has
> to be the dupfd. I think that's pscreen->fd in your updated series.
Oh, except you can't do that because the screen hasn't been created
yet. Well, either way, this has to be a dup'd fd.
-ilia
>
>> if (ret)
>> goto err;
>>
>> @@ -136,13 +69,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:
>> @@ -151,8 +78,6 @@ err:
>> } else {
>> nouveau_device_del(&dev);
>> nouveau_drm_del(&drm);
>> - close(dupfd);
>> }
>> - pipe_mutex_unlock(nouveau_screen_mutex);
>> return NULL;
>> }
>> --
>> 2.9.2
>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
