[PATCH 5/8] sync_file: add support for a semaphore object

[Chris Wilson]

On Thu, Apr 13, 2017 at 07:41:28AM +1000, Dave Airlie wrote:
> >
> > The problem, as I see it, is that you are taking functionality away from
> > sync_file. If you are wrapping them up inside a sync_file, we have a
> > fair expectation that our code to handle sync_files will continue to
> > work.
> 
> What code? there is no code existing that should be sharing sync objects
> with semaphores backing them, and trying to use them as sync_files.

By masquerading semaphores as sync_files, you are inviting them to be
used with the existing code to handle sync_file.
 
> This is parallel functionality.

And quite different from the existing CPU visible sync_files. Why try
stuffing them through the same interface? At the moment the only thing
they have in common is the single fence pointer, and really you want
them just for their handle/fd to a GPU channel.

After you strip out the fops from the semaphore code, doesn't it boil
down to:

static void semaphore_file_release(struct inode *inode, struct file *file)
{
	struct semaphore_file *s = file->private_data;

	dma_fence_put(s->fence);
	kfree(s);
}

static const struct file_operations semaphore_file_fops = {
	.release = semaphore_file_release,
};

struct semaphore_file *semaphore_file_create(void)
{
	struct semaphore_file *s;

	s = kzalloc(sizeof(*s), GFP_KERNEL);
	if (!s)
		return NULL;

	s->file = anon_inode_getfile("semaphore_"file",
				     &semaphore_file_fops, s, 0);

	return s;
}

Plus your new routines to manipulate the semaphore.

The commonality with the current sync_file {
		struct file *file;
		struct dma_fence *fence;
	};
could be extracted and sync_file become fence_file. Would it not help to
avoid any further confusion by treating them as two very distinct classes
of fd?

And for me to stop calling the user interface sync_file.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre