re-adding dri-devel, somehow got lost.
On Tue, Apr 11, 2017 at 8:00 AM, Daniel Vetter <daniel@xxxxxxxx> wrote:
> On Tue, Apr 11, 2017 at 5:06 AM, Dave Airlie <airlied@xxxxxxxxx> wrote:
>>> You can drop the DRM_UNLOCKED, it's the enforced standard for non-legacy
>>> drivers since:
>>>
>>> commit ea487835e8876abf7ad909636e308c801a2bcda6
>>> Author: Daniel Vetter <daniel.vetter@xxxxxxxx>
>>> Date: Mon Sep 28 21:42:40 2015 +0200
>>>
>>> drm: Enforce unlocked ioctl operation for kms driver ioctls
>>
>> This is a core ioctl. not a driver one, so I think I still need UNLOCKED.
>
> Oh right, we need that still for nouveau's legacy ctx usage.
>
>>>> + /* Check if we currently have a reference on the object */
>>>> + sync_file = idr_find(&file_private->syncobj_idr, handle);
>>>
>>> You need to get a reference for the sync_file (hm, should we have
>>> sync_file_get/put helpers?), since otherwise someone might sneak in a
>>> destroy ioctl call right after you drop the lock here and boom.
>>
>> Indeed, fixed locally now.
>>
>>>> + if (ret)
>>>> + return ret;
>>>> +
>>>> + sync_file = sync_file_alloc(type, flags);
>>>> + if (!sync_file)
>>>> + return -ENOMEM;
>>>> +
>>>> + idr_preload(GFP_KERNEL);
>>>> + spin_lock(&file_private->syncobj_table_lock);
>>>> + ret = idr_alloc(&file_private->syncobj_idr, sync_file, 1, 0, GFP_NOWAIT);
>>>> + spin_unlock(&file_private->syncobj_table_lock);
>>>> +
>>>> + idr_preload_end();
>>>> +
>>>> + if (ret < 0)
>>>> + return ret;
>>>> +
>>>> + *handle = ret;
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void drm_syncobj_destroy(struct drm_file *file_private,
>>>> + u32 handle)
>>>> +{
>>>> + struct sync_file *sync_file = drm_syncobj_file_lookup(file_private, handle);
>>>> + if (!sync_file)
>>>> + return;
>>>> +
>>>> + spin_lock(&file_private->syncobj_table_lock);
>>>> + idr_remove(&file_private->syncobj_idr, handle);
>>>> + spin_unlock(&file_private->syncobj_table_lock);
>>>> +
>>>> + fput(sync_file->file);
>>>> +}
>>>> +
>>>> +static int drm_syncobj_handle_to_fd(struct drm_file *file_private,
>>>> + u32 handle, int *fd)
>>>> +{
>>>> + struct sync_file *sync_file = drm_syncobj_file_lookup(file_private, handle);
>>>> + int ret;
>>>> + if (!sync_file)
>>>> + return -EINVAL;
>>>> +
>>>> + ret = get_unused_fd_flags(O_CLOEXEC);
>>>> + if (ret < 0)
>>>> + return ret;
>>>> + fd_install(ret, sync_file->file);
>>>> + *fd = ret;
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int drm_syncobj_fd_to_handle(struct drm_file *file_private,
>>>> + int fd, u32 *handle)
>>>> +{
>>>> + struct sync_file *sync_file = sync_file_fdget(fd);
>>>> + int ret;
>>>> + if (!sync_file)
>>>> + return -EINVAL;
>>>> + idr_preload(GFP_KERNEL);
>>>> + spin_lock(&file_private->syncobj_table_lock);
>>>> + ret = idr_alloc(&file_private->syncobj_idr, sync_file, 1, 0, GFP_NOWAIT);
>>>> + spin_unlock(&file_private->syncobj_table_lock);
>>>> + idr_preload_end();
>>>> +
>>>> + if (ret < 0) {
>>>> + fput(sync_file->file);
>>>> + return ret;
>>>> + }
>>>> + *handle = ret;
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int drm_syncobj_info(struct drm_file *file_private,
>>>> + u32 handle, u32 *type, u32 *flags)
>>>> +{
>>>> + struct sync_file *sync_file = drm_syncobj_file_lookup(file_private, handle);
>>>> +
>>>> + if (!sync_file)
>>>> + return -EINVAL;
>>>> + *type = sync_file->type;
>>>> + *flags = sync_file->flags;
>>>> + return 0;
>>>> +}
>>>
>>> Do we really need this? I'd have assumed that all the drm_syncobj are
>>> guaranteed to be of type sema. Why else would you want to store them in an
>>> idr for future reuse?
>>
>> I'm allowing for future changes to the API, for now we could probably
>> limit syncobj
>> to SEMA, but there is nothing stopping you from importing a sync_file fence
>> into a syncobj and exporting it again, we should consider if we should restrict
>> the drm sync obj ioctls to just sema types for now. Hopefully someone has some
>> opinions here, I'm leaning towards just limiting the initial API to syncobj.
>
> Yeah, makes sense.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
