Pairing drm_prime_handle_from_fd with close

Pairing drm_prime_handle_from_fd with close

[Thomas Hellstrom]

Hi!

It seems like prime_handle_from_fd needs to be paired with a GEM_CLOSE 
ioctl instead of a new generic HANDLE_CLOSE ioctl.

This oversight is not really a big problem but there are two solutions:
1) Create a new ioctl called HANDLE_CLOSE or something similar.
2) Use the GEM_CLOSE ioctl, but add a driver callback for drivers that 
are not GEM-aware.

It seems like GEM_CLOSE has already spread into user-space clients, so 
unless someone tell me otherwise, I'll be using
2) for the vmwgfx prime implementation.

Thanks,
Thomas

Re: Pairing drm_prime_handle_from_fd with close

[Daniel Vetter]

On Fri, Nov 8, 2013 at 11:02 AM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
> It seems like prime_handle_from_fd needs to be paired with a GEM_CLOSE ioctl
> instead of a new generic HANDLE_CLOSE ioctl.
>
> This oversight is not really a big problem but there are two solutions:
> 1) Create a new ioctl called HANDLE_CLOSE or something similar.
> 2) Use the GEM_CLOSE ioctl, but add a driver callback for drivers that are
> not GEM-aware.
>
> It seems like GEM_CLOSE has already spread into user-space clients, so
> unless someone tell me otherwise, I'll be using
> 2) for the vmwgfx prime implementation.

Oops, I didn't really realize when fixing the prime lifetime stuff
that vmwgfx doesn't use gem handles for it's buffers. What about
3) Move drm_gem_remove_prime_handles to drm_prime.c, expose it to
modules and use that in the vmwgfx handle close ioctl?
Imo GEM_CLOSE shouldn't ever be called by generic code in userspace,
so if that's already broken it should be fixed in userspace not
papered over in the kernel by partially faking gem on vmwgfx. I just
fear that if we start doing that we need to virtualize more of the few
gem bits, which renders the point of gem being optional moot.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: Pairing drm_prime_handle_from_fd with close

[Thomas Hellstrom]

On 11/08/2013 06:19 PM, Daniel Vetter wrote:
> On Fri, Nov 8, 2013 at 11:02 AM, Thomas Hellstrom <thellstrom@vmware.com> wrote:
>> It seems like prime_handle_from_fd needs to be paired with a GEM_CLOSE ioctl
>> instead of a new generic HANDLE_CLOSE ioctl.
>>
>> This oversight is not really a big problem but there are two solutions:
>> 1) Create a new ioctl called HANDLE_CLOSE or something similar.
>> 2) Use the GEM_CLOSE ioctl, but add a driver callback for drivers that are
>> not GEM-aware.
>>
>> It seems like GEM_CLOSE has already spread into user-space clients, so
>> unless someone tell me otherwise, I'll be using
>> 2) for the vmwgfx prime implementation.
> Oops, I didn't really realize when fixing the prime lifetime stuff
> that vmwgfx doesn't use gem handles for it's buffers. What about
> 3) Move drm_gem_remove_prime_handles to drm_prime.c, expose it to
> modules and use that in the vmwgfx handle close ioctl?
> Imo GEM_CLOSE shouldn't ever be called by generic code in userspace,
> so if that's already broken it should be fixed in userspace not
> papered over in the kernel by partially faking gem on vmwgfx. I just
> fear that if we start doing that we need to virtualize more of the few
> gem bits, which renders the point of gem being optional moot.
> -Daniel
Hi!

I'm afraid I don't quite follow you here..
The problem is not in the kernel, with vmwgfx we don't even need to 
populate the drm prime list structures.

The problem is for user-space clients that have a prime fd, and call 
drm_prime_handle_from_fd. Since this function is
pretty generic, they need a generic way to close that handle. I don't 
really have any preferences, except
IMHO user space shouldn't need to guess what's the type of the 
underlying kernel object of that handle..

(This is for generic user-space using prime for buffer sharing between 
processes, not primarily for sharing buffers between devices).

Thanks,
Thomas