[Intel-gfx] [PATCH 10/24] drm: Remove drm_pending_event->pid

[Daniel Vetter]

On Mon, Mar 13, 2017 at 01:05:27PM -0400, Sean Paul wrote:
> On Wed, Mar 08, 2017 at 03:12:43PM +0100, Daniel Vetter wrote:
> > We might as well dump the drm_file pointer, that's about as useful
> > a cookie as the pid. Noticed while typing docs for drm_file and friends.
> > 
> > Since the only consumer of this is the tracepoints I think we can safely
> > change this - those tracepoints should not be uapi relevant at all. It
> > all goes back to
> > 
> > commit b9c2c9ae882f058084e13e339925dbf8d2d20271
> > Author: Jesse Barnes <jbarnes at virtuousgeek.org>
> > Date:   Thu Jul 1 16:48:09 2010 -0700
> > 
> >     drm: add per-event vblank event trace points
> > 
> > which doesn't give a special justification for using pid over a pointer.
> 
> Well, it's friendlier to look at, I suppose.
> 
> > 
> > Also note that the nouveau code setting it is entirely pointless:
> > Since this isn't a vblank event, it will never hit the vblank
> > tracepoints.
> > 
> > Cc: Ben Skeggs <bskeggs at redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> > ---
> >  drivers/gpu/drm/drm_irq.c              |  5 ++---
> >  drivers/gpu/drm/drm_trace.h            | 20 ++++++++++----------
> >  drivers/gpu/drm/nouveau/nouveau_usif.c |  1 -
> >  include/drm/drm_file.h                 |  2 --
> >  4 files changed, 12 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 1906723af389..9bdca69f754c 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -978,7 +978,7 @@ static void send_vblank_event(struct drm_device *dev,
> >  	e->event.tv_sec = now->tv_sec;
> >  	e->event.tv_usec = now->tv_usec;
> >  
> > -	trace_drm_vblank_event_delivered(e->base.pid, e->pipe,
> > +	trace_drm_vblank_event_delivered(e->base.file_priv, e->pipe,
> >  					 e->event.sequence);
> >  
> >  	drm_send_event_locked(dev, &e->base);
> > @@ -1505,7 +1505,6 @@ static int drm_queue_vblank_event(struct drm_device *dev, unsigned int pipe,
> >  	}
> >  
> >  	e->pipe = pipe;
> > -	e->base.pid = current->pid;
> 
> Do you think it would be worthwhile to output the pid:file_priv mapping here in
> case someone is using pid?
> 
> Regardless, the code looks correct, and I don't any skin in this game, so I'll
> add my R-b and let you decide what to do if no one else complains.

I considered it, but then went meh. It's super-easy to change back to
fpriv->pid if anyone wants that, so if someone pipes up I'll volunteer for
that. The one issue with that is that on systems with logind, logind opens
all drm fd for compositors, so they all have the same pid. With fpriv
itself you can at least tell them apart ...

> Reviewed-by: Sean Paul <seanpaul at chromium.org>

I'll take this one :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch