[PATCH 1/5] drm: introduce sync objects

[Chris Wilson]

On Wed, Apr 26, 2017 at 01:28:29PM +1000, Dave Airlie wrote:
> +#include <drm/drmP.h>

I wonder if Daniel has already split everything used here into its own
headers?

> +#include <linux/file.h>
> +#include <linux/fs.h>
> +#include <linux/anon_inodes.h>
> +
> +#include "drm_internal.h"
> +#include <drm/drm_syncobj.h>
> +
> +static struct drm_syncobj *drm_syncobj_get(struct drm_file *file_private,
> +					   u32 handle)

I would like this exposed to the driver as well, so I can do handle to
syncobj conversion once. (drm_syncobj_find() if we go with kref_get style
naming.)

> +static struct drm_syncobj *drm_syncobj_fdget(int fd)

It will aslo be useful to export the fd -> syncobj function for drivers. In
the interface Jason has for execbuf, we can substitute the handle for an
fd + flag, which may be more convenient for the user.

> +static int drm_syncobj_handle_to_fd(struct drm_file *file_private,
> +				    u32 handle, int *p_fd)
> +{
> +	struct drm_syncobj *syncobj = drm_syncobj_get(file_private, handle);
> +	int ret;
> +	int fd;
> +
> +	if (!syncobj)
> +		return -EINVAL;
> +
> +	fd = get_unused_fd_flags(O_CLOEXEC);
> +	if (fd < 0) {
> +		drm_syncobj_unreference(syncobj);
> +		return fd;
> +	}
> +
> +	mutex_lock(&syncobj->file_lock);
> +	if (!syncobj->file) {

Mutex only being used to ensure syncobj->file is set once? Is the race
likely? If not, we can spare the mutex and just use a try instead:

	if (!syncobj->file) {
		struct file *file;

		file = anon_inode_getfile("syncobj_file",
					  &drm_syncobj_file_fops,
					  syncobj, 0);
		if (IS_ERR(file))
			return PTR_ERR(file);

		drm_syncobj_get(syncobj);
		if (cmpxchg(&syncobj->file, NULL, file) {
			/* lost the race */
			fput(file);
		}
	}

> +	fd_install(fd, syncobj->file);
> +	mutex_unlock(&syncobj->file_lock);
> +	drm_syncobj_unreference(syncobj);
> +	*p_fd = fd;
> +	return 0;
> +out_unlock:
> +	put_unused_fd(fd);
> +	mutex_unlock(&syncobj->file_lock);
> +	drm_syncobj_unreference(syncobj);
> +	return ret;
> +}
> +

> +/**
> + * drm_gem_object_reference - acquire a GEM BO reference
> + * @obj: GEM buffer object
> + *
> + * This acquires additional reference to @obj. It is illegal to call this
> + * without already holding a reference. No locks required.
> + */
> +static inline void
> +drm_syncobj_reference(struct drm_syncobj *obj)
> +{
> +	kref_get(&obj->refcount);

We've been steadily converting to the kref_get style of nomenclature for
drm object reference handling (i.e. drm_syncobj_get, drm_syncobj_put)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre