lkl: automatic syscall threads creation

[ghost]

Move the host thread creation in lkl_create_syscall_thread so that the
host thread is now properly synchronized with the main thread.

This avoid a potential deadlock where the new host thread will issue a
system call to create a new kernel syscall thread while the main host
has already issued a blocking system call which can only be unblock by
a system call issued from the new host thread, e.g.:

main thread:
write(pipe[1], ...) /* fill pipe /
create new thread
write(pipe[1], ...) / blocked due to full pipe */

new thread:
lkl_create_syscall_thread

/* we potentially dealock here because the main syscall thread is
  executing the write system call from main thread */

read(pipe[0])

lkl_stop_syscall_thread() has been removed while
lkl_create_syscall_thread() has been changed to receive a function and
argument to run in the new host thread. It now must be called from the
main thread.

Signed-off-by: Octavian Purdila octavian.purdila@intel.com

[ghost]

@pscollins @phhusson it implements what we discussed in #92, can you please take a look?

[tommie-lie]

I'm not too fond of the lkl_create_syscall_thread(void (*fn)(void*), void *arg) signatue. It will cause trouble for people that don't use pthread-like APIs directly but use some higher level abstraction like glib, boost::thread or (like I do) C++ std::thread. Calling a function at the beginning and end of a thread function is easy and should be fairly portable across tread abstractions. Creating threads through lkl_create_syscall_thread would force you to mix APIs and idioms or hack the abstraction.

My deadline just disappeared, so I will have time to have a more thorough look at this this weekend.

[pscollins]

@tommie-lie That makes sense.

I suppose the start_syscall_thread() interface might be easiest to use to hijack clone(2), where we could just do it as:

HOST_CALL(clone);
int clone(/* args */)
{
    start_syscall_thread();
    host_clone(/* args */);
}

What I'd like to do is keep the data stuff in a threadlocal variable so the lkl_host_ops->{get,set}_tls calls are no longer needed, and start up a new syscall thread on demand if a host thread tries to make a syscall and discovers that its __thread struct lkl_syscall_thread_data *data is NULL, so the process would be transparent to both hijacked and non-hijacked code.

I think it can be done nicely but I'm not totally certain how to deal with the syscall(__NR_syscall_thread_create) call that needs to be done in the original thread, rather than in the newly-created thread. I suppose we could have one thread that exists only to call syscall(__NR_syscall_thread_create) when asked to by newly-created threads, which I think is similar to what @phhusson suggested earlier up in this thread. There probably wouldn't be too much overhead as long as it slept on a condition variable until needed, or something like that. I'll try to put together an implementation this week sometime.

[tommie-lie]

Pardon my ignorance on the threading model of LKL, I spent this afternoon tracing the calls and finding my way through LKL's interrupts handlers, but there are probably still things I'm missing. So please correct me if I'm wrong in my musings below.

I don't really understand why it is necessary to explicitly create host-side threads to run interrupts in parallel.
As far as I understand the current syscall mechanism (in master, not in this PR), there is one IRQ line per host thread. lkl_syscall is executed directly in the host application's context, i.e. the thread that called the syscall, and puts a syscall struct into a queue, triggers an IRQ and waits for the syscall to complete.
In the meantime, the syscall thread dequeues the syscall and calls run_syscall which blocks as long as the syscall itself is still running (even if it waits for I/O). As there can be many different syscall threads (as of #86), the individual syscall handlers have to be reentrant (which is already a kernel feature).
The IRQ handler itself syscall_irq_handler just notifies the queue of a new item and returns immediately.

The only thing that really blocks for a longer period of time is the actuall call into the syscall table in run_syscall. The function itself can currently be called from different threads (the syscall threads) and blocks.
Why can't we simply spawn a new thread at syscalls.c:107 and pass s and the completion semaphore as arg to the thread_fn? The thread_fn would then consist of

run_syscall(s);
lkl_ops->sem_up(data->completion);

This would cause the while loop in syscalls.c:107 to immediately continue after the thread has been started. The syscall itself runs in a seperate thread. As the caller (on the host-side) has a down() on the completeion semaphore, the host is blocked as long as the syscall has not completed. Other host threads can call lkl_syscall again to add items to the wqh, trigger the interrupt and spawn yet another thread. As these actual syscall threads are bound to the host-side threads, there can be no more syscall threads than there are host threads. As they vanish as soon as they are completed, no resources are wasted. The syscall metadata (current data variable, syscall no. and parameters) is accessed only from lkl_syscall and the syscall demultiplexer (the function currently called syscall_thread) and is immediately passed to the thread. We should pthread_detach/CloseHandle the spawned threads or keep a global list of them to join them on on lkl_halt. We'd have to put a write lock on the wqh (or is it already thread-safe?) to serialize simultaneous calls to lkl_syscalls, but the duration of these mutexes would be very short in terms of execution time.

This would also get rid of the lkl_create_syscall_thread/lkl_stop_syscall_thread functions and some amount of other management infrastructure introduced by #86, i.e. reduce code size and complexity.

Is there a big issue I am not seeing here? (There probably is)

[tommie-lie]

This is imprecise language:

and puts a syscall struct into a queue, triggers an IRQ and waits for the syscall to complete.

There is no real queue data structure that the syscall is added to but a queue of (in our case only one) waiters that are woken up and that then process the syscall which has (almost) the same semantics. This can as easily be converted into a real queue of syscalls.

[ghost]

Hi @tommie-lie ,

Spawning new kernel threads works OK if we want to run all syscalls in the same process. However, if we want to support multiple processes then we do need long lived kernel threads / tasks. The cost of creating and terminating a thread needs to be taken in consideration as well.

I am slowly working on creating a uml implementation based on LKL and I want to keep supporting multiple processing.

Also, I don't think that spawning a new thread for each syscall is really required. I am working on a new PR based on @phhusson 's idea: reserve the default syscall thread to spawn new syscall thread and spawn a new syscall thread at the first syscall executed from a thread - we can use TLS to check that. I hope to have it ready in a couple of days.

[ghost]

Hi @tommie-lie , @pscollins , @phhusson ,

I have pushed a new patch based on @phhusson 's idea.

[phhusson]

Sounds good to me.
I'll check it works with my code

[pscollins]

@tommie-lie

The syscall metadata (current data variable, syscall no. and parameters) is accessed only from lkl_syscall and the syscall demultiplexer (the function currently called syscall_thread) and is immediately passed to the thread.

The data pointer is associated with the IRQ handler, so in order to make it from lkl_syscall in a host thread to run_syscall in a kernel thread, it needs to bounce through a lot of machinery and nothing else can overwrite it until run_syscall lets lkl_syscall finish and lkl_syscall returns (which is why there is one IRQ line per host thread, I think), so it's tricky to set things up in a way where you know that data is no longer being accessed by anyone else --- in your suggested implementation, wouldn't data->s be clobbered by the next host thread that makes a syscall?

[ghost]

@pscollins @phhusson @tommie-lie new version (v3) which fixes the race condition @pscollins found. We now access the threads_list only from kernel context.

I have also removed the waitid calls since from commit v2 we don't create full processes but threads (see the CLONE_THREAD in kernel_thread call) and thus we don't need to wait for children before we exit the main syscall thread.

[pscollins]

@opurdila LGTM. I benchmarked this version with a netperf-like tool and it is very good, within an order of magnitude of host performance over /dev/lo.