Re: [Mesa-dev] [PATCH 1/2] clover: Catch errors from executing event action
On Tue, 2018-07-17 at 15:22 -0700, Francisco Jerez wrote:
> Jan Vesely writes:
>
> > On Tue, 2018-07-17 at 14:17 -0700, Francisco Jerez wrote:
> > > Jan Vesely writes:
> > >
> > > > Abort all dependent events.
> > > > Signed-off-by: Jan Vesely
> > > > ---
> > > > src/gallium/state_trackers/clover/core/event.cpp | 7 ++-
> > > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/gallium/state_trackers/clover/core/event.cpp
> > > > b/src/gallium/state_trackers/clover/core/event.cpp
> > > > index cd5d786604..ed2b6ebdb8 100644
> > > > --- a/src/gallium/state_trackers/clover/core/event.cpp
> > > > +++ b/src/gallium/state_trackers/clover/core/event.cpp
> > > > @@ -51,7 +51,12 @@ event::trigger_self() {
> > > > void
> > > > event::trigger() {
> > > > if (wait_count() == 1)
> > > > - action_ok(*this);
> > > > + try {
> > > > + action_ok(*this);
> > > > + } catch (error ) {
> > > > + for (event : abort_self(e.get()))
> > > > +ev.abort(e.get());
> > > > + }
> > > >
> > >
> > > Any reason not to do it like:
> > >
> > > > try {
> > > > if (wait_count() == 1)
> > > > action_ok(*this);
> > > > } catch (error ) {
> > > > abort(e.get());
> > > > }
> > >
> > > That seems slightly simpler and it should make sure that the abort
> > > action of the failing event is executed as well.
> >
> > sure. It wasn't clear to me if it's ok to execute both action_ok and
> > action_fail for the same event. My understanding was that exactly one
> > of them is called.
> >
> > I can even do:
> > void
> > -event::trigger() {
> > +event::trigger() try {
> > if (wait_count() == 1)
> > action_ok(*this);
> >
> > for (event : trigger_self())
> >ev.trigger();
> > +} catch (error ) {
> > + abort(e.get())
> > }
>
> Yeah, that should work too.
thanks. I pushed both and added cc stable tag.
Jan
>
> >
> >
> > Jan
> >
> > >
> > > Thanks.
> > >
> > > > for (event : trigger_self())
> > > >ev.trigger();
> > > > --
> > > > 2.16.4
> >
> > --
> > Jan Vesely
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Jan Vesely
signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] clover: Catch errors from executing event action
Jan Vesely writes:
> On Tue, 2018-07-17 at 14:17 -0700, Francisco Jerez wrote:
>> Jan Vesely writes:
>>
>> > Abort all dependent events.
>> > Signed-off-by: Jan Vesely
>> > ---
>> > src/gallium/state_trackers/clover/core/event.cpp | 7 ++-
>> > 1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/src/gallium/state_trackers/clover/core/event.cpp
>> > b/src/gallium/state_trackers/clover/core/event.cpp
>> > index cd5d786604..ed2b6ebdb8 100644
>> > --- a/src/gallium/state_trackers/clover/core/event.cpp
>> > +++ b/src/gallium/state_trackers/clover/core/event.cpp
>> > @@ -51,7 +51,12 @@ event::trigger_self() {
>> > void
>> > event::trigger() {
>> > if (wait_count() == 1)
>> > - action_ok(*this);
>> > + try {
>> > + action_ok(*this);
>> > + } catch (error ) {
>> > + for (event : abort_self(e.get()))
>> > +ev.abort(e.get());
>> > + }
>> >
>>
>> Any reason not to do it like:
>>
>> > try {
>> > if (wait_count() == 1)
>> > action_ok(*this);
>> > } catch (error ) {
>> > abort(e.get());
>> > }
>>
>> That seems slightly simpler and it should make sure that the abort
>> action of the failing event is executed as well.
>
> sure. It wasn't clear to me if it's ok to execute both action_ok and
> action_fail for the same event. My understanding was that exactly one
> of them is called.
>
> I can even do:
> void
> -event::trigger() {
> +event::trigger() try {
> if (wait_count() == 1)
> action_ok(*this);
>
> for (event : trigger_self())
>ev.trigger();
> +} catch (error ) {
> + abort(e.get())
> }
Yeah, that should work too.
>
>
> Jan
>
>>
>> Thanks.
>>
>> > for (event : trigger_self())
>> >ev.trigger();
>> > --
>> > 2.16.4
>
> --
> Jan Vesely
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] clover: Catch errors from executing event action
On Tue, 2018-07-17 at 14:17 -0700, Francisco Jerez wrote:
> Jan Vesely writes:
>
> > Abort all dependent events.
> > Signed-off-by: Jan Vesely
> > ---
> > src/gallium/state_trackers/clover/core/event.cpp | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/core/event.cpp
> > b/src/gallium/state_trackers/clover/core/event.cpp
> > index cd5d786604..ed2b6ebdb8 100644
> > --- a/src/gallium/state_trackers/clover/core/event.cpp
> > +++ b/src/gallium/state_trackers/clover/core/event.cpp
> > @@ -51,7 +51,12 @@ event::trigger_self() {
> > void
> > event::trigger() {
> > if (wait_count() == 1)
> > - action_ok(*this);
> > + try {
> > + action_ok(*this);
> > + } catch (error ) {
> > + for (event : abort_self(e.get()))
> > +ev.abort(e.get());
> > + }
> >
>
> Any reason not to do it like:
>
> > try {
> > if (wait_count() == 1)
> > action_ok(*this);
> > } catch (error ) {
> > abort(e.get());
> > }
>
> That seems slightly simpler and it should make sure that the abort
> action of the failing event is executed as well.
sure. It wasn't clear to me if it's ok to execute both action_ok and
action_fail for the same event. My understanding was that exactly one
of them is called.
I can even do:
void
-event::trigger() {
+event::trigger() try {
if (wait_count() == 1)
action_ok(*this);
for (event : trigger_self())
ev.trigger();
+} catch (error ) {
+ abort(e.get())
}
Jan
>
> Thanks.
>
> > for (event : trigger_self())
> >ev.trigger();
> > --
> > 2.16.4
--
Jan Vesely
signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] clover: Catch errors from executing event action
On Tue, 2018-07-17 at 16:15 -0500, Aaron Watry wrote:
> Question: How'd you run across this? CTS, an application?
It's a part of the 3 patch series I posted:
1.) fail to create compute state if code has relocations
2.) throw exception if pipe driver failed to create compute state
3.) catch exception and fail kernel launch event
this fixes clover to be able to run full piglit run on GCN (since
llvm-6)
I haven't looked into what other things it might fix.
Jan
>
> I saw this and had hoped that it finally fixed a CTS hang I've been
> experiencing when running:
> test_conformance/events/test_events test_userevents
>
> But at least when SSH'd into my R600-based box, it still seems to
> pause waiting for an event to trigger:
>
> test_userevents...
> Enqueuing tasks
> Checking task status before setting user event status
> Setting user event status to complete
> Waiting for tasks to finish executing
> Checking task status after setting user event status
> Successful user event case passed.
> Enqueuing tasks
> Checking task status before setting user event status
> Setting user event status to unsuccessful result
> Waiting for tasks to finish executing
> Checking task status after setting user event status
> Unsuccessful user event case passed.
> test_userevents passed
> HUNG HERE
>
> --Aaron
>
> On Tue, Jul 17, 2018 at 9:38 AM, Jan Vesely wrote:
> > Abort all dependent events.
> > Signed-off-by: Jan Vesely
> > ---
> > src/gallium/state_trackers/clover/core/event.cpp | 7 ++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/core/event.cpp
> > b/src/gallium/state_trackers/clover/core/event.cpp
> > index cd5d786604..ed2b6ebdb8 100644
> > --- a/src/gallium/state_trackers/clover/core/event.cpp
> > +++ b/src/gallium/state_trackers/clover/core/event.cpp
> > @@ -51,7 +51,12 @@ event::trigger_self() {
> > void
> > event::trigger() {
> > if (wait_count() == 1)
> > - action_ok(*this);
> > + try {
> > + action_ok(*this);
> > + } catch (error ) {
> > + for (event : abort_self(e.get()))
> > +ev.abort(e.get());
> > + }
> >
> > for (event : trigger_self())
> >ev.trigger();
> > --
> > 2.16.4
> >
> > ___
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
Jan Vesely
signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] clover: Catch errors from executing event action
Jan Vesely writes:
> Abort all dependent events.
> Signed-off-by: Jan Vesely
> ---
> src/gallium/state_trackers/clover/core/event.cpp | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/clover/core/event.cpp
> b/src/gallium/state_trackers/clover/core/event.cpp
> index cd5d786604..ed2b6ebdb8 100644
> --- a/src/gallium/state_trackers/clover/core/event.cpp
> +++ b/src/gallium/state_trackers/clover/core/event.cpp
> @@ -51,7 +51,12 @@ event::trigger_self() {
> void
> event::trigger() {
> if (wait_count() == 1)
> - action_ok(*this);
> + try {
> + action_ok(*this);
> + } catch (error ) {
> + for (event : abort_self(e.get()))
> +ev.abort(e.get());
> + }
>
Any reason not to do it like:
| try {
| if (wait_count() == 1)
| action_ok(*this);
| } catch (error ) {
| abort(e.get());
| }
That seems slightly simpler and it should make sure that the abort
action of the failing event is executed as well.
Thanks.
> for (event : trigger_self())
>ev.trigger();
> --
> 2.16.4
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] clover: Catch errors from executing event action
Question: How'd you run across this? CTS, an application?
I saw this and had hoped that it finally fixed a CTS hang I've been
experiencing when running:
test_conformance/events/test_events test_userevents
But at least when SSH'd into my R600-based box, it still seems to
pause waiting for an event to trigger:
test_userevents...
Enqueuing tasks
Checking task status before setting user event status
Setting user event status to complete
Waiting for tasks to finish executing
Checking task status after setting user event status
Successful user event case passed.
Enqueuing tasks
Checking task status before setting user event status
Setting user event status to unsuccessful result
Waiting for tasks to finish executing
Checking task status after setting user event status
Unsuccessful user event case passed.
test_userevents passed
HUNG HERE
--Aaron
On Tue, Jul 17, 2018 at 9:38 AM, Jan Vesely wrote:
> Abort all dependent events.
> Signed-off-by: Jan Vesely
> ---
> src/gallium/state_trackers/clover/core/event.cpp | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/src/gallium/state_trackers/clover/core/event.cpp
> b/src/gallium/state_trackers/clover/core/event.cpp
> index cd5d786604..ed2b6ebdb8 100644
> --- a/src/gallium/state_trackers/clover/core/event.cpp
> +++ b/src/gallium/state_trackers/clover/core/event.cpp
> @@ -51,7 +51,12 @@ event::trigger_self() {
> void
> event::trigger() {
> if (wait_count() == 1)
> - action_ok(*this);
> + try {
> + action_ok(*this);
> + } catch (error ) {
> + for (event : abort_self(e.get()))
> +ev.abort(e.get());
> + }
>
> for (event : trigger_self())
>ev.trigger();
> --
> 2.16.4
>
> ___
> 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
[Mesa-dev] [PATCH 1/2] clover: Catch errors from executing event action
Abort all dependent events.
Signed-off-by: Jan Vesely
---
src/gallium/state_trackers/clover/core/event.cpp | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/gallium/state_trackers/clover/core/event.cpp
b/src/gallium/state_trackers/clover/core/event.cpp
index cd5d786604..ed2b6ebdb8 100644
--- a/src/gallium/state_trackers/clover/core/event.cpp
+++ b/src/gallium/state_trackers/clover/core/event.cpp
@@ -51,7 +51,12 @@ event::trigger_self() {
void
event::trigger() {
if (wait_count() == 1)
- action_ok(*this);
+ try {
+ action_ok(*this);
+ } catch (error ) {
+ for (event : abort_self(e.get()))
+ev.abort(e.get());
+ }
for (event : trigger_self())
ev.trigger();
--
2.16.4
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
