Mailing-List: contact harmony-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: harmony-dev@incubator.apache.org
Received-SPF: pass (asf.osuosl.org: domain of weldonwjw@gmail.com designates
 64.233.184.224 as permitted sender)
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=gmail.com;
        h=received:message-id:date:from:to:subject:in-reply-to:mime-version:content-type:references;
        b=DcoxiTPfZxvuFN997OEOFjyjZw8D0Htb52hlLx+PgTfEAdKIVvnyWzRWPH27z1KmOKVOmLYRAdW8ywyGj0W0PvJS/oGMilw7gcUWSPzJWdBB44Av/b3CUGiDkr1hCtMyC/dVgDxNMA6ajBgbJE3Ui4yw1gdBafu5BPdrorl0E4w=
Message-ID: <4dd1f3f00610181055k23a39aaelf393770d440a1e38@mail.gmail.com>
Date: Wed, 18 Oct 2006 10:55:55 -0700
From: "Weldon Washburn" <weldonwjw@gmail.com>
To: harmony-dev@incubator.apache.org
Subject: Re: [drlvm][threading] Is it safe to use hythread_suspend_all and
 hythread_resume_all?
In-Reply-To: <e29119980610180022p225cf9eayf7c29e1605cb078e@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/alternative;
	boundary="----=_Part_65694_7512858.1161194155207"
References: <e29119980610170130p1a050b25q61e22059626a770e@mail.gmail.com>
	 <eh3005$8vt$1@sea.gmane.org>
	 <e29119980610180022p225cf9eayf7c29e1605cb078e@mail.gmail.com>

------=_Part_65694_7512858.1161194155207
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

On 10/18/06, Evgueni Brevnov <evgueni.brevnov@gmail.com> wrote:
>
> On 10/17/06, Salikh Zakirov <Salikh.Zakirov@intel.com> wrote:
> > Evgueni Brevnov wrote:
> > > Hi All,
> > >
> > > I'd like to here you opinion regarding hythread_suspend_all and
> > > hythread_resume_all functionality provided by TM. Actually I have to
> > > separate questions:
> > >
> > > 1)  I found that hythread_suspend_all calls thread_safe_point_impl
> > > inside. There is no assertion regarding thread's state upon entering
> > > hythread_suspend_all. So it can be called in suspend disabled state
> > > and nobody (at least me) expects to have a safe point during
> > > hythread_suspend_all. The problem seems to be very similar with the
> > > one discussed in "[drlvm][threading] Possible race condition in
> > > implementation of conditional variables?" Your thoughts?
> >
> > The code you see is there to prevent following deadlock scenario:
> >
> >   Thread A           Thread B
> >      |                  |
> >      | <------------ suspend(A);
> >      |           A->suspend_request = 1;
> >      |        wait for A to reach a safepoint...
> >      |                  |
> >   suspend_all()         |
> >  B->suspend_request = 1
> > wait for B to reach a safepoint ...
> >
> > and then two threads are infinitely waiting one another.
>
> Salikh, I see your scenario...I don't suggest to remove safe points
> from hythread_suspend_all. Contrary I believe it makes sense to
> suspend other threads only if it suspender thread is in a safe region.
> Agree?


This seems to make the most sense.  I think there might be an argument that
Thread A trying to suspend all other threads really does not have to be in
suspend_enable mode.  But this corner case probably adds nothing but
clutter/confusion to the design.  As a design rule, I believe any time a
thread calls a function that might block, the thread should be in suspend
enable mode.  This simple rule makes it much easier for the whole team
working on the code base to know what to do.

Suppose Thread A intends to suspend a subset of all java threads.  Thread A
will need to block somehow and wait for the complete subset to get to
suspended state with their stacks enumerable (suspend_enabled).  Meanwhile
over on another CPU in the SMP box, a bunch of non-suspended threads chew up
gobs of memory and one of them calls for a stop-the-world GC.  Ultimately
Thread A as well as the subset that was suspended needs to be in a
suspend_enabled state.

The easiest sync model to reason about is one where Thread A suspends its
target subset of java threads *before* allowing the stop-the-world gc to
proceed.  A global thread/gc lock provides this guarantee.

>
> > > 2) Assume I need to suspend all threads in particular group. Ok I pass
> > > that group to hythread_suspend_all. Later when all suspended threads
> > > should be resumed I pass the same group to hythread_resume_all. But
> > > threads were suspended group has changed. For example one new thread
> > > was added to it. So the questions are. Is it acceptable to have such
> > > "unsafe" functionality? Would it better to lock the group in
> > > hythread_suspend_all and unlock it in hythread_resume_all.
> >
> > We may as well leave it as the responsibility of application / TI agent
> > writer not to modify a suspended thread group.
> > Why do you think this should be enforced?
>
> In general, any "good" design should strive to eliminate/minimize
> cases of illegal use of the interface. In other words it should be
> hard to use it in a buggy way. In our case that means it is better to
> ensure integrity inside TM instead of making application responsible
> for that .... if we can do it inside what is the reason not to do it?


Yes.  Good idea provided it really can be done.


Moreover if you look to the spec of hythread_suspend_all it states
> "...This method sets a suspend request for the every thread in the
> group and then returns the iterator that can be used to traverse
> through the suspended threads..." But implementation contradicts with
> that. If group is changed while you are traversing through the group
> you can get wrong thread.


Agreed.  What you describe can be a problem.  I would like to see a
conservative, simple design.  Once we get the sync funtional part robust, we
can look at the performance problems.  While it is possible to
simultaneously walk a link-list while the list is changing, this adds way
too much confusion at this stage of  VM development.  And the VM does not
yet have enough performance where it make sense to look at such fine detail.


 What is even worse you can get a crash when
> one thread is iterating through the group while another thread inserts
> new elements (or removes) to it.


Actually a crash would be the best failure I can think of.  At least the
crash occurs somewhere close to the bad code.  A worse scenario is that you
can fool yourself into thinking you have processed all the threads on the
list when,  in fact, you may have missed a couple.  And the impact may not
surface for 10 seconds...

Evgueni
> >
> >
> > ---------------------------------------------------------------------
> > Terms of use : http://incubator.apache.org/harmony/mailing.html
> > To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> >
> >
>
> ---------------------------------------------------------------------
> Terms of use : http://incubator.apache.org/harmony/mailing.html
> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>
>


-- 
Weldon Washburn
Intel Middleware Products Division

------=_Part_65694_7512858.1161194155207--