Add tests for register_on_task_exit_cb()
vvaltchev opened this issue · 16 comments
As you can see from codecov: https://app.codecov.io/gh/vvaltchev/tilck/blob/master/kernel/exit.c, the functions register_on_task_exit_cb() and unregister_on_task_exit_cb() don't have test coverage. It will be good to add a test for them. This can be done via a kernel self-test.
Mind if i take this?
Mind if i take this?
Not at all, be my guest :-)
Hey, just to update that I'm working on this one, life's been rough so it's been a slow process.
Sure, @0xdeadbad, no problem.
@vvaltchev Hello, so I looked a lot at the kernel files, learnt a lot on how things are done and designed, but I'm still kinda stuck trying to write a test for this one. I really want to but I gotta kinda burnout with college and etc. I even noticed the code for sb16.c (audio device) that makes use of this functionality. I tried using kernel threads, but they differ from user threads I guess and don't call the callbacks set for its task. The callbacks are called in the void terminate_process(..) but there's an assertion the assure the current task is not a kernel thread, which makes things harder for me with my current knowledge. Could you give me a hint on this, if possible?
Hello @0xdeadbad, I'm glad that you did spend some time on that. Those callbacks are designed to be used by the kernel, but are called when user tasks exist, so we both need an user process and a way to register such a "test" callback in the kernel. Plus we need a way from userspace to check that such callback has been executed.
First of all, the entry point of such test could be a function in test_misc.c. Don't forget to register that function (shellcmd) in the cmds_table.h file. From there, you could:
- fork() to create a child process
- read showhow (see below) a variable from the kernel
- tell somehow the kernel to register our test callback
- exit
Then, from the parent process, you should:
- wait for the child to exit
- read somehow (see below) the kernel variable and check that its value has increased
Now, one question is: how to communicate with the kernel? In general we have syscalls, but in this case we're talking about a test-specific interface. I've introduced a syscall for that called sys_tilck_cmd(). The idea is that each special test interface calls this syscall with a CMD parameter as 1st argument. At the top of the tilck_cmd.c file, you'll see a list of tilck cmds.
Therefore, it would make sense to register one or more tilck cmds in tilck_cmd.c and then call them from the shellcmd (user space test code). So, what interface should they have? We could make something very specific like:
- register the test callback
- get the value of a specific variable (that is increased by the callback)
But, this would be too specific for the test. Ideally, it would be better to have a more generic interface, such that more test can reuse it. I'd suggest the following:
- TILCK_CMD_CALL_FUNC_0, which calls a specific kernel function of the type
void (*)(void)by name - TILCK_CMD_GET_VAR_LONG, which returns the value of a global kernel variable by name, in a
long.
The kernel func that we'd like to call would be a non-static void function that just calls register_on_task_exit_cb() and passes to it a callback function which just increases a global variable we know.
How to call a kernel function when a name is provided? Use find_addr_of_symbol(). It will search in the symbol table of the kernel (which is loaded in-memory) for the given symbol and return it's address. Write the proper cast et voila'!
For the variable, it's the same thing. Just it has to be a global non-static variable.
Now, final detail: how do you call a TILCK CMD from userspace? Well, check how devshell.h does that:
static inline int
tilck_get_gcov_file_info(int fn,
char *fname,
unsigned fname_size,
unsigned *fsize)
{
return sysenter_call5(TILCK_CMD_SYSCALL,
TILCK_CMD_GCOV_FILE_INFO, // << place your new sub-command here
fn, fname, fname_size, fsize); // << other parameters
}Note that the userspace tests are called shellcmds, because they are actually commands of a single userspace executable called devshell. After you boot Tilck, you can just run devshell and type help to see them all.
This should be a pretty complete overview of how to implement this test. It should be pretty fun and you'll have the chance to see & learn many things.
@vvaltchev i'm not working on this anymore, feel free to assign it to someone else.
@rviu are you going to take this issue? I was trying to do it, but ye.. Life's kinda hard. I would like to ask if we could do some sort of pair programming? I think it would be interesting to share knowledge and contribute.
Hey @0xdeadbad, I'm not working on this issue.
For a similar issue #147 that I worked on, it was helpful for me to look at issue #120 and its PR. You could try taking a look at PRs for both the issues and see if it helps. Also, if you are stuck at something specific, you can reach out to @vvaltchev who I'm sure would be more than happy to help you.
Thanks
Hi, I would like to help if there is no one working on this issue, or join if there is any pair programming event :)
Hi, I would like to help if there is no one working on this issue, or join if there is any pair programming event :)
Oh hello, sorry I've been out for a while. Are you interested in doing this issue together?
Hi, I would like to help if there is no one working on this issue, or join if there is any pair programming event :)
Oh hello, sorry I've been out for a while. Are you interested in doing this issue together?
Yes, I would like to :)
@vvaltchev so, I actually got some time to work on this and I'm getting familiarized with the code, the system itself, etc. I commited some code to my fork's branch, and would you mind giving some feedback? I'm not finished yet, need to make it look better and "reliable", would appreciate it anyway.
Also a question:
- I got confused where to declare the global variable and function, so I created a file just for this (kernel/mock.c). Am I supposed to declare them in a specific file / part of the code?
Link for my commits in any case: 0xdeadbad@885b34e
Hello @0xdeadbad, I'm happy that you are working on this issue.
Your change is very close to what I had in mind, modulo a few details. I added a fair amount of comments about how to make it ready for merge.
I also answered your question: the new funcs should be, along with the new tilck_cmds, in a dedicated "test" module. Sorry, I missed to mention that in my previous long comment.
Let me know if you have any further questions.
Vlad
@vvaltchev i'm not working on this anymore, feel free to assign it to someone else.
Oh lovely. You took on a task and decided to quit? What a waste of time. (Not trying to be rude)