Message ID | 20181106135527.28354-1-daniel.vetter@ffwll.ch (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | drm/i915: Annotate dma_fence waits | expand |
Hi Daniel, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on v4.20-rc1 next-20181106] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-i915-Annotate-dma_fence-waits/20181106-232827 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: i386-randconfig-x000-201844 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): drivers/gpu//drm/i915/i915_request.c: In function 'i915_request_wait': >> drivers/gpu//drm/i915/i915_request.c:1292:3: error: implicit declaration of function 'dma_fence_wait_acquire'; did you mean 'dma_fence_wait_timeout'? [-Werror=implicit-function-declaration] dma_fence_wait_acquire(); ^~~~~~~~~~~~~~~~~~~~~~ dma_fence_wait_timeout >> drivers/gpu//drm/i915/i915_request.c:1295:3: error: implicit declaration of function 'dma_fence_wait_release'; did you mean 'dma_fence_release'? [-Werror=implicit-function-declaration] dma_fence_wait_release(); ^~~~~~~~~~~~~~~~~~~~~~ dma_fence_release cc1: some warnings being treated as errors vim +1292 drivers/gpu//drm/i915/i915_request.c 1265 1266 /** 1267 * i915_request_wait - wait until execution of request has finished 1268 * @rq: the request to wait upon 1269 * @flags: how to wait 1270 * @timeout: how long to wait in jiffies 1271 * 1272 * i915_request_wait() waits for the request to be completed, for a 1273 * maximum of @timeout jiffies (with MAX_SCHEDULE_TIMEOUT implying an 1274 * unbounded wait). 1275 * 1276 * If the caller holds the struct_mutex, the caller must pass I915_WAIT_LOCKED 1277 * in via the flags, and vice versa if the struct_mutex is not held, the caller 1278 * must not specify that the wait is locked. 1279 * 1280 * Returns the remaining time (in jiffies) if the request completed, which may 1281 * be zero or -ETIME if the request is unfinished after the timeout expires. 1282 * May return -EINTR is called with I915_WAIT_INTERRUPTIBLE and a signal is 1283 * pending before the request completes. 1284 */ 1285 long i915_request_wait(struct i915_request *rq, 1286 unsigned int flags, 1287 long timeout) 1288 { 1289 long ret; 1290 1291 if (!lockdep_is_held(&rq->i915->drm.struct_mutex)) > 1292 dma_fence_wait_acquire(); 1293 ret = __i915_request_wait(rq, flags, timeout); 1294 if (!lockdep_is_held(&rq->i915->drm.struct_mutex)) > 1295 dma_fence_wait_release(); 1296 1297 return ret; 1298 } 1299 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Daniel, Thank you for the patch! Yet something to improve: [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on v4.20-rc1 next-20181106] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/drm-i915-Annotate-dma_fence-waits/20181106-232827 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-randconfig-x016-201844 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/gpu//drm/i915/i915_request.c: In function 'i915_request_wait': >> drivers/gpu//drm/i915/i915_request.c:1291:7: error: implicit declaration of function 'lockdep_is_held'; did you mean 'lockdep_assert_held'? [-Werror=implicit-function-declaration] if (!lockdep_is_held(&rq->i915->drm.struct_mutex)) ^~~~~~~~~~~~~~~ lockdep_assert_held drivers/gpu//drm/i915/i915_request.c:1292:3: error: implicit declaration of function 'dma_fence_wait_acquire'; did you mean 'dma_fence_wait_timeout'? [-Werror=implicit-function-declaration] dma_fence_wait_acquire(); ^~~~~~~~~~~~~~~~~~~~~~ dma_fence_wait_timeout drivers/gpu//drm/i915/i915_request.c:1295:3: error: implicit declaration of function 'dma_fence_wait_release'; did you mean 'dma_fence_release'? [-Werror=implicit-function-declaration] dma_fence_wait_release(); ^~~~~~~~~~~~~~~~~~~~~~ dma_fence_release cc1: some warnings being treated as errors vim +1291 drivers/gpu//drm/i915/i915_request.c 1265 1266 /** 1267 * i915_request_wait - wait until execution of request has finished 1268 * @rq: the request to wait upon 1269 * @flags: how to wait 1270 * @timeout: how long to wait in jiffies 1271 * 1272 * i915_request_wait() waits for the request to be completed, for a 1273 * maximum of @timeout jiffies (with MAX_SCHEDULE_TIMEOUT implying an 1274 * unbounded wait). 1275 * 1276 * If the caller holds the struct_mutex, the caller must pass I915_WAIT_LOCKED 1277 * in via the flags, and vice versa if the struct_mutex is not held, the caller 1278 * must not specify that the wait is locked. 1279 * 1280 * Returns the remaining time (in jiffies) if the request completed, which may 1281 * be zero or -ETIME if the request is unfinished after the timeout expires. 1282 * May return -EINTR is called with I915_WAIT_INTERRUPTIBLE and a signal is 1283 * pending before the request completes. 1284 */ 1285 long i915_request_wait(struct i915_request *rq, 1286 unsigned int flags, 1287 long timeout) 1288 { 1289 long ret; 1290 > 1291 if (!lockdep_is_held(&rq->i915->drm.struct_mutex)) 1292 dma_fence_wait_acquire(); 1293 ret = __i915_request_wait(rq, flags, timeout); 1294 if (!lockdep_is_held(&rq->i915->drm.struct_mutex)) 1295 dma_fence_wait_release(); 1296 1297 return ret; 1298 } 1299 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 71107540581d..0f800b8967a4 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -30,6 +30,10 @@ #include "i915_drv.h" +static long __i915_request_wait(struct i915_request *rq, + unsigned int flags, + long timeout); + static const char *i915_fence_get_driver_name(struct dma_fence *fence) { return "i915"; @@ -66,7 +70,7 @@ static signed long i915_fence_wait(struct dma_fence *fence, bool interruptible, signed long timeout) { - return i915_request_wait(to_request(fence), interruptible, timeout); + return __i915_request_wait(to_request(fence), interruptible, timeout); } static void i915_fence_release(struct dma_fence *fence) @@ -1201,6 +1205,21 @@ static bool __i915_wait_request_check_and_reset(struct i915_request *request) long i915_request_wait(struct i915_request *rq, unsigned int flags, long timeout) +{ + long ret; + + if (!lockdep_is_held(&rq->i915->drm.struct_mutex)) + dma_fence_wait_acquire(); + ret = __i915_request_wait(rq, flags, timeout); + if (!lockdep_is_held(&rq->i915->drm.struct_mutex)) + dma_fence_wait_release(); + + return ret; +} + +static long __i915_request_wait(struct i915_request *rq, + unsigned int flags, + long timeout) { const int state = flags & I915_WAIT_INTERRUPTIBLE ? TASK_INTERRUPTIBLE : TASK_UNINTERRUPTIBLE;
i915_request_wait is simply our i915-optimized version of dma_fence_wait. They both use the exact same code. To help lockdep discovering all the dependencies, annotate it. v2: We do opportunistic retiring of dma-fences while holding struct_mutex. The recursion this causes is intentional, and we do have other paths which (hopefully) do depend upon the struct_mutex. 2nd option to fix these annotations (and probably even better) would be creating a dma_fence_signal_opportunistic, which doesnt have the lockdep annotations. But that also doesn't guarantee that we'll actually manage to signal fences without depending upon struct_mutex somewhere, so might as well go with this trick here. Aside, for non-i915 people: struct_mutex is our bkl. The locking rules are ... complicated. Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: intel-gfx@lists.freedesktop.org Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/i915_request.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-)