On 10/05/2016 12:45 PM, Nadav Har'El wrote:
On Tue, Sep 27, 2016 at 12:00 PM, Nadav Har'El <[email protected]
<mailto:[email protected]>> wrote:
On Mon, Sep 19, 2016 at 2:59 PM, Nadav Har'El <[email protected]
<mailto:[email protected]>> wrote:
This patch prevents on-stack sched::thread construction by
hiding the
constructor, and only allowing a thread to be created through
the function
sched::thread::make(...), which creates the object on the heap.
I had a chat about this with Avi.
He would prefer a different solution, which I previously
considered but thought would be too much work to implement...
The idea is that the sched::thread will contain nothing but a
pointer to the real thread structure. The user can create the
sched::thread object wherever he pleases, but the constructor will
allocate the internal thread structure in the way we want (on the
heap).
Avi, I've been thinking about this.
I've been thinking quite a bit about the "pimpl" style solution
(user-visible thread holds pointer to an "internal" thread structure).
This is not too hard to do conceptually, but in my thinking I reached
the following problems:
1. It will require large amounts of changes to sched.cc/sched.hh
<http://sched.cc/sched.hh>.
2. And there is a be a big question of what to do with the
detached_state substructure. I will be silly to have two levels of
indirection (thread pointing to internal_thread pointing to
detached_state) so it would make sense to conflate two, namely to make
the internal_thread structure the "detached state". However, this
would mean that most of the code (even in sched.cc) will still need to
work on the external "thread" object, not the internal_thread
structure. Maybe that's not a big problem, thought. It just sounds a
bit less efficient.
3. You wanted to avoid changing the user-visible sched::thread API.
However, most code (including, for example, mutexes) work with a
"thread*". But now that a thread will only contain a pointer, why do
we need a pointer to that pointer? Maybe like in pthreads, the
user-visible object ("thread_t") should be officially a copyable and
transfered-by-value handle. This will require tons of changes to OSv
(even more than in my original patch).
Because of all these reasons, I wonder if my patch isn't simpler and
"good enough"? Avi, Timmons, what is your opinion on whether we should
go with my patch (in this thread), or the other two approaches
(Timmons' or Avi's).
It's certainly not worth huge churn, we can go with your patch.
\
We actually already have exactly this sort of indirection in
sched::thread, and it is detached_state, which we needed to hold
parts of the thread that we needed to survive the thread just a
bit, until the RCU quiet period. So basically now we'll move
*all* the thread's state into this detached state. The
sched::thread destructor will rcu_dispose() the real thread
structure rather than delete it immediately, but can call
immediately a new method which cleans up part of the thread (like
its stack) which we are sure we no longer need.
Most of the private "thread" methods will become methods on the
new detached state structure (need a new name for it, maybe
thread_impl) and scheduler code which currently handles thread
pointers - e.g., the scheduler's run queue - will now have
pointers directly to this thread_impl - so the new scheme will not
hurt scheduling performance, even the opposite (currently a few of
the thread's fields need to be indirected through "detached_state"
and this will go away).
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
