[PATCH libdrm 1/2] xf86drm: inline drmIoctl()

[Eric Engestrom]

On Thursday, 2017-06-22 10:42:21 +0900, Michel Dänzer wrote:
> On 22/06/17 08:16 AM, Eric Engestrom wrote:
> > As Chad [1] puts it:
> >> it's excessive to jump through the PLT into a shared object for a mere
> >> ioctl wrapper.
> > 
> > [1] https://lists.freedesktop.org/archives/mesa-dev/2017-June/159951.html
> > 
> > Signed-off-by: Eric Engestrom <eric at engestrom.ch>
> 
> NAK, at least in this form:
> 
> 
> > diff --git a/xf86drm.c b/xf86drm.c
> > index 728ac78c..c82a76d2 100644
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -179,20 +179,6 @@ void drmFree(void *pt)
> >      free(pt);
> >  }
> >  
> > -/**
> > - * Call ioctl, restarting if it is interupted
> > - */
> > -int
> > -drmIoctl(int fd, unsigned long request, void *arg)
> > -{
> > -    int ret;
> > -
> > -    do {
> > -        ret = ioctl(fd, request, arg);
> > -    } while (ret == -1 && (errno == EINTR || errno == EAGAIN));
> > -    return ret;
> > -}
> 
> libdrm.so.2 must continue exporting a working drmIoctl symbol, otherwise
> it's an ABI break.

Oops, you're absolutely right, my bad :/

> 
> 
> Also, there are downsides to exposing library API calls as inline
> functions: E.g. if drmIoctl(2) ever needs a change (worst case a
> security bug fix), every user has to be recompiled to get the fix. It's
> kind of like static linking through the back door. Is it really worth it
> for drmIoctl(2)? Are they ever an actual bottleneck?

The start of the conversation [1] was that the profiler was spending
more time going through drmIoctl() than the actual syscall, which
prompted Chris to write a local copy that would be inlined, and the fact
some of that time was spend converting the error returned into a `-1`
and set the error in `errno`, which is rather pointless from a caller's
perspective.

I simply took his proposal and applied it to libdrm, although I obviously
did so badly.

Is the updatability of a loop around ioctl() really an issue?
I can drop the first patch and respin the second as a normal dso function,
would that be something people are interested in?

[1] https://lists.freedesktop.org/archives/mesa-dev/2017-June/159894.html

> 
> 
> -- 
> Earthling Michel Dänzer               |               http://www.amd.com
> Libre software enthusiast             |             Mesa and X developer