On Fri, May 14, 2021 at 05:10:29PM +0200, Christian König wrote:
> Am 14.05.21 um 17:03 schrieb Tvrtko Ursulin:
> >
> > On 14/05/2021 15:56, Christian König wrote:
> > > Am 14.05.21 um 16:47 schrieb Tvrtko Ursulin:
> > > >
> > > > On 14/05/2021 14:53, Christian König wrote:
> > > > > >
> > > > > > David also said that you considered sysfs but were wary
> > > > > > of exposing process info in there. To clarify, my patch
> > > > > > is not exposing sysfs entry per process, but one per
> > > > > > open drm fd.
> > > > > >
> > > > >
> > > > > Yes, we discussed this as well, but then rejected the approach.
> > > > >
> > > > > To have useful information related to the open drm fd you
> > > > > need to related that to process(es) which have that file
> > > > > descriptor open. Just tracking who opened it first like DRM
> > > > > does is pretty useless on modern systems.
> > > >
> > > > We do update the pid/name for fds passed over unix sockets.
> > >
> > > Well I just double checked and that is not correct.
> > >
> > > Could be that i915 has some special code for that, but on my laptop
> > > I only see the X server under the "clients" debugfs file.
> >
> > Yes we have special code in i915 for this. Part of this series we are
> > discussing here.
>
> Ah, yeah you should mention that. Could we please separate that into common
> code instead? Cause I really see that as a bug in the current handling
> independent of the discussion here.
>
> As far as I know all IOCTLs go though some common place in DRM anyway.

Yeah, might be good to fix that confusion in debugfs. But since that's
non-uapi, I guess no one ever cared (enough).

> > > > > But an "lsof /dev/dri/renderD128" for example does exactly
> > > > > what top does as well, it iterates over /proc and sees which
> > > > > process has that file open.
> > > >
> > > > Lsof is quite inefficient for this use case. It has to open
> > > > _all_ open files for _all_ processes on the system to find a
> > > > handful of ones which may have the DRM device open.
> > >
> > > Completely agree.
> > >
> > > The key point is you either need to have all references to an open
> > > fd, or at least track whoever last used that fd.
> > >
> > > At least the last time I looked even the fs layer didn't know which
> > > fd is open by which process. So there wasn't really any alternative
> > > to the lsof approach.
> >
> > I asked you about the use case you have in mind which you did not
> > answer. Otherwise I don't understand when do you need to walk all files.
> > What information you want to get?
>
> Per fd debugging information, e.g. instead of the top use case you know
> which process you want to look at.
>
> >
> > For the use case of knowing which DRM file is using how much GPU time on
> > engine X we do not need to walk all open files either with my sysfs
> > approach or the proc approach from Chris. (In the former case we
> > optionally aggregate by PID at presentation time, and in the latter case
> > aggregation is implicit.)
>
> I'm unsure if we should go with the sysfs, proc or some completely different
> approach.
>
> In general it would be nice to have a way to find all the fd references for
> an open inode.

Yeah, but that maybe needs to be an ioctl or syscall or something on the
inode, that givey you a list of (procfd, fd_nr) pairs pointing back at all
open files? If this really is a real world problem, but given that
top/lsof and everyone else hasn't asked for it yet maybe it's not.

Also I replied in some other thread, I really like the fdinfo stuff, and I
think trying to somewhat standardized this across drivers would be neat.
Especially since i915 is going to adopt drm/scheduler for front-end
scheduling too, so at least some of this should be fairly easy to share.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ffwll.ch%2F&data=04%7C01%7CDavid.Nieto%40amd.com%7C3711fdd207484d6bb5fd08d919405eb0%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637568586536251118%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=so13elRn0Ffl7w51QEk%2F%2FXmxOav9n5p6fNXrnDBVY%2B0%3D&reserved=0