On Thu, 2024-12-26 at 09:17 -0500, Mathieu Desnoyers wrote:
> On 2024-12-26 04:04, Gabriele Monaco wrote:
> >
> > On Tue, 2024-12-24 at 11:20 -0500, Mathieu Desnoyers wrote:
> > > On 2024-12-16 08:09, Gabriele Monaco wrote:
> > > > + if (curr_mm_cid == 0) {
> > > > + printf_verbose(
> > > > + "mm_cids successfully compacted, exiting\n");
> > > > + pthread_exit(NULL);
> > > > + }
> > > > + usleep(RUNNER_PERIOD);
> > > > + }
> > > > + assert(false);
> > >
> > > I suspect we'd want an explicit error message here
> > > with an abort() rather than an assertion which can be
> > > compiled-out with -DNDEBUG.
> > >
> > > > + }
> > > > + printf_verbose("cpu%d has %d and is going to terminate\n",
> > > > + sched_getcpu(), curr_mm_cid);
> > > > + pthread_exit(NULL);
> > > > +}
> > > > +
> > > > +void test_mm_cid_compaction(void)
> > >
> > > This function should return its error to the caller
> > > rather than assert.
> > >
> > > > + if (num_threads == 1) {
> > > > + printf_verbose(
> > > > + "Running on a single cpu, cannot test anything\n");
> > > > + return;
> > >
> > > This should return a value telling the caller that
> > > the test is skipped (not an error per se).
> > >
> >
> > Thanks for the review!
> > I'm not sure how to properly handle these, but it seems to me the
> > cleanest way is to use ksft_* functions to report failures and
> > skipped
> > tests. Other tests in rseq don't use the library but it doesn't
> > seem a
> > big deal if just one test is using it, for now.
>
> For the moment, we could do like the following test which
> does a skip:
>
> void test_membarrier(void)
> {
> fprintf(stderr, "rseq_offset_deref_addv is not implemented
> on this architecture. "
> "Skipping membarrier test.\n");
> }
>
> We can revamp the rest of the tests to use ksft in the future.
>
> Currently everything is driven from run_param_test.sh, and it would
> require significant rework to move to ksft.
>
> >
> > It gets a bit complicated to return values since we are exiting
> > from
> > the main thread (sure we could join the remaining /winning/ thread
> > but
> > we would end up with 2 threads running). The ksft_* functions solve
> > this quite nicely using exit codes, though.
>
> Then thread_running should be marked with the noreturn attribute.
>
> test_mm_cid_compaction can indeed return if it fails in the
> preparation
> stages, just not when calling thread_running.
>
> So we want test_mm_cid_compaction to return errors so main can handle
> them, and we may want to move the call to thread_running directly
> into main after success of test_mm_cid_compaction preparation step.
>
> It's not like we can append any further test after this noreturn
> call.
>
Alright, I'm a bit confused now. I see how in the rseq folder there are
tests in param_test (run by a shell script) and tests on their own c
file that are run just as binary.
For simplicity I added this new test in a separate file and I tried to
mirror what the other tests are doing: all of them are calling one or
more void functions from main (test_*) and some minimal initialisation
(register rseq, which I believe I may not even need, since I'm already
not doing it for all threads).
Now, I can have my test_* function return a value and handle it from
main e.g. aborting if the function returns some value, but that would
require me to define some return values (e.g. abort, fail, perhaps
skip) in use only for this test.
It felt it more consistent to just stick to the void function and
abort/exit directly from there (or return in case of skip).
All other tests do use abort for errors and assert for the pass/fail
condition, but since in my case nothing else can execute after, I'd
say I can simply use exit(0)/exit(1) from the winning thread.
What do you think?
Thanks,
Gabriele
