lkml.org 
[lkml]   [2022]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [RFC 02/17] drm: Track clients per owning process
From

On 20/10/2022 12:33, Christian König wrote:
> Am 20.10.22 um 09:34 schrieb Tvrtko Ursulin:
>>
>> On 20/10/2022 07:40, Christian König wrote:
>>> Am 19.10.22 um 19:32 schrieb Tvrtko Ursulin:
>>>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>>>
>>>> To enable propagation of settings from the cgroup drm controller to
>>>> drm we
>>>> need to start tracking which processes own which drm clients.
>>>>
>>>> Implement that by tracking the struct pid pointer of the owning
>>>> process in
>>>> a new XArray, pointing to a structure containing a list of associated
>>>> struct drm_file pointers.
>>>>
>>>> Clients are added and removed under the filelist mutex and RCU list
>>>> operations are used below it to allow for lockless lookup.
>>>
>>> That won't work easily like this. The problem is that file_priv->pid
>>> is usually not accurate these days:
>>>
>>>  From the debugfs clients file:
>>>
>>>        systemd-logind   773   0   y    y     0          0
>>>                  Xorg  1639 128   n    n  1000          0
>>>                  Xorg  1639 128   n    n  1000          0
>>>                  Xorg  1639 128   n    n  1000          0
>>>               firefox  2945 128   n    n  1000          0
>>>                  Xorg  1639 128   n    n  1000          0
>>>                  Xorg  1639 128   n    n  1000          0
>>>                  Xorg  1639 128   n    n  1000          0
>>>                  Xorg  1639 128   n    n  1000          0
>>>                chrome 35940 128   n    n  1000          0
>>>                chrome 35940   0   n    y  1000          1
>>>                chrome 35940   0   n    y  1000          2
>>>                  Xorg  1639 128   n    n  1000          0
>>>                  Xorg  1639 128   n    n  1000          0
>>>                  Xorg  1639 128   n    n  1000          0
>>>
>>> This is with glxgears and a bunch other OpenGL applications running.
>>>
>>> The problem is that for most applications the X/Wayland server is now
>>> opening the render node. The only exceptions in this case are apps
>>> using DRI2 (VA-API?).
>>>
>>> I always wanted to fix this and actually track who is using the file
>>> descriptor instead of who opened it, but never had the time to do this.
>>
>> There's a patch later in the series which allows client records to be
>> migrated to a new PID, and then i915 patch to do that when fd is used
>> for context create. That approach I think worked well enough in the
>> past. So maybe it could be done in the DRM core at some suitable entry
>> point.
>
> Yeah, that makes some sense. I think you should wire that inside
> drm_ioctl(), as far as I know more or less all uses of a file descriptor
> would go through that function.
>
> And maybe make that a stand alone patch, cause that can go upstream as a
> bug fix independently if you ask me.

I've put it on my todo list to try and come up with something standalone
for this problem. Will see if I manage to send it separately or perhaps
will start the next cgroup controller RFC with those patches.

>>> I think you need to fix this problem first. And BTW: and unsigned
>>> long doesn't work as PID either with containers.
>>
>> This I am not familiar with so would like to hear more if you could
>> point me in the right direction at least.
>
> Uff, I'm the wrong person to ask stuff like that. I just can say from
> experience because I've ran into that trap as well.
>
>>
>> My assumption was that struct pid *, which is what I store in unsigned
>> long, would be unique in a system where there is a single kernel
>> running, so as long as lifetimes are correct (released from tracking
>> here when fd is closed, which is implicit on process exit) would work.
>> You are suggesting that is not so?
>
> I think you should have the pointer to struct pid directly here since
> that is a reference counted structure IIRC. But don't ask me what the
> semantics is how to get or put a reference.

Yeah I think I have all that. I track struct pid, with a reference, in
drm client, and release it when file descriptor is closed (indirectly
via the DRM close hook). All I need, I think, is for that mapping to
answer me "which drm_file objects" are in use by this struct pid
pointer. I don't see a problem with lifetimes or scope yet.

Regards,

Tvrtko

\
 
 \ /
  Last update: 2022-10-27 16:38    [W:0.084 / U:1.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site