[PATCH 0/2] annotate i915_gem_object_trylock() as __must_check

[PATCH 0/2] annotate i915_gem_object_trylock() as __must_check

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

A while ago I did an attempt to convert some *_trylock*() functions to 
__must_check annotation. I have refreshed that and the only new compile error 
I found was in i915/gt/selftest_migrate.c.

Here is what I have come up with as a solution for this. I have not observed 
any actual error about this, so you may prefer an entirely different way.

Regards,

Eike
-- 
Rolf Eike Beer

emlix GmbH
Headquarters: Berliner Str. 12, 37073 Göttingen, Germany
Phone +49 (0)551 30664-0, e-mail info@emlix.com
District Court of Göttingen, Registry Number HR B 3160
Managing Directors: Heike Jordan, Dr. Uwe Kracke
VAT ID No. DE 205 198 055
Office Berlin: Panoramastr. 1, 10178 Berlin, Germany
Office Bonn: Bachstr. 6, 53115 Bonn, Germany
http://www.emlix.com

emlix - your embedded Linux partner

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 313 bytes --]

[PATCH 1/2] drm/i915/selftests: check the return value of i915_gem_object_trylock()

[Rolf Eike Beer]

A trylock can fail, in which case operating on the object is unsafe and
unconditionally unlocking is wrong.

Signed-off-by: Rolf Eike Beer <eb@emlix.com>
---
 drivers/gpu/drm/i915/gt/selftest_migrate.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/selftest_migrate.c b/drivers/gpu/drm/i915/gt/selftest_migrate.c
index ca460cee4f8b..b2cb501febe8 100644
--- a/drivers/gpu/drm/i915/gt/selftest_migrate.c
+++ b/drivers/gpu/drm/i915/gt/selftest_migrate.c
@@ -822,7 +822,10 @@ create_init_lmem_internal(struct intel_gt *gt, size_t sz, bool try_lmem)
 			return obj;
 	}
 
-	i915_gem_object_trylock(obj, NULL);
+	if (!i915_gem_object_trylock(obj, NULL)) {
+		i915_gem_object_put(obj);
+		return ERR_PTR(-EBUSY);
+	}
 	err = i915_gem_object_pin_pages(obj);
 	if (err) {
 		i915_gem_object_unlock(obj);
-- 
2.47.1


-- 
Rolf Eike Beer

emlix GmbH
Headquarters: Berliner Str. 12, 37073 Göttingen, Germany
Phone +49 (0)551 30664-0, e-mail info@emlix.com
District Court of Göttingen, Registry Number HR B 3160
Managing Directors: Heike Jordan, Dr. Uwe Kracke
VAT ID No. DE 205 198 055
Office Berlin: Panoramastr. 1, 10178 Berlin, Germany
Office Bonn: Bachstr. 6, 53115 Bonn, Germany
http://www.emlix.com

emlix - your embedded Linux partner

[PATCH 2/2] drm/i915: mark i915_gem_object_trylock() as __must_check

[Rolf Eike Beer]

When you don't look at the return code you can't know if you actually got the
lock.

Signed-off-by: Rolf Eike Beer <eb@emlix.com>
---
 drivers/gpu/drm/i915/gem/i915_gem_object.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index 3dc61cbd2e11..b6da8b561ae0 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -198,8 +198,8 @@ static inline int i915_gem_object_lock_interruptible(struct drm_i915_gem_object
 	return __i915_gem_object_lock(obj, ww, true);
 }
 
-static inline bool i915_gem_object_trylock(struct drm_i915_gem_object *obj,
-					   struct i915_gem_ww_ctx *ww)
+static inline bool __must_check i915_gem_object_trylock(struct drm_i915_gem_object *obj,
+							struct i915_gem_ww_ctx *ww)
 {
 	if (!ww)
 		return dma_resv_trylock(obj->base.resv);
-- 
2.47.1


-- 
Rolf Eike Beer

emlix GmbH
Headquarters: Berliner Str. 12, 37073 Göttingen, Germany
Phone +49 (0)551 30664-0, e-mail info@emlix.com
District Court of Göttingen, Registry Number HR B 3160
Managing Directors: Heike Jordan, Dr. Uwe Kracke
VAT ID No. DE 205 198 055
Office Berlin: Panoramastr. 1, 10178 Berlin, Germany
Office Bonn: Bachstr. 6, 53115 Bonn, Germany
http://www.emlix.com

emlix - your embedded Linux partner

✗ Fi.CI.CHECKPATCH: warning for annotate i915_gem_object_trylock() as __must_check

[Patchwork]

== Series Details ==

Series: annotate i915_gem_object_trylock() as __must_check
URL   : https://patchwork.freedesktop.org/series/142836/
State : warning

== Summary ==

Error: dim checkpatch failed
d9ac0678af4a drm/i915/selftests: check the return value of i915_gem_object_trylock()
72b7193baf3f drm/i915: mark i915_gem_object_trylock() as __must_check
-:6: WARNING:COMMIT_LOG_LONG_LINE: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#6: 
When you don't look at the return code you can't know if you actually got the

total: 0 errors, 1 warnings, 0 checks, 10 lines checked

✗ Fi.CI.SPARSE: warning for annotate i915_gem_object_trylock() as __must_check

[Patchwork]

== Series Details ==

Series: annotate i915_gem_object_trylock() as __must_check
URL   : https://patchwork.freedesktop.org/series/142836/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

✗ i915.CI.BAT: failure for annotate i915_gem_object_trylock() as __must_check

[Patchwork]

== Series Details ==

Series: annotate i915_gem_object_trylock() as __must_check
URL   : https://patchwork.freedesktop.org/series/142836/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_15873 -> Patchwork_142836v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_142836v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_142836v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/index.html

Participating hosts (45 -> 44)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_142836v1:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_busy@basic@modeset:
    - fi-kbl-7567u:       [PASS][1] -> [DMESG-WARN][2] +1 other test dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15873/fi-kbl-7567u/igt@kms_busy@basic@modeset.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/fi-kbl-7567u/igt@kms_busy@basic@modeset.html

  
Known issues
------------

  Here are the changes found in Patchwork_142836v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - fi-pnv-d510:        NOTRUN -> [SKIP][3] +3 other tests skip
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/fi-pnv-d510/igt@debugfs_test@basic-hwmon.html

  * igt@gem_exec_gttfill@basic:
    - fi-pnv-d510:        NOTRUN -> [ABORT][4] ([i915#13169])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/fi-pnv-d510/igt@gem_exec_gttfill@basic.html

  * igt@gem_lmem_swapping@parallel-random-engines@lmem0:
    - bat-dg2-11:         [PASS][5] -> [INCOMPLETE][6] ([i915#12217]) +1 other test incomplete
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15873/bat-dg2-11/igt@gem_lmem_swapping@parallel-random-engines@lmem0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/bat-dg2-11/igt@gem_lmem_swapping@parallel-random-engines@lmem0.html

  * igt@i915_pm_rpm@module-reload:
    - bat-rpls-4:         [PASS][7] -> [FAIL][8] ([i915#12903])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15873/bat-rpls-4/igt@i915_pm_rpm@module-reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/bat-rpls-4/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@gt_mocs:
    - bat-twl-2:          [PASS][9] -> [ABORT][10] ([i915#12919]) +1 other test abort
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15873/bat-twl-2/igt@i915_selftest@live@gt_mocs.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/bat-twl-2/igt@i915_selftest@live@gt_mocs.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         [PASS][11] -> [SKIP][12] ([i915#9197]) +3 other tests skip
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15873/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  
#### Possible fixes ####

  * igt@i915_module_load@load:
    - fi-pnv-d510:        [ABORT][13] ([i915#13203]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15873/fi-pnv-d510/igt@i915_module_load@load.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/fi-pnv-d510/igt@i915_module_load@load.html

  * igt@i915_selftest@live@workarounds:
    - bat-arlh-2:         [ABORT][15] ([i915#12061]) -> [PASS][16] +1 other test pass
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15873/bat-arlh-2/igt@i915_selftest@live@workarounds.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/bat-arlh-2/igt@i915_selftest@live@workarounds.html

  
#### Warnings ####

  * igt@i915_module_load@reload:
    - fi-cfl-8109u:       [DMESG-WARN][17] ([i915#11621]) -> [DMESG-WARN][18] ([i915#11621] / [i915#1982])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_15873/fi-cfl-8109u/igt@i915_module_load@reload.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/fi-cfl-8109u/igt@i915_module_load@reload.html

  
  [i915#11621]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11621
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12217]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12217
  [i915#12903]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12903
  [i915#12919]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12919
  [i915#13169]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13169
  [i915#13203]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13203
  [i915#1982]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/1982
  [i915#9197]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/9197


Build changes
-------------

  * Linux: CI_DRM_15873 -> Patchwork_142836v1

  CI-20190529: 20190529
  CI_DRM_15873: a5b4c40929f3263a92e34e3f6b3c3c0de57e0e58 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8166: 197cca38ae5c494511843112d43351aeab2314be @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_142836v1: a5b4c40929f3263a92e34e3f6b3c3c0de57e0e58 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_142836v1/index.html