libdrm: ERESTART undefined

[Mark Kettenis]

> Date: Sat, 12 Apr 2014 09:21:06 +0200
> From: Thomas Klausner <wiz at NetBSD.org>
> 
> After updating libdrm in pkgsrc, I got a bug report from David Shao
> 
> --- begin quote ---
> 
> For current cvs pkgsrc on DragonFly 3.7-DEVELOPMENT and perhaps other
> BSDs, ERESTART is not defined for userland programs instead being
> defined in errno.h only for
> 
> #if defined(_KERNEL) || defined(_KERNEL_STRUCTURES)
> 
> In any case the DragonFly value of (-1) for ERESTART differs from
> Linux's. As DragonFly does not even implement KMS for VMWare vmx
> graphics its value can basically be anything as long as it is defined.
> Without some definition building libdrm 2.4.53 fails with
> 
> Making all in libkms
>   CC       linux.lo
>   CC       intel.lo
>   CC       dumb.lo
>   CC       api.lo
>   CC       vmwgfx.lo
> vmwgfx.c: In function 'vmwgfx_bo_create':
> vmwgfx.c:107:20: error: 'ERESTART' undeclared (first use in this function)
> vmwgfx.c:107:20: note: each undeclared identifier is reported only once for each function it appears in
> 
> --- end quote ---
>  
> David suggests defining ERESTART to just any value if it's
> not defined, i.e.
> 
> +#ifndef ERESTART
> +#define ERESTART 85
> +#endif
> 
> Should I do that or are there better solutions?

Well, vmgfx.c should almost certainly not be doing that ERESTART
check.  On *BSD and Linux, system calls that are interrupted by a
signal should be restarted automatically by the kernel or return
EINTR.  And libdrm already handles EINTR.

System V derived kernels behave a little bit different.  Here
interrupted system calls are restarted by the system call stubs in
libc.  So here the kernel does expose ERESTART and <errno.h> defines
it such that library code can use it to implement the system call
stubs.  The libkms code should still not see ERESTART, as the ioctl(2)
system call stub will hide it.

Now Linux itself doesn't actually use ERESTART for interrupted system
calls that need to be restarted.  Instead it has various ERESTART*
error codes.  But it does define ERESTART, probably for System V
compatibility.  The define is there in Linux 0.99.15, but not used
anywhere in the kernel code.  Unfortunately through the years some
misguided Linux kernel developers have started using ERESTART for
things that are unrelated to interrupted system calls.  For example
the lguest driver has:

        /* Special case: Guest is 'dead' but wants a reboot. */

But as far as I can tell the vmwgfx driver never actually returns ERESTART.
Hence my claim that the right thing to do is to remove the check:

diff --git a/libkms/vmwgfx.c b/libkms/vmwgfx.c
index d594b3b..1667bd7 100644
--- a/libkms/vmwgfx.c
+++ b/libkms/vmwgfx.c
@@ -100,12 +100,9 @@ vmwgfx_bo_create(struct kms_driver *kms,
 		bo->base.pitch = width * 4;
 		bo->base.kms = kms;
 
-		do {
-			ret = drmCommandWriteRead(bo->base.kms->fd,
-						  DRM_VMW_ALLOC_DMABUF,
-						  &arg, sizeof(arg));
-		} while (ret == -ERESTART);
-
+		ret = drmCommandWriteRead(bo->base.kms->fd,
+					  DRM_VMW_ALLOC_DMABUF,
+					  &arg, sizeof(arg));
 		if (ret)
 			goto err_free;