From 8c07fa7cf5cadf29083801c5ab6ed00be268bf6a Mon Sep 17 00:00:00 2001 From: mrg Date: Tue, 16 Apr 2019 10:00:04 +0000 Subject: [PATCH] fix various problems i've seen where cv_*wait*() return ERESTART, which is -3 in netbsd, which we have mapped linux ERESTARTSYS to. this has a problem because linux code often returns errors and pointers in the same value, and pointer values between -4095 and -1 are considered as error returns, but -3 ends up as 3 and thus is not considered an error, and mayhem ensues. with this in place my kabylake system seems actually stable, i have not triggered any of my prior issues in almost 4 weeks now. Taylor asked me to write up a description and then wrote most of the text below for me :-) In Linux code, we always work with ERESTARTSYS so the code meaning start over is a positive NetBSD errno safe for PTR_ERR/ERR_PTR. To achieve this: 1. adapt all cv_waits that return to Linux so they map ERESTART to ERESTARTSYS, and 2. adapt all returns to userland so they convert ERESTARTSYS to ERESTART. Leave EINTR and all other error codes alone. --- sys/external/bsd/common/include/linux/err.h | 4 +-- sys/external/bsd/common/include/linux/errno.h | 9 ++++-- .../bsd/drm2/dist/drm/nouveau/nouveau_drm.c | 9 +++--- .../bsd/drm2/dist/drm/nouveau/nouveau_fence.c | 9 ++++-- sys/external/bsd/drm2/drm/drm_cdevsw.c | 24 ++++++++++---- .../bsd/drm2/include/drm/drm_wait_netbsd.h | 31 ++++++++++++++----- sys/external/bsd/drm2/linux/linux_fence.c | 14 ++++++--- sys/external/bsd/drm2/linux/linux_ww_mutex.c | 9 ++++-- 8 files changed, 78 insertions(+), 31 deletions(-) diff --git a/sys/external/bsd/common/include/linux/err.h b/sys/external/bsd/common/include/linux/err.h index fcca2517edff..4ab80bbd0068 100644 --- a/sys/external/bsd/common/include/linux/err.h +++ b/sys/external/bsd/common/include/linux/err.h @@ -1,4 +1,4 @@ -/* $NetBSD: err.h,v 1.2 2018/08/27 07:20:25 riastradh Exp $ */ +/* $NetBSD: err.h,v 1.3 2019/04/16 10:00:04 mrg Exp $ */ /*- * Copyright (c) 2013 The NetBSD Foundation, Inc. @@ -38,7 +38,7 @@ #include #include -#define MAX_ERRNO ELAST +#define MAX_ERRNO 4095 static inline bool IS_ERR_VALUE(uintptr_t n) diff --git a/sys/external/bsd/common/include/linux/errno.h b/sys/external/bsd/common/include/linux/errno.h index b1676ef8a213..d258ae786b81 100644 --- a/sys/external/bsd/common/include/linux/errno.h +++ b/sys/external/bsd/common/include/linux/errno.h @@ -1,4 +1,4 @@ -/* $NetBSD: errno.h,v 1.3 2014/07/16 20:56:24 riastradh Exp $ */ +/* $NetBSD: errno.h,v 1.4 2019/04/16 10:00:04 mrg Exp $ */ /*- * Copyright (c) 2013 The NetBSD Foundation, Inc. @@ -35,7 +35,10 @@ * - Linux consistently passes around negative errno values. NetBSD * consistently passes around positive ones, except the special magic * in-kernel ones (EJUSTRETURN, ERESTART, &c.) which should not be - * exposed to userland. Be careful! + * exposed to userland *or* linux-only code using the negative pointer + * means error return pattern. Be careful! If Using ERESTARTSYS from + * Linux code, be sure it is remapped back to ERESTART before NetBSD + * code sees it. */ #ifndef _LINUX_ERRNO_H_ @@ -43,7 +46,7 @@ #include -#define ERESTARTSYS ERESTART +#define ERESTARTSYS (ELAST+1) /* XXX */ #define ENOTSUPP ENOTSUP /* XXX ??? */ #define EREMOTEIO EIO /* XXX Urk... */ #define ECHRNG ERANGE /* XXX ??? */ diff --git a/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_drm.c b/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_drm.c index 3cfca1c20a00..1d7c39f2d8a0 100644 --- a/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_drm.c +++ b/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_drm.c @@ -1,4 +1,4 @@ -/* $NetBSD: nouveau_drm.c,v 1.16 2018/12/21 07:51:17 maya Exp $ */ +/* $NetBSD: nouveau_drm.c,v 1.17 2019/04/16 10:00:04 mrg Exp $ */ /* * Copyright 2012 Red Hat Inc. @@ -25,7 +25,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: nouveau_drm.c,v 1.16 2018/12/21 07:51:17 maya Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nouveau_drm.c,v 1.17 2019/04/16 10:00:04 mrg Exp $"); #include #include @@ -966,11 +966,12 @@ nouveau_ioctl_override(struct file *fp, unsigned long cmd, void *data) ret = pm_runtime_get_sync(dev->dev); if (ret < 0 && ret != -EACCES) - return ret; + /* XXX errno Linux->NetBSD */ + return -ret; switch (DRM_IOCTL_NR(cmd) - DRM_COMMAND_BASE) { case DRM_NOUVEAU_NVIF: - /* XXX errno NetBSD->Linux */ + /* XXX errno Linux->NetBSD */ ret = -usif_ioctl(file, data, IOCPARM_LEN(cmd)); break; default: diff --git a/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_fence.c b/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_fence.c index ad2d7ca0ee29..9deceae2ff5d 100644 --- a/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_fence.c +++ b/sys/external/bsd/drm2/dist/drm/nouveau/nouveau_fence.c @@ -1,4 +1,4 @@ -/* $NetBSD: nouveau_fence.c,v 1.13 2018/08/28 20:59:21 mrg Exp $ */ +/* $NetBSD: nouveau_fence.c,v 1.14 2019/04/16 10:00:04 mrg Exp $ */ /* * Copyright (C) 2007 Ben Skeggs. @@ -27,7 +27,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: nouveau_fence.c,v 1.13 2018/08/28 20:59:21 mrg Exp $"); +__KERNEL_RCSID(0, "$NetBSD: nouveau_fence.c,v 1.14 2019/04/16 10:00:04 mrg Exp $"); #include @@ -334,8 +334,11 @@ nouveau_fence_wait_legacy(struct fence *f, bool intr, long wait) /* XXX what lock? */ /* XXX errno NetBSD->Linux */ ret = -kpause("nvfencel", intr, 1, NULL); - if (ret) + if (ret) { + if (ret == -ERESTART) + ret = -ERESTARTSYS; return ret; + } t = jiffies; if (t >= timeout) return 0; diff --git a/sys/external/bsd/drm2/drm/drm_cdevsw.c b/sys/external/bsd/drm2/drm/drm_cdevsw.c index c02ff41a87bd..20073f5a5dc8 100644 --- a/sys/external/bsd/drm2/drm/drm_cdevsw.c +++ b/sys/external/bsd/drm2/drm/drm_cdevsw.c @@ -1,4 +1,4 @@ -/* $NetBSD: drm_cdevsw.c,v 1.13 2018/12/21 07:51:18 maya Exp $ */ +/* $NetBSD: drm_cdevsw.c,v 1.14 2019/04/16 10:00:04 mrg Exp $ */ /*- * Copyright (c) 2013 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: drm_cdevsw.c,v 1.13 2018/12/21 07:51:18 maya Exp $"); +__KERNEL_RCSID(0, "$NetBSD: drm_cdevsw.c,v 1.14 2019/04/16 10:00:04 mrg Exp $"); #include #include @@ -184,6 +184,8 @@ fail2: mutex_lock(&drm_global_mutex); (void)drm_lastclose(dev); fail1: drm_minor_release(dminor); fail0: KASSERT(error); + if (error == ERESTARTSYS) + error = ERESTART; return error; } @@ -299,6 +301,8 @@ drm_read(struct file *fp, off_t *off, struct uio *uio, kauth_cred_t cred, } /* Success! */ + if (error == ERESTARTSYS) + error = ERESTART; return error; } @@ -345,11 +349,16 @@ drm_ioctl_shim(struct file *fp, unsigned long cmd, void *data) { struct drm_file *file = fp->f_data; struct drm_driver *driver = file->minor->dev->driver; + int error; if (driver->ioctl_override) - return driver->ioctl_override(fp, cmd, data); + error = driver->ioctl_override(fp, cmd, data); else - return drm_ioctl(fp, cmd, data); + error = drm_ioctl(fp, cmd, data); + if (error == ERESTARTSYS) + error = ERESTART; + + return error; } static int @@ -475,11 +484,14 @@ drm_fop_mmap(struct file *fp, off_t *offp, size_t len, int prot, int *flagsp, int error; KASSERT(fp == file->filp); - error = (*dev->driver->mmap_object)(dev, *offp, len, prot, uobjp, + /* XXX errno Linux->NetBSD */ + error = -(*dev->driver->mmap_object)(dev, *offp, len, prot, uobjp, offp, file->filp); *maxprotp = prot; *advicep = UVM_ADV_RANDOM; - return -error; + if (error == ERESTARTSYS) + error = ERESTART; + return error; } static paddr_t diff --git a/sys/external/bsd/drm2/include/drm/drm_wait_netbsd.h b/sys/external/bsd/drm2/include/drm/drm_wait_netbsd.h index 9d8795353977..576565b7e3ed 100644 --- a/sys/external/bsd/drm2/include/drm/drm_wait_netbsd.h +++ b/sys/external/bsd/drm2/include/drm/drm_wait_netbsd.h @@ -1,4 +1,4 @@ -/* $NetBSD: drm_wait_netbsd.h,v 1.14 2016/05/13 15:25:57 christos Exp $ */ +/* $NetBSD: drm_wait_netbsd.h,v 1.15 2019/04/16 10:00:04 mrg Exp $ */ /*- * Copyright (c) 2013 The NetBSD Foundation, Inc. @@ -114,10 +114,13 @@ DRM_SPIN_WAKEUP_ALL(drm_waitqueue_t *q, spinlock_t *interlock) * Like the legacy DRM_WAIT_ON, DRM_SPIN_WAIT_ON returns * * . -EBUSY if timed out (yes, -EBUSY, not -ETIMEDOUT or -EWOULDBLOCK), - * . -EINTR/-ERESTART if interrupted by a signal, or + * . -EINTR/-ERESTARTSYS if interrupted by a signal, or * . 0 if the condition was true before or just after the timeout. * * Note that cv_timedwait* return -EWOULDBLOCK, not -EBUSY, on timeout. + * + * Note that ERESTARTSYS is actually ELAST+1 and only used in Linux + * code and must be converted for use in NetBSD code (user or kernel.) */ #define DRM_SPIN_WAIT_ON(RET, Q, INTERLOCK, TICKS, CONDITION) do \ @@ -148,6 +151,8 @@ DRM_SPIN_WAKEUP_ALL(drm_waitqueue_t *q, spinlock_t *interlock) else \ _dswo_ticks = 0; \ if (RET) { \ + if ((RET) == -ERESTART) \ + (RET) = -ERESTARTSYS; \ if ((RET) == -EWOULDBLOCK) \ /* Waited only one tick. */ \ continue; \ @@ -165,26 +170,29 @@ DRM_SPIN_WAKEUP_ALL(drm_waitqueue_t *q, spinlock_t *interlock) * * The untimed DRM_*WAIT*_UNTIL macros return * - * . -EINTR/-ERESTART if interrupted by a signal, or + * . -EINTR/-ERESTARTSYS if interrupted by a signal, or * . zero if the condition evaluated * * The timed DRM_*TIMED_WAIT*_UNTIL macros return * - * . -EINTR/-ERESTART if interrupted by a signal, + * . -EINTR/-ERESTARTSYS if interrupted by a signal, * . 0 if the condition was false after the timeout, * . 1 if the condition was true just after the timeout, or * . the number of ticks remaining if the condition was true before the * timeout. * - * Contrast DRM_SPIN_WAIT_ON which returns -EINTR/-ERESTART on signal, + * Contrast DRM_SPIN_WAIT_ON which returns -EINTR/-ERESTARTSYS on signal, * -EBUSY on timeout, and zero on success; and cv_*wait*, which return - * -EINTR/-ERESTART on signal, -EWOULDBLOCK on timeout, and zero on + * -EINTR/-ERESTARTSYS on signal, -EWOULDBLOCK on timeout, and zero on * success. * * XXX In retrospect, giving the timed and untimed macros a different * return convention from one another to match Linux may have been a * bad idea. All of this inconsistent timeout return convention logic * has been a consistent source of bugs. + * + * Note that ERESTARTSYS is actually ELAST+1 and only used in Linux + * code and must be converted for use in NetBSD code (user or kernel.) */ #define _DRM_WAIT_UNTIL(RET, WAIT, Q, INTERLOCK, CONDITION) do \ @@ -199,8 +207,11 @@ DRM_SPIN_WAKEUP_ALL(drm_waitqueue_t *q, spinlock_t *interlock) } \ /* XXX errno NetBSD->Linux */ \ (RET) = -WAIT((Q), &(INTERLOCK)->mtx_lock); \ - if (RET) \ + if (RET) { \ + if ((RET) == -ERESTART) \ + (RET) = -ERESTARTSYS; \ break; \ + } \ } \ } while (0) @@ -240,6 +251,8 @@ DRM_SPIN_WAKEUP_ALL(drm_waitqueue_t *q, spinlock_t *interlock) else \ _dtwu_ticks = 0; \ if (RET) { \ + if ((RET) == -ERESTART) \ + (RET) = -ERESTARTSYS; \ if ((RET) == -EWOULDBLOCK) \ (RET) = (CONDITION) ? 1 : 0; \ break; \ @@ -270,6 +283,8 @@ DRM_SPIN_WAKEUP_ALL(drm_waitqueue_t *q, spinlock_t *interlock) while (!(CONDITION)) { \ /* XXX errno NetBSD->Linux */ \ (RET) = -WAIT((Q), &(INTERLOCK)->sl_lock); \ + if ((RET) == -ERESTART) \ + (RET) = -ERESTARTSYS; \ if (RET) \ break; \ } \ @@ -311,6 +326,8 @@ DRM_SPIN_WAKEUP_ALL(drm_waitqueue_t *q, spinlock_t *interlock) else \ _dstwu_ticks = 0; \ if (RET) { \ + if ((RET) == -ERESTART) \ + (RET) = -ERESTARTSYS; \ if ((RET) == -EWOULDBLOCK) \ (RET) = (CONDITION) ? 1 : 0; \ break; \ diff --git a/sys/external/bsd/drm2/linux/linux_fence.c b/sys/external/bsd/drm2/linux/linux_fence.c index e28f2a75e1d1..ee1e6e4b8b60 100644 --- a/sys/external/bsd/drm2/linux/linux_fence.c +++ b/sys/external/bsd/drm2/linux/linux_fence.c @@ -1,4 +1,4 @@ -/* $NetBSD: linux_fence.c,v 1.14 2019/01/05 22:24:24 tnn Exp $ */ +/* $NetBSD: linux_fence.c,v 1.15 2019/04/16 10:00:04 mrg Exp $ */ /*- * Copyright (c) 2018 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: linux_fence.c,v 1.14 2019/01/05 22:24:24 tnn Exp $"); +__KERNEL_RCSID(0, "$NetBSD: linux_fence.c,v 1.15 2019/04/16 10:00:04 mrg Exp $"); #include #include @@ -565,8 +565,11 @@ fence_wait_any_timeout(struct fence **fences, uint32_t nfences, bool intr, } end = hardclock_ticks; __insn_barrier(); - if (ret) + if (ret) { + if (ret == -ERESTART) + ret = -ERESTARTSYS; break; + } timeout -= MIN(timeout, (unsigned)end - (unsigned)start); } mutex_exit(&common.lock); @@ -709,8 +712,11 @@ fence_default_wait(struct fence *fence, bool intr, long timeout) } } /* If the wait failed, give up. */ - if (ret) + if (ret) { + if (ret == -ERESTART) + ret = -ERESTARTSYS; break; + } } out: diff --git a/sys/external/bsd/drm2/linux/linux_ww_mutex.c b/sys/external/bsd/drm2/linux/linux_ww_mutex.c index 8f1e7a921cfa..e8a7dcf26916 100644 --- a/sys/external/bsd/drm2/linux/linux_ww_mutex.c +++ b/sys/external/bsd/drm2/linux/linux_ww_mutex.c @@ -1,4 +1,4 @@ -/* $NetBSD: linux_ww_mutex.c,v 1.5 2018/08/27 15:11:32 riastradh Exp $ */ +/* $NetBSD: linux_ww_mutex.c,v 1.6 2019/04/16 10:00:04 mrg Exp $ */ /*- * Copyright (c) 2014 The NetBSD Foundation, Inc. @@ -30,7 +30,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: linux_ww_mutex.c,v 1.5 2018/08/27 15:11:32 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: linux_ww_mutex.c,v 1.6 2019/04/16 10:00:04 mrg Exp $"); #include #include @@ -41,6 +41,7 @@ __KERNEL_RCSID(0, "$NetBSD: linux_ww_mutex.c,v 1.5 2018/08/27 15:11:32 riastradh #include #include +#include #define WW_WANTLOCK(WW) \ LOCKDEBUG_WANTLOCK((WW)->wwm_debug, (WW), \ @@ -250,6 +251,8 @@ ww_mutex_state_wait_sig(struct ww_mutex *mutex, enum ww_mutex_state state) do { /* XXX errno NetBSD->Linux */ ret = -cv_wait_sig(&mutex->wwm_cv, &mutex->wwm_lock); + if (ret == -ERESTART) + ret = -ERESTARTSYS; if (ret) break; } while (mutex->wwm_state == state); @@ -315,6 +318,8 @@ ww_mutex_lock_wait_sig(struct ww_mutex *mutex, struct ww_acquire_ctx *ctx) do { /* XXX errno NetBSD->Linux */ ret = -cv_wait_sig(&mutex->wwm_cv, &mutex->wwm_lock); + if (ret == -ERESTART) + ret = -ERESTARTSYS; if (ret) goto out; } while (!(((mutex->wwm_state == WW_CTX) ||