* [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
@ 2019-01-09 16:42 Chris Wilson
2019-01-09 16:42 ` [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker Chris Wilson
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Chris Wilson @ 2019-01-09 16:42 UTC (permalink / raw)
To: intel-gfx
If the current process is being killed (it was interrupted with SIGKILL
or equivalent), it will not make any progress in page allocation and we
can abort performing the shrinking on its behalf. So we can use
mutex_lock_killable() instead (although this path should only be
reachable from kswapd currently).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem_shrinker.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 34b108f73f1d..8ad9519779cc 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -39,18 +39,18 @@ static bool shrinker_lock(struct drm_i915_private *i915,
unsigned int flags,
bool *unlock)
{
- switch (mutex_trylock_recursive(&i915->drm.struct_mutex)) {
+ struct mutex *m = &i915->drm.struct_mutex;
+
+ switch (mutex_trylock_recursive(m)) {
case MUTEX_TRYLOCK_RECURSIVE:
*unlock = false;
return true;
case MUTEX_TRYLOCK_FAILED:
*unlock = false;
- if (flags & I915_SHRINK_ACTIVE) {
- mutex_lock_nested(&i915->drm.struct_mutex,
- I915_MM_SHRINKER);
+ if (flags & I915_SHRINK_ACTIVE &&
+ mutex_lock_killable_nested(m, I915_MM_SHRINKER) == 0)
*unlock = true;
- }
return *unlock;
case MUTEX_TRYLOCK_SUCCESS:
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker
2019-01-09 16:42 [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Chris Wilson
@ 2019-01-09 16:42 ` Chris Wilson
2019-01-10 10:28 ` Tvrtko Ursulin
2019-01-09 16:54 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Patchwork
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2019-01-09 16:42 UTC (permalink / raw)
To: intel-gfx
The wait-for-idle used from within the shrinker_lock_uninterruptible
depends on the struct_mutex locking state being known and declared to
i915_request_wait(). As it is conceivable that we reach the vmap
notifier from underneath struct_mutex (and so keep on relying on the
mutex_trylock_recursive), we should not blindly call i915_request_wait.
In the process we can remove the dubious polling to acquire
struct_mutex, and simply act, or not, on a successful trylock.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_gem_shrinker.c | 35 +++---------------------
1 file changed, 4 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 8ad9519779cc..6cc2b964c955 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -385,31 +385,6 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
return sc->nr_scanned ? freed : SHRINK_STOP;
}
-static bool
-shrinker_lock_uninterruptible(struct drm_i915_private *i915, bool *unlock,
- int timeout_ms)
-{
- unsigned long timeout = jiffies + msecs_to_jiffies_timeout(timeout_ms);
-
- do {
- if (i915_gem_wait_for_idle(i915,
- 0, MAX_SCHEDULE_TIMEOUT) == 0 &&
- shrinker_lock(i915, 0, unlock))
- break;
-
- schedule_timeout_killable(1);
- if (fatal_signal_pending(current))
- return false;
-
- if (time_after(jiffies, timeout)) {
- pr_err("Unable to lock GPU to purge memory.\n");
- return false;
- }
- } while (1);
-
- return true;
-}
-
static int
i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
{
@@ -461,16 +436,14 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
struct i915_vma *vma, *next;
unsigned long freed_pages = 0;
bool unlock;
- int ret;
- if (!shrinker_lock_uninterruptible(i915, &unlock, 5000))
+ if (!shrinker_lock(i915, 0, &unlock))
return NOTIFY_DONE;
/* Force everything onto the inactive lists */
- ret = i915_gem_wait_for_idle(i915,
- I915_WAIT_LOCKED,
- MAX_SCHEDULE_TIMEOUT);
- if (ret)
+ if (i915_gem_wait_for_idle(i915,
+ I915_WAIT_LOCKED,
+ MAX_SCHEDULE_TIMEOUT))
goto out;
intel_runtime_pm_get(i915);
--
2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 15+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
2019-01-09 16:42 [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Chris Wilson
2019-01-09 16:42 ` [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker Chris Wilson
@ 2019-01-09 16:54 ` Patchwork
2019-01-09 16:55 ` ✗ Fi.CI.SPARSE: " Patchwork
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-01-09 16:54 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
URL : https://patchwork.freedesktop.org/series/54952/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
5591d0fa3da6 drm/i915: Use mutex_lock_killable() from inside the shrinker
-:26: ERROR:LOCKING: recursive locking is bad, do not use this ever.
#26: FILE: drivers/gpu/drm/i915/i915_gem_shrinker.c:44:
+ switch (mutex_trylock_recursive(m)) {
total: 1 errors, 0 warnings, 0 checks, 23 lines checked
255c22d39a75 drm/i915: Removing polling for struct_mutex from vmap shrinker
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
2019-01-09 16:42 [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Chris Wilson
2019-01-09 16:42 ` [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker Chris Wilson
2019-01-09 16:54 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Patchwork
@ 2019-01-09 16:55 ` Patchwork
2019-01-09 17:13 ` ✓ Fi.CI.BAT: success " Patchwork
` (2 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-01-09 16:55 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
URL : https://patchwork.freedesktop.org/series/54952/
State : warning
== Summary ==
$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Use mutex_lock_killable() from inside the shrinker
Okay!
Commit: drm/i915: Removing polling for struct_mutex from vmap shrinker
-drivers/gpu/drm/i915/i915_drv.h:3546:16: warning: expression using sizeof(void)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
2019-01-09 16:42 [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Chris Wilson
` (2 preceding siblings ...)
2019-01-09 16:55 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-01-09 17:13 ` Patchwork
2019-01-09 18:46 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-01-10 10:24 ` [PATCH 1/2] " Tvrtko Ursulin
5 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-01-09 17:13 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
URL : https://patchwork.freedesktop.org/series/54952/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_5383 -> Patchwork_11265
====================================================
Summary
-------
**WARNING**
Minor unknown changes coming with Patchwork_11265 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11265, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://patchwork.freedesktop.org/api/1.0/series/54952/revisions/1/
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_11265:
### IGT changes ###
#### Warnings ####
* igt@kms_busy@basic-flip-a:
- fi-kbl-7567u: SKIP -> PASS +7
* igt@pm_rpm@basic-pci-d3-state:
- fi-bsw-kefka: SKIP -> PASS
Known issues
------------
Here are the changes found in Patchwork_11265 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-kbl-7567u: PASS -> DMESG-WARN [fdo#108473]
* igt@kms_frontbuffer_tracking@basic:
- fi-hsw-peppy: PASS -> DMESG-WARN [fdo#102614]
#### Possible fixes ####
* igt@debugfs_test@read_all_entries:
- fi-kbl-7567u: DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS
* igt@gem_exec_suspend@basic-s3:
- fi-kbl-7567u: DMESG-WARN [fdo#103558] / [fdo#105079] / [fdo#105602] -> PASS
* igt@i915_module_load@reload-with-fault-injection:
- fi-kbl-7567u: DMESG-WARN [fdo#105602] / [fdo#108529] -> PASS +1
* igt@kms_chamelium@hdmi-edid-read:
- fi-kbl-7567u: FAIL [fdo#108767] -> PASS +2
* igt@kms_flip@basic-flip-vs-modeset:
- fi-skl-6700hq: DMESG-WARN [fdo#105998] -> PASS +1
* igt@kms_flip@basic-flip-vs-wf_vblank:
- fi-bsw-n3050: FAIL [fdo#100368] -> PASS
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-kbl-7567u: DMESG-FAIL [fdo#105079] -> PASS
* igt@pm_rpm@basic-rte:
- fi-bsw-kefka: FAIL [fdo#108800] -> PASS
* igt@pm_rpm@module-reload:
- fi-kbl-7567u: DMESG-WARN [fdo#108529] -> PASS
[fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
[fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
[fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
[fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
[fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
[fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
[fdo#108473]: https://bugs.freedesktop.org/show_bug.cgi?id=108473
[fdo#108529]: https://bugs.freedesktop.org/show_bug.cgi?id=108529
[fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
[fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
Participating hosts (48 -> 43)
------------------------------
Additional (1): fi-pnv-d510
Missing (6): fi-kbl-soraka fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u3
Build changes
-------------
* Linux: CI_DRM_5383 -> Patchwork_11265
CI_DRM_5383: f0835c765f5520b13d032a1904a2f90a44297b3b @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4757: 738f43a54d626f08e250c926a5aeec53458fbd3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11265: 255c22d39a753d04be387f1d09421d7887b8d296 @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
255c22d39a75 drm/i915: Removing polling for struct_mutex from vmap shrinker
5591d0fa3da6 drm/i915: Use mutex_lock_killable() from inside the shrinker
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11265/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
2019-01-09 16:42 [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Chris Wilson
` (3 preceding siblings ...)
2019-01-09 17:13 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2019-01-09 18:46 ` Patchwork
2019-01-10 10:24 ` [PATCH 1/2] " Tvrtko Ursulin
5 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2019-01-09 18:46 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
URL : https://patchwork.freedesktop.org/series/54952/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_5383_full -> Patchwork_11265_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_11265_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11265_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_11265_full:
### IGT changes ###
#### Possible regressions ####
* igt@perf_pmu@busy-accuracy-98-bcs0:
- shard-iclb: PASS -> INCOMPLETE
#### Warnings ####
* igt@pm_rc6_residency@rc6-accuracy:
- shard-snb: SKIP -> PASS
Known issues
------------
Here are the changes found in Patchwork_11265_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_schedule@pi-ringfull-bsd:
- shard-skl: NOTRUN -> FAIL [fdo#103158]
* igt@gem_userptr_blits@readonly-unsync:
- shard-kbl: PASS -> TIMEOUT [fdo#108887]
* igt@kms_busy@extended-modeset-hang-newfb-render-b:
- shard-snb: PASS -> DMESG-WARN [fdo#107956]
- shard-iclb: NOTRUN -> DMESG-WARN [fdo#107956]
* igt@kms_busy@extended-modeset-hang-newfb-render-c:
- shard-skl: NOTRUN -> DMESG-WARN [fdo#107956]
* igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
- shard-hsw: NOTRUN -> DMESG-WARN [fdo#107956]
* igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
- shard-glk: PASS -> DMESG-WARN [fdo#107956]
* igt@kms_color@pipe-a-gamma:
- shard-skl: PASS -> FAIL [fdo#104782]
* igt@kms_cursor_crc@cursor-128x42-onscreen:
- shard-iclb: NOTRUN -> FAIL [fdo#103232]
* igt@kms_cursor_crc@cursor-256x256-random:
- shard-apl: PASS -> FAIL [fdo#103232] +1
* igt@kms_cursor_crc@cursor-64x64-suspend:
- shard-glk: PASS -> FAIL [fdo#103232]
* igt@kms_draw_crc@draw-method-xrgb2101010-render-ytiled:
- shard-skl: PASS -> FAIL [fdo#103184]
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
- shard-apl: PASS -> FAIL [fdo#103167] +1
* igt@kms_frontbuffer_tracking@fbc-2p-indfb-fliptrack:
- shard-glk: PASS -> FAIL [fdo#103167]
* igt@kms_frontbuffer_tracking@fbc-2p-rte:
- shard-glk: PASS -> FAIL [fdo#103167] / [fdo#105682]
* igt@kms_frontbuffer_tracking@fbc-stridechange:
- shard-skl: NOTRUN -> FAIL [fdo#105683]
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-draw-pwrite:
- shard-iclb: PASS -> FAIL [fdo#103167] +4
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- shard-skl: PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]
* igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
- shard-skl: NOTRUN -> DMESG-WARN [fdo#106885]
* igt@kms_plane@plane-position-covered-pipe-a-planes:
- shard-iclb: NOTRUN -> FAIL [fdo#103166]
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
- shard-skl: NOTRUN -> FAIL [fdo#108145]
* igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
- shard-glk: PASS -> FAIL [fdo#108145]
* igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
- shard-apl: PASS -> FAIL [fdo#103166] +1
* igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
- shard-iclb: PASS -> FAIL [fdo#103166] +2
* igt@kms_plane_scaling@pipe-a-scaler-with-pixel-format:
- shard-iclb: NOTRUN -> DMESG-WARN [fdo#107724]
* igt@kms_plane_scaling@pipe-b-scaler-with-pixel-format:
- shard-iclb: PASS -> DMESG-WARN [fdo#107724]
* igt@kms_psr@no_drrs:
- shard-iclb: PASS -> FAIL [fdo#108341]
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-glk: PASS -> DMESG-FAIL [fdo#105763] / [fdo#106538]
* igt@kms_universal_plane@universal-plane-pipe-b-functional:
- shard-glk: PASS -> FAIL [fdo#103166] +1
* igt@pm_rpm@fences:
- shard-skl: PASS -> INCOMPLETE [fdo#107807] +1
* igt@pm_rpm@gem-execbuf-stress-extra-wait:
- shard-iclb: NOTRUN -> INCOMPLETE [fdo#108840]
* igt@pm_rpm@legacy-planes:
- shard-iclb: NOTRUN -> DMESG-WARN [fdo#108654]
#### Possible fixes ####
* igt@kms_atomic_transition@1x-modeset-transitions-fencing:
- shard-skl: FAIL [fdo#107815] / [fdo#108470] -> PASS
* igt@kms_color@pipe-b-ctm-negative:
- shard-skl: FAIL [fdo#107361] -> PASS
* igt@kms_cursor_crc@cursor-256x256-random:
- shard-glk: FAIL [fdo#103232] -> PASS
* igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
- shard-hsw: FAIL [fdo#105767] -> PASS
* igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-ytiled:
- shard-iclb: WARN [fdo#108336] -> PASS +2
* igt@kms_flip@flip-vs-expired-vblank:
- shard-glk: FAIL [fdo#102887] / [fdo#105363] -> PASS
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
- shard-apl: FAIL [fdo#103167] -> PASS
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
- shard-iclb: FAIL [fdo#103167] -> PASS +1
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-iclb: INCOMPLETE [fdo#107713] -> PASS +1
* igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
- shard-hsw: INCOMPLETE [fdo#103540] -> SKIP
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-mmap-gtt:
- shard-iclb: DMESG-FAIL [fdo#107724] -> PASS +1
* igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
- shard-iclb: FAIL [fdo#105683] / [fdo#108040] -> PASS
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-gtt:
- shard-iclb: DMESG-WARN [fdo#107724] / [fdo#108336] -> PASS +4
* igt@kms_panel_fitting@atomic-fastset:
- shard-iclb: DMESG-WARN [fdo#109263] -> PASS
* igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
- shard-skl: FAIL [fdo#103191] / [fdo#107362] -> PASS
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
- shard-glk: FAIL [fdo#108145] -> PASS
* igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
- shard-apl: FAIL [fdo#108145] -> PASS
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: FAIL [fdo#107815] -> PASS +1
* igt@kms_plane_multiple@atomic-pipe-a-tiling-none:
- shard-apl: FAIL [fdo#103166] -> PASS
* igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
- shard-glk: FAIL [fdo#103166] -> PASS +1
* igt@kms_vblank@pipe-c-wait-forked-busy:
- shard-iclb: DMESG-WARN [fdo#107724] -> PASS +6
* igt@pm_rpm@gem-execbuf-stress:
- shard-skl: INCOMPLETE [fdo#107803] / [fdo#107807] -> PASS
* igt@pm_rpm@reg-read-ioctl:
- shard-skl: INCOMPLETE [fdo#107807] -> PASS
* igt@pm_rpm@system-suspend-execbuf:
- shard-iclb: INCOMPLETE [fdo#107713] / [fdo#108840] -> PASS
#### Warnings ####
* igt@kms_ccs@pipe-a-crc-primary-rotation-180:
- shard-iclb: DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#107725]
* igt@kms_cursor_crc@cursor-128x42-sliding:
- shard-iclb: DMESG-WARN [fdo#107724] / [fdo#108336] -> FAIL [fdo#103232] +1
* igt@kms_rotation_crc@multiplane-rotation-cropping-top:
- shard-kbl: DMESG-WARN [fdo#105604] -> DMESG-FAIL [fdo#108950]
[fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
[fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
[fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
[fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
[fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#105604]: https://bugs.freedesktop.org/show_bug.cgi?id=105604
[fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
[fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
[fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
[fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
[fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
[fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
[fdo#107361]: https://bugs.freedesktop.org/show_bug.cgi?id=107361
[fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
[fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
[fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
[fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
[fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
[fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
[fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
[fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
[fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
[fdo#108470]: https://bugs.freedesktop.org/show_bug.cgi?id=108470
[fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
[fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
[fdo#108887]: https://bugs.freedesktop.org/show_bug.cgi?id=108887
[fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
[fdo#109263]: https://bugs.freedesktop.org/show_bug.cgi?id=109263
Participating hosts (7 -> 7)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_5383 -> Patchwork_11265
CI_DRM_5383: f0835c765f5520b13d032a1904a2f90a44297b3b @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4757: 738f43a54d626f08e250c926a5aeec53458fbd3c @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11265: 255c22d39a753d04be387f1d09421d7887b8d296 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11265/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
2019-01-09 16:42 [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Chris Wilson
` (4 preceding siblings ...)
2019-01-09 18:46 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-01-10 10:24 ` Tvrtko Ursulin
2019-01-10 10:47 ` Chris Wilson
5 siblings, 1 reply; 15+ messages in thread
From: Tvrtko Ursulin @ 2019-01-10 10:24 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 09/01/2019 16:42, Chris Wilson wrote:
> If the current process is being killed (it was interrupted with SIGKILL
> or equivalent), it will not make any progress in page allocation and we
> can abort performing the shrinking on its behalf. So we can use
> mutex_lock_killable() instead (although this path should only be
> reachable from kswapd currently).
kswapd is hopefully not killable so it is not reachable via that route.
But should be via other i915_gem_shrink_all callers. Is it starting to
look like we need to pass some flags to say
normal/interruptible/killable (kswapd/debugfs/?)?
Regards,
Tvrtko
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_shrinker.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 34b108f73f1d..8ad9519779cc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -39,18 +39,18 @@ static bool shrinker_lock(struct drm_i915_private *i915,
> unsigned int flags,
> bool *unlock)
> {
> - switch (mutex_trylock_recursive(&i915->drm.struct_mutex)) {
> + struct mutex *m = &i915->drm.struct_mutex;
> +
> + switch (mutex_trylock_recursive(m)) {
> case MUTEX_TRYLOCK_RECURSIVE:
> *unlock = false;
> return true;
>
> case MUTEX_TRYLOCK_FAILED:
> *unlock = false;
> - if (flags & I915_SHRINK_ACTIVE) {
> - mutex_lock_nested(&i915->drm.struct_mutex,
> - I915_MM_SHRINKER);
> + if (flags & I915_SHRINK_ACTIVE &&
> + mutex_lock_killable_nested(m, I915_MM_SHRINKER) == 0)
> *unlock = true;
> - }
> return *unlock;
>
> case MUTEX_TRYLOCK_SUCCESS:
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker
2019-01-09 16:42 ` [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker Chris Wilson
@ 2019-01-10 10:28 ` Tvrtko Ursulin
2019-01-10 10:39 ` Chris Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Tvrtko Ursulin @ 2019-01-10 10:28 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 09/01/2019 16:42, Chris Wilson wrote:
> The wait-for-idle used from within the shrinker_lock_uninterruptible
> depends on the struct_mutex locking state being known and declared to
> i915_request_wait(). As it is conceivable that we reach the vmap
> notifier from underneath struct_mutex (and so keep on relying on the
> mutex_trylock_recursive), we should not blindly call i915_request_wait.
>
> In the process we can remove the dubious polling to acquire
> struct_mutex, and simply act, or not, on a successful trylock.
Makes sense and removes the special casing. I only miss the historical
reference as to why did vmap notifier need this special handling?
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem_shrinker.c | 35 +++---------------------
> 1 file changed, 4 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 8ad9519779cc..6cc2b964c955 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -385,31 +385,6 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
> return sc->nr_scanned ? freed : SHRINK_STOP;
> }
>
> -static bool
> -shrinker_lock_uninterruptible(struct drm_i915_private *i915, bool *unlock,
> - int timeout_ms)
> -{
> - unsigned long timeout = jiffies + msecs_to_jiffies_timeout(timeout_ms);
> -
> - do {
> - if (i915_gem_wait_for_idle(i915,
> - 0, MAX_SCHEDULE_TIMEOUT) == 0 &&
> - shrinker_lock(i915, 0, unlock))
> - break;
> -
> - schedule_timeout_killable(1);
> - if (fatal_signal_pending(current))
> - return false;
> -
> - if (time_after(jiffies, timeout)) {
> - pr_err("Unable to lock GPU to purge memory.\n");
> - return false;
> - }
> - } while (1);
> -
> - return true;
> -}
> -
> static int
> i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
> {
> @@ -461,16 +436,14 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
> struct i915_vma *vma, *next;
> unsigned long freed_pages = 0;
> bool unlock;
> - int ret;
>
> - if (!shrinker_lock_uninterruptible(i915, &unlock, 5000))
> + if (!shrinker_lock(i915, 0, &unlock))
> return NOTIFY_DONE;
>
> /* Force everything onto the inactive lists */
> - ret = i915_gem_wait_for_idle(i915,
> - I915_WAIT_LOCKED,
> - MAX_SCHEDULE_TIMEOUT);
> - if (ret)
> + if (i915_gem_wait_for_idle(i915,
> + I915_WAIT_LOCKED,
> + MAX_SCHEDULE_TIMEOUT))
> goto out;
>
> intel_runtime_pm_get(i915);
>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker
2019-01-10 10:28 ` Tvrtko Ursulin
@ 2019-01-10 10:39 ` Chris Wilson
2019-01-10 13:16 ` Tvrtko Ursulin
0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2019-01-10 10:39 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-01-10 10:28:14)
>
> On 09/01/2019 16:42, Chris Wilson wrote:
> > The wait-for-idle used from within the shrinker_lock_uninterruptible
> > depends on the struct_mutex locking state being known and declared to
> > i915_request_wait(). As it is conceivable that we reach the vmap
> > notifier from underneath struct_mutex (and so keep on relying on the
> > mutex_trylock_recursive), we should not blindly call i915_request_wait.
> >
> > In the process we can remove the dubious polling to acquire
> > struct_mutex, and simply act, or not, on a successful trylock.
>
> Makes sense and removes the special casing. I only miss the historical
> reference as to why did vmap notifier need this special handling?
Cargo culting. I copied the code I had for oom-notifier, thinking the
heavier the hammer the better.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
2019-01-10 10:24 ` [PATCH 1/2] " Tvrtko Ursulin
@ 2019-01-10 10:47 ` Chris Wilson
2019-01-10 10:54 ` Tvrtko Ursulin
0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2019-01-10 10:47 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-01-10 10:24:09)
>
> On 09/01/2019 16:42, Chris Wilson wrote:
> > If the current process is being killed (it was interrupted with SIGKILL
> > or equivalent), it will not make any progress in page allocation and we
> > can abort performing the shrinking on its behalf. So we can use
> > mutex_lock_killable() instead (although this path should only be
> > reachable from kswapd currently).
>
> kswapd is hopefully not killable so it is not reachable via that route.
> But should be via other i915_gem_shrink_all callers. Is it starting to
> look like we need to pass some flags to say
> normal/interruptible/killable (kswapd/debugfs/?)?
killable is justifiable for all callers, I think, even if SIGKILL may
never be delivered. interruptible? Do we want to conceptually fail a
kmalloc due to a signal, as that's likely to end up with ENOMEM and not
EINTR. (Pretty sure that's not common practice but there's a bit of
shrink-unless-killable around.) So I don't think we need to make
normal aka uninterruptible a special case, and returning before
shrinking on any signal seems unexpected.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
2019-01-10 10:47 ` Chris Wilson
@ 2019-01-10 10:54 ` Tvrtko Ursulin
2019-01-10 11:15 ` Chris Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Tvrtko Ursulin @ 2019-01-10 10:54 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 10/01/2019 10:47, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-10 10:24:09)
>>
>> On 09/01/2019 16:42, Chris Wilson wrote:
>>> If the current process is being killed (it was interrupted with SIGKILL
>>> or equivalent), it will not make any progress in page allocation and we
>>> can abort performing the shrinking on its behalf. So we can use
>>> mutex_lock_killable() instead (although this path should only be
>>> reachable from kswapd currently).
>>
>> kswapd is hopefully not killable so it is not reachable via that route.
>> But should be via other i915_gem_shrink_all callers. Is it starting to
>> look like we need to pass some flags to say
>> normal/interruptible/killable (kswapd/debugfs/?)?
>
> killable is justifiable for all callers, I think, even if SIGKILL may
> never be delivered. interruptible? Do we want to conceptually fail a
As long as using mutex_lock_killable doesn't make something killable
which otherwise wouldn't be. I have to say I don't know how the details
of that work.
> kmalloc due to a signal, as that's likely to end up with ENOMEM and not
> EINTR. (Pretty sure that's not common practice but there's a bit of
> shrink-unless-killable around.) So I don't think we need to make
> normal aka uninterruptible a special case, and returning before
> shrinking on any signal seems unexpected.
debugfs was the only reason I considered interruptible. There I think
makes sense to allow bail up. I hate stuck shell sessions at least so
anything which can be done to avoid them is tempting.
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
2019-01-10 10:54 ` Tvrtko Ursulin
@ 2019-01-10 11:15 ` Chris Wilson
2019-01-10 13:15 ` Tvrtko Ursulin
0 siblings, 1 reply; 15+ messages in thread
From: Chris Wilson @ 2019-01-10 11:15 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-01-10 10:54:33)
>
> On 10/01/2019 10:47, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-01-10 10:24:09)
> >>
> >> On 09/01/2019 16:42, Chris Wilson wrote:
> >>> If the current process is being killed (it was interrupted with SIGKILL
> >>> or equivalent), it will not make any progress in page allocation and we
> >>> can abort performing the shrinking on its behalf. So we can use
> >>> mutex_lock_killable() instead (although this path should only be
> >>> reachable from kswapd currently).
> >>
> >> kswapd is hopefully not killable so it is not reachable via that route.
> >> But should be via other i915_gem_shrink_all callers. Is it starting to
> >> look like we need to pass some flags to say
> >> normal/interruptible/killable (kswapd/debugfs/?)?
> >
> > killable is justifiable for all callers, I think, even if SIGKILL may
> > never be delivered. interruptible? Do we want to conceptually fail a
>
> As long as using mutex_lock_killable doesn't make something killable
> which otherwise wouldn't be. I have to say I don't know how the details
> of that work.
I don't think so... (Famous last words.) My understanding was that
signals, not even SIGKILL, would be delivered to kthreads.
mutex_lock_killable doesn't install a signal handler, it just allows the
scheduler to wake up the process should a high priority signal be
delivered.
> > kmalloc due to a signal, as that's likely to end up with ENOMEM and not
> > EINTR. (Pretty sure that's not common practice but there's a bit of
> > shrink-unless-killable around.) So I don't think we need to make
> > normal aka uninterruptible a special case, and returning before
> > shrinking on any signal seems unexpected.
>
> debugfs was the only reason I considered interruptible. There I think
> makes sense to allow bail up. I hate stuck shell sessions at least so
> anything which can be done to avoid them is tempting.
I see. killable is an improvement for debugfs, not much of one but still
infinitely better ;)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker
2019-01-10 11:15 ` Chris Wilson
@ 2019-01-10 13:15 ` Tvrtko Ursulin
0 siblings, 0 replies; 15+ messages in thread
From: Tvrtko Ursulin @ 2019-01-10 13:15 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 10/01/2019 11:15, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-10 10:54:33)
>>
>> On 10/01/2019 10:47, Chris Wilson wrote:
>>> Quoting Tvrtko Ursulin (2019-01-10 10:24:09)
>>>>
>>>> On 09/01/2019 16:42, Chris Wilson wrote:
>>>>> If the current process is being killed (it was interrupted with SIGKILL
>>>>> or equivalent), it will not make any progress in page allocation and we
>>>>> can abort performing the shrinking on its behalf. So we can use
>>>>> mutex_lock_killable() instead (although this path should only be
>>>>> reachable from kswapd currently).
>>>>
>>>> kswapd is hopefully not killable so it is not reachable via that route.
>>>> But should be via other i915_gem_shrink_all callers. Is it starting to
>>>> look like we need to pass some flags to say
>>>> normal/interruptible/killable (kswapd/debugfs/?)?
>>>
>>> killable is justifiable for all callers, I think, even if SIGKILL may
>>> never be delivered. interruptible? Do we want to conceptually fail a
>>
>> As long as using mutex_lock_killable doesn't make something killable
>> which otherwise wouldn't be. I have to say I don't know how the details
>> of that work.
>
> I don't think so... (Famous last words.) My understanding was that
> signals, not even SIGKILL, would be delivered to kthreads.
> mutex_lock_killable doesn't install a signal handler, it just allows the
> scheduler to wake up the process should a high priority signal be
> delivered.
Okay having snooped around I indeed don't see that it would make kswapd
killable.
>>> kmalloc due to a signal, as that's likely to end up with ENOMEM and not
>>> EINTR. (Pretty sure that's not common practice but there's a bit of
>>> shrink-unless-killable around.) So I don't think we need to make
>>> normal aka uninterruptible a special case, and returning before
>>> shrinking on any signal seems unexpected.
>>
>> debugfs was the only reason I considered interruptible. There I think
>> makes sense to allow bail up. I hate stuck shell sessions at least so
>> anything which can be done to avoid them is tempting.
>
> I see. killable is an improvement for debugfs, not much of one but still
> infinitely better ;)
Truer words haven't been spoken. :) Having said that, debugfs does try
to be interruptible and we could pass a flag, but okay, I can send
another patch you'll dislike later.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker
2019-01-10 10:39 ` Chris Wilson
@ 2019-01-10 13:16 ` Tvrtko Ursulin
2019-01-10 13:48 ` Chris Wilson
0 siblings, 1 reply; 15+ messages in thread
From: Tvrtko Ursulin @ 2019-01-10 13:16 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On 10/01/2019 10:39, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-01-10 10:28:14)
>>
>> On 09/01/2019 16:42, Chris Wilson wrote:
>>> The wait-for-idle used from within the shrinker_lock_uninterruptible
>>> depends on the struct_mutex locking state being known and declared to
>>> i915_request_wait(). As it is conceivable that we reach the vmap
>>> notifier from underneath struct_mutex (and so keep on relying on the
>>> mutex_trylock_recursive), we should not blindly call i915_request_wait.
>>>
>>> In the process we can remove the dubious polling to acquire
>>> struct_mutex, and simply act, or not, on a successful trylock.
>>
>> Makes sense and removes the special casing. I only miss the historical
>> reference as to why did vmap notifier need this special handling?
>
> Cargo culting. I copied the code I had for oom-notifier, thinking the
> heavier the hammer the better.
Go for it.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker
2019-01-10 13:16 ` Tvrtko Ursulin
@ 2019-01-10 13:48 ` Chris Wilson
0 siblings, 0 replies; 15+ messages in thread
From: Chris Wilson @ 2019-01-10 13:48 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx
Quoting Tvrtko Ursulin (2019-01-10 13:16:30)
>
> On 10/01/2019 10:39, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-01-10 10:28:14)
> >>
> >> On 09/01/2019 16:42, Chris Wilson wrote:
> >>> The wait-for-idle used from within the shrinker_lock_uninterruptible
> >>> depends on the struct_mutex locking state being known and declared to
> >>> i915_request_wait(). As it is conceivable that we reach the vmap
> >>> notifier from underneath struct_mutex (and so keep on relying on the
> >>> mutex_trylock_recursive), we should not blindly call i915_request_wait.
> >>>
> >>> In the process we can remove the dubious polling to acquire
> >>> struct_mutex, and simply act, or not, on a successful trylock.
> >>
> >> Makes sense and removes the special casing. I only miss the historical
> >> reference as to why did vmap notifier need this special handling?
> >
> > Cargo culting. I copied the code I had for oom-notifier, thinking the
> > heavier the hammer the better.
>
> Go for it.
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Definitely a long way from perfection! Hopefully not one filled with
later regret.
Ta,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-01-10 13:48 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 16:42 [PATCH 1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Chris Wilson
2019-01-09 16:42 ` [PATCH 2/2] drm/i915: Removing polling for struct_mutex from vmap shrinker Chris Wilson
2019-01-10 10:28 ` Tvrtko Ursulin
2019-01-10 10:39 ` Chris Wilson
2019-01-10 13:16 ` Tvrtko Ursulin
2019-01-10 13:48 ` Chris Wilson
2019-01-09 16:54 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Use mutex_lock_killable() from inside the shrinker Patchwork
2019-01-09 16:55 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-01-09 17:13 ` ✓ Fi.CI.BAT: success " Patchwork
2019-01-09 18:46 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-01-10 10:24 ` [PATCH 1/2] " Tvrtko Ursulin
2019-01-10 10:47 ` Chris Wilson
2019-01-10 10:54 ` Tvrtko Ursulin
2019-01-10 11:15 ` Chris Wilson
2019-01-10 13:15 ` Tvrtko Ursulin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.