nouveau: safen up nouveau_device list usage against concurrent access
I cannot make nouveau_bo_wrap thread-safe (by design), but it seems to be used to convert drm fb's to nouveau_bo's and to get a notify handle from fifo->notify in nv30_screen.c Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@ubuntu.com>main
parent
504d606275
commit
b1d4def059
|
@ -85,6 +85,12 @@ nouveau_device_wrap(int fd, int close, struct nouveau_device **pdev)
|
||||||
|
|
||||||
if (!nvdev)
|
if (!nvdev)
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
|
ret = pthread_mutex_init(&nvdev->lock, NULL);
|
||||||
|
if (ret) {
|
||||||
|
free(nvdev);
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
|
||||||
nvdev->base.fd = fd;
|
nvdev->base.fd = fd;
|
||||||
|
|
||||||
ver = drmGetVersion(fd);
|
ver = drmGetVersion(fd);
|
||||||
|
@ -161,6 +167,7 @@ nouveau_device_del(struct nouveau_device **pdev)
|
||||||
if (nvdev->close)
|
if (nvdev->close)
|
||||||
drmClose(nvdev->base.fd);
|
drmClose(nvdev->base.fd);
|
||||||
free(nvdev->client);
|
free(nvdev->client);
|
||||||
|
pthread_mutex_destroy(&nvdev->lock);
|
||||||
free(nvdev);
|
free(nvdev);
|
||||||
*pdev = NULL;
|
*pdev = NULL;
|
||||||
}
|
}
|
||||||
|
@ -191,6 +198,8 @@ nouveau_client_new(struct nouveau_device *dev, struct nouveau_client **pclient)
|
||||||
int id = 0, i, ret = -ENOMEM;
|
int id = 0, i, ret = -ENOMEM;
|
||||||
uint32_t *clients;
|
uint32_t *clients;
|
||||||
|
|
||||||
|
pthread_mutex_lock(&nvdev->lock);
|
||||||
|
|
||||||
for (i = 0; i < nvdev->nr_client; i++) {
|
for (i = 0; i < nvdev->nr_client; i++) {
|
||||||
id = ffs(nvdev->client[i]) - 1;
|
id = ffs(nvdev->client[i]) - 1;
|
||||||
if (id >= 0)
|
if (id >= 0)
|
||||||
|
@ -199,7 +208,7 @@ nouveau_client_new(struct nouveau_device *dev, struct nouveau_client **pclient)
|
||||||
|
|
||||||
clients = realloc(nvdev->client, sizeof(uint32_t) * (i + 1));
|
clients = realloc(nvdev->client, sizeof(uint32_t) * (i + 1));
|
||||||
if (!clients)
|
if (!clients)
|
||||||
return ret;
|
goto unlock;
|
||||||
nvdev->client = clients;
|
nvdev->client = clients;
|
||||||
nvdev->client[i] = 0;
|
nvdev->client[i] = 0;
|
||||||
nvdev->nr_client++;
|
nvdev->nr_client++;
|
||||||
|
@ -214,6 +223,9 @@ out:
|
||||||
}
|
}
|
||||||
|
|
||||||
*pclient = &pcli->base;
|
*pclient = &pcli->base;
|
||||||
|
|
||||||
|
unlock:
|
||||||
|
pthread_mutex_unlock(&nvdev->lock);
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -225,7 +237,9 @@ nouveau_client_del(struct nouveau_client **pclient)
|
||||||
if (pcli) {
|
if (pcli) {
|
||||||
int id = pcli->base.id;
|
int id = pcli->base.id;
|
||||||
nvdev = nouveau_device(pcli->base.device);
|
nvdev = nouveau_device(pcli->base.device);
|
||||||
|
pthread_mutex_lock(&nvdev->lock);
|
||||||
nvdev->client[id / 32] &= ~(1 << (id % 32));
|
nvdev->client[id / 32] &= ~(1 << (id % 32));
|
||||||
|
pthread_mutex_unlock(&nvdev->lock);
|
||||||
free(pcli->kref);
|
free(pcli->kref);
|
||||||
free(pcli);
|
free(pcli);
|
||||||
}
|
}
|
||||||
|
@ -331,12 +345,43 @@ nouveau_object_find(struct nouveau_object *obj, uint32_t pclass)
|
||||||
static void
|
static void
|
||||||
nouveau_bo_del(struct nouveau_bo *bo)
|
nouveau_bo_del(struct nouveau_bo *bo)
|
||||||
{
|
{
|
||||||
|
struct nouveau_device_priv *nvdev = nouveau_device(bo->device);
|
||||||
struct nouveau_bo_priv *nvbo = nouveau_bo(bo);
|
struct nouveau_bo_priv *nvbo = nouveau_bo(bo);
|
||||||
struct drm_gem_close req = { bo->handle };
|
struct drm_gem_close req = { bo->handle };
|
||||||
DRMLISTDEL(&nvbo->head);
|
|
||||||
|
pthread_mutex_lock(&nvdev->lock);
|
||||||
|
if (nvbo->name) {
|
||||||
|
if (atomic_read(&bo->refcnt)) {
|
||||||
|
/*
|
||||||
|
* bo has been revived by a race with
|
||||||
|
* nouveau_bo_prime_handle_ref, or nouveau_bo_name_ref.
|
||||||
|
*
|
||||||
|
* In theory there's still a race possible with
|
||||||
|
* nouveau_bo_wrap, but when using this function
|
||||||
|
* the lifetime of the handle is probably already
|
||||||
|
* handled in another way. If there are races
|
||||||
|
* you're probably using nouveau_bo_wrap wrong.
|
||||||
|
*/
|
||||||
|
pthread_mutex_unlock(&nvdev->lock);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
DRMLISTDEL(&nvbo->head);
|
||||||
|
/*
|
||||||
|
* This bo has to be closed with the lock held because gem
|
||||||
|
* handles are not refcounted. If a shared bo is closed and
|
||||||
|
* re-opened in another thread a race against
|
||||||
|
* DRM_IOCTL_GEM_OPEN or drmPrimeFDToHandle might cause the
|
||||||
|
* bo to be closed accidentally while re-importing.
|
||||||
|
*/
|
||||||
|
drmIoctl(bo->device->fd, DRM_IOCTL_GEM_CLOSE, &req);
|
||||||
|
pthread_mutex_unlock(&nvdev->lock);
|
||||||
|
} else {
|
||||||
|
DRMLISTDEL(&nvbo->head);
|
||||||
|
pthread_mutex_unlock(&nvdev->lock);
|
||||||
|
drmIoctl(bo->device->fd, DRM_IOCTL_GEM_CLOSE, &req);
|
||||||
|
}
|
||||||
if (bo->map)
|
if (bo->map)
|
||||||
munmap(bo->map, bo->size);
|
munmap(bo->map, bo->size);
|
||||||
drmIoctl(bo->device->fd, DRM_IOCTL_GEM_CLOSE, &req);
|
|
||||||
free(nvbo);
|
free(nvbo);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -363,15 +408,17 @@ nouveau_bo_new(struct nouveau_device *dev, uint32_t flags, uint32_t align,
|
||||||
return ret;
|
return ret;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pthread_mutex_lock(&nvdev->lock);
|
||||||
DRMLISTADD(&nvbo->head, &nvdev->bo_list);
|
DRMLISTADD(&nvbo->head, &nvdev->bo_list);
|
||||||
|
pthread_mutex_unlock(&nvdev->lock);
|
||||||
|
|
||||||
*pbo = bo;
|
*pbo = bo;
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
int
|
int
|
||||||
nouveau_bo_wrap(struct nouveau_device *dev, uint32_t handle,
|
nouveau_bo_wrap_locked(struct nouveau_device *dev, uint32_t handle,
|
||||||
struct nouveau_bo **pbo)
|
struct nouveau_bo **pbo)
|
||||||
{
|
{
|
||||||
struct nouveau_device_priv *nvdev = nouveau_device(dev);
|
struct nouveau_device_priv *nvdev = nouveau_device(dev);
|
||||||
struct drm_nouveau_gem_info req = { .handle = handle };
|
struct drm_nouveau_gem_info req = { .handle = handle };
|
||||||
|
@ -404,6 +451,18 @@ nouveau_bo_wrap(struct nouveau_device *dev, uint32_t handle,
|
||||||
return -ENOMEM;
|
return -ENOMEM;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
int
|
||||||
|
nouveau_bo_wrap(struct nouveau_device *dev, uint32_t handle,
|
||||||
|
struct nouveau_bo **pbo)
|
||||||
|
{
|
||||||
|
struct nouveau_device_priv *nvdev = nouveau_device(dev);
|
||||||
|
int ret;
|
||||||
|
pthread_mutex_lock(&nvdev->lock);
|
||||||
|
ret = nouveau_bo_wrap_locked(dev, handle, pbo);
|
||||||
|
pthread_mutex_unlock(&ndev->lock);
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
|
||||||
int
|
int
|
||||||
nouveau_bo_name_ref(struct nouveau_device *dev, uint32_t name,
|
nouveau_bo_name_ref(struct nouveau_device *dev, uint32_t name,
|
||||||
struct nouveau_bo **pbo)
|
struct nouveau_bo **pbo)
|
||||||
|
@ -413,18 +472,21 @@ nouveau_bo_name_ref(struct nouveau_device *dev, uint32_t name,
|
||||||
struct drm_gem_open req = { .name = name };
|
struct drm_gem_open req = { .name = name };
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
|
pthread_mutex_lock(&nvdev->lock);
|
||||||
DRMLISTFOREACHENTRY(nvbo, &nvdev->bo_list, head) {
|
DRMLISTFOREACHENTRY(nvbo, &nvdev->bo_list, head) {
|
||||||
if (nvbo->name == name) {
|
if (nvbo->name == name) {
|
||||||
*pbo = NULL;
|
*pbo = NULL;
|
||||||
nouveau_bo_ref(&nvbo->base, pbo);
|
nouveau_bo_ref(&nvbo->base, pbo);
|
||||||
|
pthread_mutex_unlock(&nvdev->lock);
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
ret = drmIoctl(dev->fd, DRM_IOCTL_GEM_OPEN, &req);
|
ret = drmIoctl(dev->fd, DRM_IOCTL_GEM_OPEN, &req);
|
||||||
if (ret == 0) {
|
if (ret == 0) {
|
||||||
ret = nouveau_bo_wrap(dev, req.handle, pbo);
|
ret = nouveau_bo_wrap_locked(dev, req.handle, pbo);
|
||||||
nouveau_bo((*pbo))->name = name;
|
nouveau_bo((*pbo))->name = name;
|
||||||
|
pthread_mutex_unlock(&nvdev->lock);
|
||||||
}
|
}
|
||||||
|
|
||||||
return ret;
|
return ret;
|
||||||
|
@ -435,13 +497,16 @@ nouveau_bo_name_get(struct nouveau_bo *bo, uint32_t *name)
|
||||||
{
|
{
|
||||||
struct drm_gem_flink req = { .handle = bo->handle };
|
struct drm_gem_flink req = { .handle = bo->handle };
|
||||||
struct nouveau_bo_priv *nvbo = nouveau_bo(bo);
|
struct nouveau_bo_priv *nvbo = nouveau_bo(bo);
|
||||||
if (!nvbo->name) {
|
|
||||||
int ret = drmIoctl(bo->device->fd, DRM_IOCTL_GEM_FLINK, &req);
|
|
||||||
if (ret)
|
|
||||||
return ret;
|
|
||||||
nvbo->name = req.name;
|
|
||||||
}
|
|
||||||
*name = nvbo->name;
|
*name = nvbo->name;
|
||||||
|
if (!*name || *name == ~0) {
|
||||||
|
int ret = drmIoctl(bo->device->fd, DRM_IOCTL_GEM_FLINK, &req);
|
||||||
|
if (ret) {
|
||||||
|
*name = 0;
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
nvbo->name = *name = req.name;
|
||||||
|
}
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -466,19 +531,18 @@ nouveau_bo_prime_handle_ref(struct nouveau_device *dev, int prime_fd,
|
||||||
int ret;
|
int ret;
|
||||||
unsigned int handle;
|
unsigned int handle;
|
||||||
|
|
||||||
|
nouveau_bo_ref(NULL, bo);
|
||||||
|
|
||||||
|
pthread_mutex_lock(&nvdev->lock);
|
||||||
ret = drmPrimeFDToHandle(dev->fd, prime_fd, &handle);
|
ret = drmPrimeFDToHandle(dev->fd, prime_fd, &handle);
|
||||||
if (ret) {
|
if (ret == 0) {
|
||||||
nouveau_bo_ref(NULL, bo);
|
ret = nouveau_bo_wrap_locked(dev, handle, bo);
|
||||||
return ret;
|
if (!bo->name)
|
||||||
|
// XXX: Force locked DRM_IOCTL_GEM_CLOSE to rule out race conditions
|
||||||
|
bo->name = ~0;
|
||||||
}
|
}
|
||||||
|
pthread_mutex_unlock(&nvdev->lock);
|
||||||
ret = nouveau_bo_wrap(dev, handle, bo);
|
return ret;
|
||||||
if (ret) {
|
|
||||||
nouveau_bo_ref(NULL, bo);
|
|
||||||
return ret;
|
|
||||||
}
|
|
||||||
|
|
||||||
return 0;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
int
|
int
|
||||||
|
@ -490,6 +554,8 @@ nouveau_bo_set_prime(struct nouveau_bo *bo, int *prime_fd)
|
||||||
ret = drmPrimeHandleToFD(bo->device->fd, nvbo->base.handle, DRM_CLOEXEC, prime_fd);
|
ret = drmPrimeHandleToFD(bo->device->fd, nvbo->base.handle, DRM_CLOEXEC, prime_fd);
|
||||||
if (ret)
|
if (ret)
|
||||||
return ret;
|
return ret;
|
||||||
|
if (!nvbo->name)
|
||||||
|
nvbo->name = ~0;
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -3,6 +3,7 @@
|
||||||
|
|
||||||
#include <xf86drm.h>
|
#include <xf86drm.h>
|
||||||
#include <xf86atomic.h>
|
#include <xf86atomic.h>
|
||||||
|
#include <pthread.h>
|
||||||
#include "nouveau_drm.h"
|
#include "nouveau_drm.h"
|
||||||
|
|
||||||
#include "nouveau.h"
|
#include "nouveau.h"
|
||||||
|
@ -94,7 +95,7 @@ nouveau_bo(struct nouveau_bo *bo)
|
||||||
struct nouveau_device_priv {
|
struct nouveau_device_priv {
|
||||||
struct nouveau_device base;
|
struct nouveau_device base;
|
||||||
int close;
|
int close;
|
||||||
atomic_t lock;
|
pthread_mutex_t lock;
|
||||||
struct nouveau_list bo_list;
|
struct nouveau_list bo_list;
|
||||||
uint32_t *client;
|
uint32_t *client;
|
||||||
int nr_client;
|
int nr_client;
|
||||||
|
|
Loading…
Reference in New Issue