All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Zero initialize this_cpu in busywait_stop
@ 2019-03-08  1:20 Nathan Chancellor
  2019-03-08  2:02 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Nathan Chancellor @ 2019-03-08  1:20 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
  Cc: intel-gfx, dri-devel, linux-kernel, Nick Desaulniers,
	clang-built-linux, Nathan Chancellor

When building with -Wsometimes-uninitialized, Clang warns:

drivers/gpu/drm/i915/i915_request.c:1032:6: warning: variable 'this_cpu'
is used uninitialized whenever '&&' condition is false
[-Wsometimes-uninitialized]

time_after expands to use two typecheck with logical ANDs between them.
typecheck evaluates to 1 but Clang clearly gets confused with the logic
that as semantic analysis happens early in the pipeline. Fix this by
just zero initializing this_cpu as it will always be properly
initialized before the comparison below.

Link: https://github.com/ClangBuiltLinux/linux/issues/415
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

Alternatively, this can be solved by having the return value of
local_clock_us(&this_cpu) be a local variable but this seems less
controversial.

 drivers/gpu/drm/i915/i915_request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index c2a5c48c7541..06c0c952191f 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1027,7 +1027,7 @@ static unsigned long local_clock_us(unsigned int *cpu)
 
 static bool busywait_stop(unsigned long timeout, unsigned int cpu)
 {
-	unsigned int this_cpu;
+	unsigned int this_cpu = 0;
 
 	if (time_after(local_clock_us(&this_cpu), timeout))
 		return true;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Zero initialize this_cpu in busywait_stop
  2019-03-08  1:20 [PATCH] drm/i915: Zero initialize this_cpu in busywait_stop Nathan Chancellor
@ 2019-03-08  2:02 ` Patchwork
  2019-03-08  6:20 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-03-08  2:02 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Zero initialize this_cpu in busywait_stop
URL   : https://patchwork.freedesktop.org/series/57718/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5718 -> Patchwork_12413
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57718/revisions/1/mbox/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-sdma:
    - fi-kbl-7560u:       NOTRUN -> SKIP [fdo#109271] +17

  * igt@amdgpu/amd_cs_nop@sync-fork-compute0:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109315] +17

  * igt@amdgpu/amd_cs_nop@sync-gfx0:
    - fi-cfl-guc:         NOTRUN -> SKIP [fdo#109271] +49

  * igt@gem_exec_basic@gtt-bsd1:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109289] +1

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_contexts:
    - fi-icl-u3:          NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109284] +8

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  
#### Possible fixes ####

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-kbl-7560u:       INCOMPLETE -> PASS

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-byt-j1900:       SKIP [fdo#109271] -> PASS

  * igt@i915_pm_rpm@basic-rte:
    - fi-byt-j1900:       FAIL [fdo#108800] -> PASS

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-ilk-650:         INCOMPLETE [fdo#109723] -> PASS

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-kbl-7567u:       WARN [fdo#109380] -> PASS

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - fi-kbl-7567u:       SKIP [fdo#109271] -> PASS +33

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
    - fi-kbl-7560u:       FAIL [fdo#103375] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109380]: https://bugs.freedesktop.org/show_bug.cgi?id=109380
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#109723]: https://bugs.freedesktop.org/show_bug.cgi?id=109723


Participating hosts (42 -> 38)
------------------------------

  Additional (2): fi-cfl-guc fi-icl-u3 
  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-skl-gvtdvm fi-skl-6770hq fi-byt-squawks fi-bsw-cyan 


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

    * Linux: CI_DRM_5718 -> Patchwork_12413

  CI_DRM_5718: 0d3ac523602514ac4473fca213efd8ffba373844 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4876: 51b8d4cfde8d5b0176180b9683accea91474c7ff @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12413: 4df12bd8f18dd7c63bc1d7098c25a044d8392875 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4df12bd8f18d drm/i915: Zero initialize this_cpu in busywait_stop

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12413/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Zero initialize this_cpu in busywait_stop
  2019-03-08  1:20 [PATCH] drm/i915: Zero initialize this_cpu in busywait_stop Nathan Chancellor
  2019-03-08  2:02 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2019-03-08  6:20 ` Patchwork
  2019-03-08  8:27   ` Chris Wilson
  2019-03-08 20:43 ` ✗ Fi.CI.BAT: failure for drm/i915: Zero initialize this_cpu in busywait_stop (rev2) Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-03-08  6:20 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Zero initialize this_cpu in busywait_stop
URL   : https://patchwork.freedesktop.org/series/57718/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5718_full -> Patchwork_12413_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@preempt-other-chain-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +114

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-apl:          PASS -> FAIL [fdo#109660]

  * igt@kms_atomic_transition@3x-modeset-transitions-nonblocking:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +12

  * igt@kms_busy@basic-modeset-e:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-e:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-f:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]
    - shard-kbl:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-glk:          PASS -> FAIL [fdo#108145]

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
    - shard-hsw:          PASS -> FAIL [fdo#105767]

  * igt@kms_cursor_legacy@pipe-b-forked-bo:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#109507]

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-skl:          PASS -> FAIL [fdo#100368]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-apl:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-glk:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-indfb-fliptrack:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +46

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166] +1

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-none:
    - shard-apl:          PASS -> FAIL [fdo#103166] +2

  * igt@kms_psr@primary_blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +19

  * igt@kms_setmode@basic:
    - shard-kbl:          PASS -> FAIL [fdo#99912]

  * igt@perf_pmu@idle-vcs1:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +13

  
#### Possible fixes ####

  * igt@i915_pm_rpm@universal-planes:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-skl:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_ccs@pipe-a-crc-sprite-planes-basic:
    - shard-apl:          FAIL [fdo#106510] / [fdo#108145] -> PASS +1

  * igt@kms_color@pipe-a-ctm-max:
    - shard-apl:          FAIL [fdo#108147] -> PASS

  * igt@kms_color@pipe-b-ctm-max:
    - shard-skl:          FAIL [fdo#108147] -> PASS

  * igt@kms_color@pipe-b-degamma:
    - shard-apl:          FAIL [fdo#104782] -> PASS

  * igt@kms_color@pipe-c-ctm-green-to-red:
    - shard-skl:          FAIL [fdo#107201] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> PASS +2

  * igt@kms_cursor_crc@cursor-128x42-offscreen:
    - shard-skl:          FAIL [fdo#103232] -> PASS

  * igt@kms_draw_crc@draw-method-rgb565-mmap-cpu-xtiled:
    - shard-skl:          FAIL [fdo#103184] -> PASS

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
    - shard-apl:          FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-glk:          FAIL [fdo#103167] -> PASS +4

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-apl:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane@plane-panning-bottom-right-pipe-b-planes:
    - shard-skl:          FAIL [fdo#103166] -> PASS

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-a-planes:
    - shard-glk:          INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] / [fdo#108145] -> PASS

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
    - shard-snb:          SKIP [fdo#109271] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-y:
    - shard-apl:          FAIL [fdo#103166] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          DMESG-FAIL [fdo#105763] -> PASS

  * igt@kms_setmode@basic:
    - shard-hsw:          FAIL [fdo#99912] -> PASS

  
#### Warnings ####

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-skl:          FAIL [fdo#103191] / [fdo#103232] -> INCOMPLETE [fdo#104108]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [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#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [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#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
  [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#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (6 -> 6)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5718 -> Patchwork_12413

  CI_DRM_5718: 0d3ac523602514ac4473fca213efd8ffba373844 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4876: 51b8d4cfde8d5b0176180b9683accea91474c7ff @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12413: 4df12bd8f18dd7c63bc1d7098c25a044d8392875 @ 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_12413/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Zero initialize this_cpu in busywait_stop
  2019-03-08  1:20 [PATCH] drm/i915: Zero initialize this_cpu in busywait_stop Nathan Chancellor
@ 2019-03-08  8:27   ` Chris Wilson
  2019-03-08  6:20 ` ✓ Fi.CI.IGT: " Patchwork
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2019-03-08  8:27 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Nathan Chancellor, Rodrigo Vivi
  Cc: intel-gfx, Nick Desaulniers, linux-kernel, dri-devel,
	clang-built-linux, Nathan Chancellor

Quoting Nathan Chancellor (2019-03-08 01:20:24)
> When building with -Wsometimes-uninitialized, Clang warns:
> 
> drivers/gpu/drm/i915/i915_request.c:1032:6: warning: variable 'this_cpu'
> is used uninitialized whenever '&&' condition is false
> [-Wsometimes-uninitialized]
> 
> time_after expands to use two typecheck with logical ANDs between them.
> typecheck evaluates to 1 but Clang clearly gets confused with the logic
> that as semantic analysis happens early in the pipeline. Fix this by
> just zero initializing this_cpu as it will always be properly
> initialized before the comparison below.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/415
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> 
> Alternatively, this can be solved by having the return value of
> local_clock_us(&this_cpu) be a local variable but this seems less
> controversial.

I'll just wait for clang to be fixed, as this severely undermines any
respect I have for its semantic analysis.
-Chris

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm/i915: Zero initialize this_cpu in busywait_stop
@ 2019-03-08  8:27   ` Chris Wilson
  0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2019-03-08  8:27 UTC (permalink / raw)
  To: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
  Cc: intel-gfx, Nick Desaulniers, linux-kernel, dri-devel,
	clang-built-linux, Nathan Chancellor

Quoting Nathan Chancellor (2019-03-08 01:20:24)
> When building with -Wsometimes-uninitialized, Clang warns:
> 
> drivers/gpu/drm/i915/i915_request.c:1032:6: warning: variable 'this_cpu'
> is used uninitialized whenever '&&' condition is false
> [-Wsometimes-uninitialized]
> 
> time_after expands to use two typecheck with logical ANDs between them.
> typecheck evaluates to 1 but Clang clearly gets confused with the logic
> that as semantic analysis happens early in the pipeline. Fix this by
> just zero initializing this_cpu as it will always be properly
> initialized before the comparison below.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/415
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> 
> Alternatively, this can be solved by having the return value of
> local_clock_us(&this_cpu) be a local variable but this seems less
> controversial.

I'll just wait for clang to be fixed, as this severely undermines any
respect I have for its semantic analysis.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915: Zero initialize this_cpu in busywait_stop
  2019-03-08  8:27   ` Chris Wilson
  (?)
@ 2019-03-08 19:26   ` Nick Desaulniers
  -1 siblings, 0 replies; 7+ messages in thread
From: Nick Desaulniers @ 2019-03-08 19:26 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Jani Nikula, Joonas Lahtinen, Nathan Chancellor, Rodrigo Vivi,
	intel-gfx, LKML, dri-devel, clang-built-linux, Greg KH,
	Sandeep Patil

On Fri, Mar 8, 2019 at 12:27 AM Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> Quoting Nathan Chancellor (2019-03-08 01:20:24)
> > When building with -Wsometimes-uninitialized, Clang warns:
> >
> > drivers/gpu/drm/i915/i915_request.c:1032:6: warning: variable 'this_cpu'
> > is used uninitialized whenever '&&' condition is false
> > [-Wsometimes-uninitialized]
> >
> > time_after expands to use two typecheck with logical ANDs between them.
> > typecheck evaluates to 1 but Clang clearly gets confused with the logic
> > that as semantic analysis happens early in the pipeline. Fix this by
> > just zero initializing this_cpu as it will always be properly
> > initialized before the comparison below.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/415
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> > ---
> >
> > Alternatively, this can be solved by having the return value of
> > local_clock_us(&this_cpu) be a local variable but this seems less
> > controversial.
>
> I'll just wait for clang to be fixed, as this severely undermines any
> respect I have for its semantic analysis.
> -Chris

I'm still playing around with this in Godbolt (my hunch is that GNU C
statement expressions are maybe inlined as part of GCC's early
inlining phase).  For example:
https://godbolt.org/z/G54s5z

If you change `typecheck(unsigned long, a)` and `typecheck(unsigned
long, b)` in `time_after()` both to `1` (what `typecheck` would
evaluate to), then the warning goes away.  But a further
simplification shows that GNU C statement expressions are not the
problem:
https://godbolt.org/z/KzCN8m

I need to keep investigating, but there may be more we can do on the
compiler side.

It seems that another workaround that avoid default initialization is
to just create another local for the temporary expression that
provably initialized this_cpu, ie.

diff --git a/drivers/gpu/drm/i915/i915_request.c
b/drivers/gpu/drm/i915/i915_request.c
index c2a5c48c7541..5b90b5c35c8b 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1028,8 +1028,9 @@ static unsigned long local_clock_us(unsigned int *cpu)
 static bool busywait_stop(unsigned long timeout, unsigned int cpu)
 {
        unsigned int this_cpu;
+       unsigned long local_clock = local_clock_us(&this_cpu);

-       if (time_after(local_clock_us(&this_cpu), timeout))
+       if (time_after(local_clock, timeout))
                return true;

        return this_cpu != cpu;

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915: Zero initialize this_cpu in busywait_stop (rev2)
  2019-03-08  1:20 [PATCH] drm/i915: Zero initialize this_cpu in busywait_stop Nathan Chancellor
                   ` (2 preceding siblings ...)
  2019-03-08  8:27   ` Chris Wilson
@ 2019-03-08 20:43 ` Patchwork
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-03-08 20:43 UTC (permalink / raw)
  To: Nick Desaulniers; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Zero initialize this_cpu in busywait_stop (rev2)
URL   : https://patchwork.freedesktop.org/series/57718/
State : failure

== Summary ==

Applying: drm/i915: Zero initialize this_cpu in busywait_stop
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/i915_request.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 drm/i915: Zero initialize this_cpu in busywait_stop
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-03-08 20:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-08  1:20 [PATCH] drm/i915: Zero initialize this_cpu in busywait_stop Nathan Chancellor
2019-03-08  2:02 ` ✓ Fi.CI.BAT: success for " Patchwork
2019-03-08  6:20 ` ✓ Fi.CI.IGT: " Patchwork
2019-03-08  8:27 ` [Intel-gfx] [PATCH] " Chris Wilson
2019-03-08  8:27   ` Chris Wilson
2019-03-08 19:26   ` [Intel-gfx] " Nick Desaulniers
2019-03-08 20:43 ` ✗ Fi.CI.BAT: failure for drm/i915: Zero initialize this_cpu in busywait_stop (rev2) Patchwork

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.