Re: [kvm-unit-tests PATCH v6 2/2] s390x: Test specification exceptions during transaction

[Janis Schoetterl-Glausch <scgl@linux.ibm.com>]

On Thu, 2022-09-01 at 16:59 +0200, Nico Boehr wrote:
> Quoting Janis Schoetterl-Glausch (2022-08-26 18:11:12)
> > diff --git a/s390x/spec_ex.c b/s390x/spec_ex.c
> > index 68469e4b..56f26564 100644
> [...]
> > +#define TRANSACTION_COMPLETED 4
> > +#define TRANSACTION_MAX_RETRIES 5
> > +
> > +/*
> > + * NULL must be passed to __builtin_tbegin via constant, forbid diagnose from
> > + * being NULL to keep things simple
> > + */
> 
> For some reason, it took me a while to get this, because the context was not clear to me. Maybe rephrase it a tiny bit:
> 
> If diagnose should be NULL, it must be passed to __builtin_tbegin via constant, so forbid NULL to keep things simple
> 
> [...]
> > +static void test_spec_ex_trans(struct args *args, const struct spec_ex_trigger *trigger)
> > +{
> [...]
> > +       case TRANSACTION_MAX_RETRIES:
> > +               report_skip("Transaction retried %lu times with transient failures, giving up",
> > +                           args->max_retries);
> 
> Hmhm, I am unsure whether a skip is the right thing here. On one hand, it might hide bugs, on the other hand, it might cause spurious failures. Why did you decide for the skip?

Yeah, it's a toss-up. Claudio asked me to change it in v4.