Hi Tobias,
On 08.07.2012 18:58, Tobias Börtitz wrote:
> - Add the implementation of a manager thread for incoming ipc calls (see
> Ipc_manager_thread). The idea is to have one object per tasks which can
> be asked from every thread whether there is an incoming call for this
> special thread. The manager thread itself is started when the first
> thread is looking for an incoming call (so tasks not using any incoming
> ipc won't have this specific thread). The new thread is only for looking
> up the answerbox for incoming calls and storing them in the private call
> queue of the Ipc_manager_thread class. When a worker thread is asking
> the Ipc_manager_thread for incoming calls, it looks up it's call queue
> for calls specified for the asking thread. If there is more than one
> call addressed to one and the same thread pending in the call queue,
> those calls will be delivered to the thread in the same order as the
> dropped into the answerbox (thread specific FiFo). Asking for calls and
> the delivery of calls are thereby executed in the context of the asking
> thread (I must admit the name "Ipc_manager_thread" is a bit misleading
> since this object is not only used by the manager thread but also by the
> worker threads. I will chance this with the next commit).
I was wondering, why don't you use one queue by thread, and the
Ipc_manager_thread sorts incoming message into them. Thereby you reduce
synchronization overhead between threads, because you only have to
synchronize with the Ipc_manager_thread itself?
> Sadly this
> implementation is not working yet. Looking up any private attribute in
> the Ipc_call_queue from the actual manager thread (the one thread
> accessing the answerbox and filling all incoming calls into the call
> queue) is currently resulting in a page fault.
I've looked into your code. The problem is that you start a thread with
an object's function as starting point for its execution:
Spartan::thread_create((void*)&Ipc_manager_thread::_wait_for_call, ...
The resulting thread starts to execute the correct function, but all
information about the corresponding object is lost. You can just print
the the object's 'this' pointer at the beginning of the _wait_for_call
function, and you will get zero.
If you want to execute a member function within a new thread you should
give the new thread a pointer to the Ipc_manager_thread object. So the
new thread can execute the _wait_for_call function on the correct
object. As you have always only one Ipc_manager_thread object you might
use the well-tried 'singleton' design pattern here, having a
class-function that delivers the global object, e.g.:
class Ipc_manager_thread {
...
static Ipc_manager_thread *singleton()
{
static Ipc_manager_thread thread;
return &thread;
}
...
};
With that you can use a simple static function as entry-function for
your new thread, that calls the _wait_for_call function directly on the
correct object:
Ipc_manager_thread::singleton()->_wait_for_call()
Alternatively you can propagate the object's pointer to the newly
constructed thread e.g. as an argument.
> There are two possible
> solutions to this: 1) patch gdb with the description [1] and then attach
> it to the running qemu which has been started with the -s argument. Then
> analyse and fix the problem; 2) code around this problem: declare a
> static function in the ipc.cc file, which holds the ipc manager and is
> used from the worker threads to retrieve ipc call from the ipc manager.
> The ipc manager would then run entirely in a separate thread. The
> second approach would avoid the current problem completely and I think
> would be the faster one.
I'm not sure whether I got your last point correctly. It seems to me
there is no big difference to your current semantic. Whether you have a
global object (like the ipc_manager variable in ipc.cc) and the threads
use its member functions, or you provide static functions for the same
purpose makes no odds (at least regarding the performance), or did I got
you wrong?
Regards
Stefan
--
Stefan Kalkowski
Genode Labs
http://www.genode-labs.com/ · http://genode.org/
Genode Labs GmbH · Amtsgericht Dresden · HRB 28424 · Sitz Dresden
Geschäftsführer: Dr.-Ing. Norman Feske, Christian Helmuth
_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
