Re: [Mesa-dev] [PATCH] meson: fix race condition revealed by using 0.44

2018-04-27 Thread Emil Velikov
On 26 April 2018 at 18:57, Dylan Baker  wrote:
> Quoting Emil Velikov (2018-04-26 10:35:50)
>> On 26 April 2018 at 18:24, Dylan Baker  wrote:
>> > It turns out that the blocking target we had was hiding some race
>> > conditions in the anv driver with anv_extensions.h generation, we should
>> > fix those.
>> >
>> anv_entrypoints[0] is the first output of the anv_entrypoints custom_target.
>> Aka it's effectively the same as anv_extensions_h, isn't it?
>
> Actually, there is anv_entrypoints.[ch], and anv_extensions.[ch], and it's the
> latter that we're missing as an explicit dependency (I got confused about that
> too). The thing that happened here is that the blocking target created a 
> proper
> dependency chain, but when we removed it that was broken, and we got a race.
>
Thanks for the explanation - not sure how I read anv_entrypoints as
anv_extensions :-\

Since it seems commit to confuse the two, it would be nice to add your
description (or anything like it) to the commit message.
With that the patch is
Reviewed-by: Emil Velikov 

-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] meson: fix race condition revealed by using 0.44

2018-04-26 Thread Scott D Phillips
Dylan Baker  writes:

> It turns out that the blocking target we had was hiding some race
> conditions in the anv driver with anv_extensions.h generation, we should
> fix those.
>
> CC: Scott D Phillips 
> CC: Mark Janes 
> Fixes: 92550d9b16d2b295bdac087f31b1fd6d0f808e02
>("meson: remove workaround for custom target creating .h and .c files")
> Signed-off-by: Dylan Baker 

Reviewed-by: Scott D Phillips 

> ---
>  src/intel/vulkan/meson.build | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/intel/vulkan/meson.build b/src/intel/vulkan/meson.build
> index 06acc78391f..f20a8a54c9b 100644
> --- a/src/intel/vulkan/meson.build
> +++ b/src/intel/vulkan/meson.build
> @@ -1,4 +1,4 @@
> -# Copyright © 2017 Intel Corporation
> +# Copyright © 2017-2018 Intel Corporation
>  
>  # Permission is hereby granted, free of charge, to any person obtaining a 
> copy
>  # of this software and associated documentation files (the "Software"), to 
> deal
> @@ -96,7 +96,7 @@ foreach g : [['70', ['gen7_cmd_buffer.c']], ['75', 
> ['gen7_cmd_buffer.c']],
>_gen = g[0]
>libanv_gen_libs += static_library(
>  'libanv_gen@0@'.format(_gen),
> -[anv_gen_files, g[1], anv_entrypoints[0]],
> +[anv_gen_files, g[1], anv_entrypoints[0], anv_extensions_h],
>  include_directories : [
>inc_common, inc_compiler, inc_drm_uapi, inc_intel, inc_vulkan_util,
>inc_vulkan_wsi,
> @@ -220,7 +220,7 @@ if with_tests
>'anv_@0@'.format(t),
>executable(
>  t,
> -['tests/@0@.c'.format(t), anv_entrypoints[0]],
> +['tests/@0@.c'.format(t), anv_entrypoints[0], anv_extensions_h],
>  link_with : libvulkan_intel_test,
>  dependencies : [dep_libdrm, dep_thread, dep_m, dep_valgrind],
>  include_directories : [
> -- 
> 2.17.0
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] meson: fix race condition revealed by using 0.44

2018-04-26 Thread Dylan Baker
Quoting Emil Velikov (2018-04-26 10:35:50)
> On 26 April 2018 at 18:24, Dylan Baker  wrote:
> > It turns out that the blocking target we had was hiding some race
> > conditions in the anv driver with anv_extensions.h generation, we should
> > fix those.
> >
> anv_entrypoints[0] is the first output of the anv_entrypoints custom_target.
> Aka it's effectively the same as anv_extensions_h, isn't it?

Actually, there is anv_entrypoints.[ch], and anv_extensions.[ch], and it's the
latter that we're missing as an explicit dependency (I got confused about that
too). The thing that happened here is that the blocking target created a proper
dependency chain, but when we removed it that was broken, and we got a race.

Dylan


signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] meson: fix race condition revealed by using 0.44

2018-04-26 Thread Emil Velikov
On 26 April 2018 at 18:24, Dylan Baker  wrote:
> It turns out that the blocking target we had was hiding some race
> conditions in the anv driver with anv_extensions.h generation, we should
> fix those.
>
anv_entrypoints[0] is the first output of the anv_entrypoints custom_target.
Aka it's effectively the same as anv_extensions_h, isn't it?

Please add a brief description of the race. Can we have a meson bug
number inline?

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] meson: fix race condition revealed by using 0.44

2018-04-26 Thread Dylan Baker
It turns out that the blocking target we had was hiding some race
conditions in the anv driver with anv_extensions.h generation, we should
fix those.

CC: Scott D Phillips 
CC: Mark Janes 
Fixes: 92550d9b16d2b295bdac087f31b1fd6d0f808e02
   ("meson: remove workaround for custom target creating .h and .c files")
Signed-off-by: Dylan Baker 
---
 src/intel/vulkan/meson.build | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/intel/vulkan/meson.build b/src/intel/vulkan/meson.build
index 06acc78391f..f20a8a54c9b 100644
--- a/src/intel/vulkan/meson.build
+++ b/src/intel/vulkan/meson.build
@@ -1,4 +1,4 @@
-# Copyright © 2017 Intel Corporation
+# Copyright © 2017-2018 Intel Corporation
 
 # Permission is hereby granted, free of charge, to any person obtaining a copy
 # of this software and associated documentation files (the "Software"), to deal
@@ -96,7 +96,7 @@ foreach g : [['70', ['gen7_cmd_buffer.c']], ['75', 
['gen7_cmd_buffer.c']],
   _gen = g[0]
   libanv_gen_libs += static_library(
 'libanv_gen@0@'.format(_gen),
-[anv_gen_files, g[1], anv_entrypoints[0]],
+[anv_gen_files, g[1], anv_entrypoints[0], anv_extensions_h],
 include_directories : [
   inc_common, inc_compiler, inc_drm_uapi, inc_intel, inc_vulkan_util,
   inc_vulkan_wsi,
@@ -220,7 +220,7 @@ if with_tests
   'anv_@0@'.format(t),
   executable(
 t,
-['tests/@0@.c'.format(t), anv_entrypoints[0]],
+['tests/@0@.c'.format(t), anv_entrypoints[0], anv_extensions_h],
 link_with : libvulkan_intel_test,
 dependencies : [dep_libdrm, dep_thread, dep_m, dep_valgrind],
 include_directories : [
-- 
2.17.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev