[PATCH 2/2] drm: make unplugged flag specific to udl driver

[David Herrmann]

Hi

On Wed, Feb 10, 2016 at 10:46 PM, Stéphane Marchesin
<stephane.marchesin at gmail.com> wrote:
> On Wed, Feb 10, 2016 at 1:38 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
>> Hi
>>
>> On Wed, Feb 10, 2016 at 9:39 PM, Haixia Shi <hshi at chromium.org> wrote:
>>>
>>>> +       if (udl_device_is_unplugged(dev) &&
>>>> +               nr != DRM_IOCTL_NR(DRM_IOCTL_MODE_SETCRTC) &&
>>>> +               nr != DRM_IOCTL_NR(DRM_IOCTL_MODE_RMFB) &&
>>>> +               nr != DRM_IOCTL_NR(DRM_IOCTL_MODE_DESTROY_DUMB))
>>>> +               return -ENODEV;
>>>>
>>>>Why?
>>>>
>>>>Just do:
>>>>
>>>>        if (udl_device_is_unplugged(dev))
>>>>                return -ENODEV;
>>>>
>>>>Why this complex logic here?
>>>
>>> Because there are legitimate ioctl calls after UDL is unplugged. See
>>> crbug.com/583508 and crbug.com/583758 for some background.
>>>
>>> The userspace (Chrome in this case) has allocated FBs and Dumb buffers on
>>> the drm device and needs to be given a chance to properly deallocate them
>>> (via RMFB and DESTROY_DUMB). In addition, it needs to call SETCRTC with
>>> fb_id = 0 to properly release the last refcount on the primary fb.
>>>
>>> I initially proposed adding an "UNPLUG_DISALLOW" flag to ioctls so that we
>>> can whitelist them on a case-by-case basis but that proposal got shot down
>>> as being unnecessary, but you can see my original patch at
>>> https://chromium-review.googlesource.com/#/c/326160/
>>
>> If a device is unplugged, you should consider all your resources to be
>> destroyed. There is no reason to release them manually. User-space
>> *must* be able to deal with asynchronous unplugs.
>
> So the problem if you do that is that things like a buffer's memory
> pages can disappear from under you. How would you deal with that case?
> User space certainly can't have a segfault handler catch that just in
> case :)

If you rip out hardware resources, then you better be able to deal
with it. Sure, UDL is an exception as it doesn't have memory resources
on the chip. But it sounds fishy to me, if you base your API on it. On
a lot of other devices, the memory will simply not be there. So you
cannot keep it around.

There are many ways to invalidate memory mappings. You either unmap
the entire range (and user-space must deal with SIGBUS, which is
completely feasible and a lot of code already does it), or you replace
all with a zero page, or you duplicate all pages, ... IMO, user-space
has to start dealing with hardware unplug properly and stop pretending
it cannot happen.

If you mmap() your filesystem, and you rip out your block device, then
you also will get SIGBUS if you access pages that are not in
pagecache. Why are graphics buffers different?

Thanks
David