diff --git a/CHANGELOG.md b/CHANGELOG.md index 01cdead..24cb615 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add `OVNI_TRACEDIR` envar to change the trace directory (default is `ovni`). +### Changed + +- Don't modify nOS-V subsystem state on task pause. The "Task: Running" + state is now renamed to "Task: In body" to reflect the change. + ## [1.3.0] - 2023-09-07 ### Added diff --git a/doc/user/emulation/events.md b/doc/user/emulation/events.md index 27c8cc8..6258971 100644 --- a/doc/user/emulation/events.md +++ b/doc/user/emulation/events.md @@ -35,8 +35,8 @@ OU] Exits the region of past events (HACK) -------------------- nOS-V (model=V) ---------------------- VTc Creates a new task (punctual event) -VTx Task execute -VTe Task end +VTx Task execute (enter task body) +VTe Task end (exit task body) VTp Task pause VTr Task resume diff --git a/doc/user/emulation/nosv.md b/doc/user/emulation/nosv.md index 7d5b541..a9ebfc4 100644 --- a/doc/user/emulation/nosv.md +++ b/doc/user/emulation/nosv.md @@ -22,3 +22,9 @@ provides two desirable properties: For more details, see [this MR][1]. [1]: https://pm.bsc.es/gitlab/rarias/ovni/-/merge_requests/27 + +## Subsystem view + +The subsystem view provides a simplified view on what is the nOS-V +runtime doing over time. The view follows the same rules described in +the [subsystem view of Nanos6](../nanos6/#subsystem_view). diff --git a/src/emu/nosv/event.c b/src/emu/nosv/event.c index 01c7197..f4fad6d 100644 --- a/src/emu/nosv/event.c +++ b/src/emu/nosv/event.c @@ -120,11 +120,6 @@ chan_task_stopped(struct emu *emu) } } - if (chan_pop(&ch[CH_SUBSYSTEM], value_int64(ST_TASK_RUNNING)) != 0) { - err("chan_pop subsystem failed"); - return -1; - } - return 0; } @@ -167,16 +162,12 @@ chan_task_running(struct emu *emu, struct task *task) return -1; } } - if (chan_push(&ch[CH_SUBSYSTEM], value_int64(ST_TASK_RUNNING)) != 0) { - err("chan_push subsystem failed"); - return -1; - } return 0; } static int -chan_task_switch(struct emu *emu, struct task *prev, struct task *next, char tr) +chan_task_switch(struct emu *emu, struct task *prev, struct task *next) { struct nosv_thread *th = EXT(emu->thread, 'V'); struct chan *ch = th->m.ch; @@ -227,20 +218,6 @@ chan_task_switch(struct emu *emu, struct task *prev, struct task *next, char tr) } } - /* Update the subsystem depending if we are pushing (X) or - * poping (E) a task from the stack */ - if (tr == 'X') { - if (chan_push(&ch[CH_SUBSYSTEM], value_int64(ST_TASK_RUNNING)) != 0) { - err("chan_push subsystem failed"); - return -1; - } - } else if (tr == 'E') { - if (chan_pop(&ch[CH_SUBSYSTEM], value_int64(ST_TASK_RUNNING)) != 0) { - err("chan_pop subsystem failed"); - return -1; - } - } - return 0; } @@ -331,7 +308,7 @@ update_task_channels(struct emu *emu, char tr, struct task *prev, struct task *n /* Additional nested transitions */ case 'X': case 'E': - ret = chan_task_switch(emu, prev, next, tr); + ret = chan_task_switch(emu, prev, next); break; default: err("unexpected transition value %c", tr); @@ -347,22 +324,8 @@ update_task_channels(struct emu *emu, char tr, struct task *prev, struct task *n } static int -enforce_task_rules(struct emu *emu, char tr, struct task_stack *stack, struct task *next) +enforce_task_rules(struct emu *emu, char tr, struct task *next) { - struct nosv_thread *th = EXT(emu->thread, 'V'); - struct value ss; - if (chan_read(&th->m.ch[CH_SUBSYSTEM], &ss) != 0) { - err("chan_read failed"); - return -1; - } - - /* If there is no task in the stack, the subsystem must not be - * running a task */ - if (stack->top == NULL && ss.type == VALUE_INT64 && ss.i == ST_TASK_RUNNING) { - err("task stack empty but subsystem is Task running"); - return -1; - } - if (tr != 'x' && tr != 'X') return 0; @@ -374,7 +337,14 @@ enforce_task_rules(struct emu *emu, char tr, struct task_stack *stack, struct ta return -1; } - if (ss.type == VALUE_INT64 && ss.i != ST_TASK_RUNNING) { + struct nosv_thread *th = EXT(emu->thread, 'V'); + struct value ss; + if (chan_read(&th->m.ch[CH_SUBSYSTEM], &ss) != 0) { + err("chan_read failed"); + return -1; + } + + if (ss.type == VALUE_INT64 && ss.i != ST_TASK_BODY) { err("wrong subsystem state on task begin"); return -1; } @@ -382,6 +352,28 @@ enforce_task_rules(struct emu *emu, char tr, struct task_stack *stack, struct ta return 0; } +static int +update_task_ss_channel(struct emu *emu, char tr) +{ + struct nosv_thread *th = EXT(emu->thread, 'V'); + struct chan *ss = &th->m.ch[CH_SUBSYSTEM]; + + /* Only if starting or ending. Not changed when paused or resumed */ + if (tr == 'x') { + if (chan_push(ss, value_int64(ST_TASK_BODY)) != 0) { + err("chan_push subsystem failed"); + return -1; + } + } else if (tr == 'e') { + if (chan_pop(ss, value_int64(ST_TASK_BODY)) != 0) { + err("chan_pop subsystem failed"); + return -1; + } + } + + return 0; +} + static int update_task(struct emu *emu) { @@ -398,6 +390,12 @@ update_task(struct emu *emu) struct task *next = task_get_running(stack); + /* Update the subsystem channel */ + if (update_task_ss_channel(emu, emu->ev->v) != 0) { + err("update_task_ss_channel failed"); + return -1; + } + char tr; int was_running = (prev != NULL); int runs_now = (next != NULL); @@ -412,7 +410,7 @@ update_task(struct emu *emu) return -1; } - if (enforce_task_rules(emu, tr, stack, next) != 0) { + if (enforce_task_rules(emu, tr, next) != 0) { err("enforce_task_rules failed"); return -1; } diff --git a/src/emu/nosv/nosv_priv.h b/src/emu/nosv/nosv_priv.h index 7d1e7c5..16b7330 100644 --- a/src/emu/nosv/nosv_priv.h +++ b/src/emu/nosv/nosv_priv.h @@ -26,7 +26,7 @@ enum nosv_ss_values { ST_SCHED_SUBMITTING, ST_MEM_ALLOCATING, ST_MEM_FREEING, - ST_TASK_RUNNING, + ST_TASK_BODY, ST_API_CREATE, ST_API_DESTROY, ST_API_SUBMIT, diff --git a/src/emu/nosv/setup.c b/src/emu/nosv/setup.c index bae4fc7..7f0f9a6 100644 --- a/src/emu/nosv/setup.c +++ b/src/emu/nosv/setup.c @@ -83,7 +83,7 @@ static const struct pcf_value_label nosv_ss_values[] = { { ST_SCHED_SUBMITTING, "Scheduler: Submitting" }, { ST_MEM_ALLOCATING, "Memory: Allocating" }, { ST_MEM_FREEING, "Memory: Freeing" }, - { ST_TASK_RUNNING, "Task: Running" }, + { ST_TASK_BODY, "Task: In body" }, { ST_API_CREATE, "API: Create" }, { ST_API_DESTROY, "API: Destroy" }, { ST_API_SUBMIT, "API: Submit" }, diff --git a/test/emu/nosv/CMakeLists.txt b/test/emu/nosv/CMakeLists.txt index e91eedb..29e9b9e 100644 --- a/test/emu/nosv/CMakeLists.txt +++ b/test/emu/nosv/CMakeLists.txt @@ -9,3 +9,4 @@ test_emu(pause.c MP) test_emu(mp-rank.c MP) test_emu(switch-same-type.c) test_emu(multiple-segment.c MP NPROC 4) +test_emu(task-pause-from-submit.c) diff --git a/test/emu/nosv/task-pause-from-submit.c b/test/emu/nosv/task-pause-from-submit.c new file mode 100644 index 0000000..2dd3c44 --- /dev/null +++ b/test/emu/nosv/task-pause-from-submit.c @@ -0,0 +1,68 @@ +/* Copyright (c) 2023 Barcelona Supercomputing Center (BSC) + * SPDX-License-Identifier: GPL-3.0-or-later */ + +#include +#include +#include "common.h" +#include "emu_prv.h" +#include "instr.h" +#include "instr_nosv.h" +#include "emu/nosv/nosv_priv.h" + +int +main(void) +{ + /* Tests the case where a task is paused from nosv_submit(): + * + * A VTp event causes a pop of the subsystem channel, which is expected + * to be ST_TASK_RUNNING. This is not true in the case of a blocking + * submit (NOSV_SUBMIT_BLOCKING), which calls nosv_pause from inside + * nosv_submit, causing the transition to happen from the unexpected + * ST_API_SUBMIT state. + */ + + instr_start(0, 1); + + /* Match the PRV line in the trace */ + FILE *f = fopen("match.sh", "w"); + if (f == NULL) + die("fopen failed:"); + + uint32_t typeid = 100; + instr_nosv_type_create(typeid); + + instr_nosv_task_create(1, typeid); + + instr_nosv_task_execute(1); + + /* When starting a task, it must cause the subsystems to enter + * the "Task: In body" state */ + int prvtype = PRV_NOSV_SUBSYSTEM; + int st = ST_TASK_BODY; + fprintf(f, "grep ':%ld:%d:%d$' ovni/thread.prv\n", get_delta(), prvtype, st); + fprintf(f, "grep ':%ld:%d:%d$' ovni/cpu.prv\n", get_delta(), prvtype, st); + + instr_nosv_submit_enter(); /* Blocking submit */ + instr_nosv_task_pause(1); + + /* Should be left in the submit state, so no state transition in + * subsystems view */ + fprintf(f, "! grep ':%ld:%d:[0-9]*$' ovni/thread.prv\n", get_delta(), prvtype); + fprintf(f, "! grep ':%ld:%d:[0-9]*$' ovni/cpu.prv\n", get_delta(), prvtype); + + /* But the task state must be set to pause, so the task id + * must be null */ + prvtype = PRV_NOSV_TASKID; + fprintf(f, "grep ':%ld:%d:0$' ovni/thread.prv\n", get_delta(), prvtype); + fprintf(f, "grep ':%ld:%d:0$' ovni/cpu.prv\n", get_delta(), prvtype); + + instr_nosv_submit_exit(); + instr_nosv_task_resume(1); + instr_nosv_task_end(1); + + fclose(f); + + instr_end(); + + return 0; +}